sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/clangd/index/ProjectAware.h:27
/// Returns an index that answers queries using external indices.
IndexGenerator
/// can be used to customize how to generate an index from an external source.
+std::unique_ptr<SymbolIndex> createProjectAwareIndex(IndexFactory);
----------------
while here, IndexGenerator -> IndexFactory, and "can be use to customize" -->
"specifies" now that it's mandatory
================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:558
+// We define this function here as depending on clangdRemoteIndex within
+// clangDaemon introduces a cyclic dependency.
----------------
This sort of a comment seems to be rebutting the previous version of the code,
which is important today but probably not in the long run.
It may not be needed at all - this factoring actually looks more natural than I
expected!
If it is, I think it belongs on createProjectAwareIndex rather than here, as an
aside.
`(IndexFactory must be injected because this code cannot depend on the remote
index client)`
================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:561
+std::unique_ptr<SymbolIndex>
+projectAwareIndexFactory(const Config::ExternalIndexSpec &External,
+ AsyncTaskRunner &Tasks) {
----------------
maybe just loadExternalIndex, which more clearly describes what it *does* as
opposed to where it's passed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91860/new/
https://reviews.llvm.org/D91860
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
