[PATCH] D83536: [clangd] Index refs to main-file symbols as well

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/index/Background.h:143
+      std::function<Context(PathRef)> ContextProvider = nullptr,
+      bool CollectMainFileRefs = true);
   ~BackgroundIndex(); // Blocks while the current task finishes.
----------------
please set the default to false


================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:219
+                             const CanonicalIncludes &Includes,
+                             bool CollectMainFileRefs) {
   std::vector<Decl *> DeclsToIndex(
----------------
we don't need the option here right? as `IsMainFileOnly` flag should always be false for symbols inside headers(preamble).


================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:421
+void FileIndex::updateMain(PathRef Path, ParsedAST &AST,
+                           bool CollectMainFileRefs) {
+  auto Contents = indexMainDecls(AST, CollectMainFileRefs);
----------------
i think we should rather put this option into `FileIndex` as a field (similar to `UseDex`) that way we could get rid of some plumbing


================
Comment at: clang-tools-extra/clangd/index/FileIndex.h:119
+  void updateMain(PathRef Path, ParsedAST &AST,
+                  bool CollectMainFileRefs = true);
 
----------------
please drop the default


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83536/new/

https://reviews.llvm.org/D83536