ilya-biryukov added inline comments.

================
Comment at: clangd/ClangdUnit.h:143
+std::shared_ptr<const PreambleData>
+buildPreamble(PathRef FileName, CompilerInvocation &CI,
+              std::shared_ptr<const PreambleData> OldPreamble,
----------------
sammccall wrote:
> nit: i think filename here is only used for logging, just use 
> Inputs.CompileCommand.Filename?
Tried doing that, but the filename parameter is actually passed to 
PreambleCallback that updates the dynamic index. Using filename from compile 
command there seems fragile, so I kept the parameter for now.


================
Comment at: clangd/TUScheduler.h:66
+              std::chrono::steady_clock::duration UpdateDebounce,
+              ASTRetentionPolicy RetentionPolicy = {});
   ~TUScheduler();
----------------
sammccall wrote:
> does this actually have more than one caller? what's the plan for exposing 
> this option to embedders/CLI users (not saying we necessarily need the 
> latter)?
Yes, just one caller outside the tests.
The plan was to expose it only in `ClangdServer` for now. Giving this knob in 
CLI might be useful, if we have good reasons for that, but I hope that we could 
pick the default that work for everyone instead.
Added that as a parameter of `ClangdServer`.

Maybe we should move the default value of 3 to `ClangdServer`? WDYT?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to