kbobyrev added inline comments.

================
Comment at: clangd/tool/ClangdMain.cpp:48
+// loading.
+class AsyncLoadIndex : public SymbolIndex {
+public:
----------------
Also, do we want only static index to be built asynchronously? Do we want it to 
be used only in our Clangd tool driver? Loading dynamic index might also be 
useful if we have this kind of behavior for the static one. I'm not completely 
sure about that, though.


================
Comment at: clangd/tool/ClangdMain.cpp:78
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-                : MemIndex::build(std::move(SymsBuilder).build());
+  size_t estimateMemoryUsage() const override { return 0; }
+
----------------
Shouldn't this `index()->estimateMemoryUsage()` when the index is built?


================
Comment at: clangd/tool/ClangdMain.cpp:102
+// that all symbols can be managed in memory.
+std::unique_ptr<SymbolIndex> buildStaticIndex(llvm::StringRef YamlSymbolFile) {
+  return llvm::make_unique<AsyncLoadIndex>(
----------------
Do we want to be more explicit about loading index asynchronously? If we're not 
that fact might be implicit to the user (e.g. "my Clangd is not frozen, but 
global completion doesn't work"), CLI flag/documentation entry might solve that 
issue. Introducing tons of not-so-useful flags is not optimal, though, and I'm 
also not sure about that.

What do you think?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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

Reply via email to