ioeric added inline comments.

================
Comment at: clangd/ClangdServer.cpp:70
+// FIXME(ibiryukov): this should be a generic helper instead.
+class NoopCallbacks : public ParsingCallbacks {
 public:
----------------
Maybe provide noop implementations for `ParsingCallbacks::onPreambleAST()` and 
`ParsingCallbacks::onMainAST` by default?


================
Comment at: clangd/ClangdServer.h:213
+  /// from the file itself. Those have different lifetimes and we merge 
results from both 
+  class DynamicIndex : public ParsingCallbacks {
+  public:
----------------
Can we move this into ClangdServer.cpp as an implementation helper? 


================
Comment at: clangd/ClangdServer.h:246
+  /// If present, an up-to-date of symbols in open files. Read via Index.
+  std::unique_ptr<DynamicIndex> FileIdx;
   // If present, a merged view of FileIdx and an external index. Read via 
Index.
----------------
nit: `s/FileIdx/DymIdx`? (also need to update the comment above)


================
Comment at: clangd/index/FileIndex.cpp:46
+                   AST.getTranslationUnitDecl()->decls().end());
+    TopLevelDecls = Storage;
+  }
----------------
nit: I'd try avoiding modify the input argument if possible. Maybe set 
`Storage` properly according to `TopLevelDecls`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50889



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

Reply via email to