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