Details
Diff Detail
- Repository
- rCTE Clang Tools Extra
- Build Status
Buildable 13166 Build 13166: arc lint + arc unit
Event Timeline
OK, this is pretty clean now! :-)
clangd/ClangdServer.cpp | ||
---|---|---|
139 | 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. | |
clangd/ClangdUnit.cpp | ||
617 | CppFile doesn't need to pass the path, do you want [FileName, ASTCallback](const Context &C, ParsedAST *AST) { ASTCallback(C, FileName, AST); } | |
clangd/ClangdUnitStore.h | ||
28 | synchronously... maybe mention this will block diagnostics and doing expensive things would be bad | |
clangd/tool/ClangdMain.cpp | ||
98 | llvm:🆑:Hidden, if it's experimental? | |
unittests/clangd/CodeCompleteTests.cpp | ||
521 | for this test, I don't see a clear sign that the results come from the index. For the second test we can tell because there's no #include, but it could use a comment | |
544 | This repeated setup is a bit ugly but I'm planning to pull out a helper for this anyway. |
Thanks for the quick review!
clangd/ClangdUnit.cpp | ||
---|---|---|
617 | The downside is 1) we need to worry about the life time of FileName 2) we need another type for the new callback, so I am inclined to simply forward the callback. |
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.