sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
OK, this is pretty clean now! :-)
================
Comment at: clangd/ClangdServer.cpp:139
+ FileIdx(BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
+ Units(FileIdx
+ ? [this](const Context &Ctx, PathRef Path,
----------------
A comment explaining when this runs and what it does might be nice.
Also this doesn't seem like an ideal long-term solution: rebuilding an index
can be slow (less the symbol extraction, and more the rebuilding of index data
structures) and we may be able to index on less critical paths.
Probably also worth a comment.
================
Comment at: clangd/ClangdUnit.cpp:617
+ new CppFile(FileName, std::move(Command), StorePreamblesInMemory,
+ std::move(PCHs), std::move(ASTCallback)));
}
----------------
CppFile doesn't need to pass the path, do you want `[FileName,
ASTCallback](const Context &C, ParsedAST *AST) { ASTCallback(C, FileName, AST);
}`
================
Comment at: clangd/ClangdUnitStore.h:28
public:
+ /// \p ASTCallback is called when a file is parsed.
+ explicit CppFileCollection(ASTParsedCallback ASTCallback)
----------------
synchronously... maybe mention this will block diagnostics and doing expensive
things would be bad
================
Comment at: clangd/tool/ClangdMain.cpp:98
+ "use index built from symbols in opened files"),
+ llvm::cl::init(false));
+
----------------
llvm::cl::Hidden, if it's experimental?
================
Comment at: unittests/clangd/CodeCompleteTests.cpp:561
+TEST(CompletionTest, ASTIndexOneFile) {
+ MockFSProvider FS;
----------------
for this test, I don't see a clear sign that the results come from the index.
Is there a way we know this that you can point out?
For the second test we can tell because there's no #include, but it could use a
comment
================
Comment at: unittests/clangd/CodeCompleteTests.cpp:584
+TEST(CompletionTest, ASTIndexMultiFile) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
----------------
This repeated setup is a bit ugly but I'm planning to pull out a helper for
this anyway.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41289
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
