diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -111,6 +111,9 @@ /// on background threads. The index is stored in the project root. bool BackgroundIndex = false; + /// Store refs to main-file symbols in the index. + bool CollectMainFileRefs = false; + /// If set, use this index to augment code completion results. SymbolIndex *StaticIndex = nullptr; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -62,20 +62,22 @@ struct UpdateIndexCallbacks : public ParsingCallbacks { UpdateIndexCallbacks(FileIndex *FIndex, ClangdServer::Callbacks *ServerCallbacks, - bool TheiaSemanticHighlighting) + bool TheiaSemanticHighlighting, bool CollectMainFileRefs) : FIndex(FIndex), ServerCallbacks(ServerCallbacks), - TheiaSemanticHighlighting(TheiaSemanticHighlighting) {} + TheiaSemanticHighlighting(TheiaSemanticHighlighting), + CollectMainFileRefs(CollectMainFileRefs) {} void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx, std::shared_ptr PP, const CanonicalIncludes &CanonIncludes) override { if (FIndex) - FIndex->updatePreamble(Path, Version, Ctx, std::move(PP), CanonIncludes); + FIndex->updatePreamble(Path, Version, Ctx, std::move(PP), CanonIncludes, + CollectMainFileRefs); } void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override { if (FIndex) - FIndex->updateMain(Path, AST); + FIndex->updateMain(Path, AST, CollectMainFileRefs); std::vector Diagnostics = AST.getDiagnostics(); std::vector Highlightings; @@ -108,6 +110,7 @@ FileIndex *FIndex; ClangdServer::Callbacks *ServerCallbacks; bool TheiaSemanticHighlighting; + bool CollectMainFileRefs; }; } // namespace @@ -157,8 +160,9 @@ }; return O; }(), - std::make_unique( - DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting)) { + std::make_unique(DynamicIdx.get(), Callbacks, + Opts.TheiaSemanticHighlighting, + Opts.CollectMainFileRefs)) { // Adds an index to the stack, at higher priority than existing indexes. auto AddIndex = [&](SymbolIndex *Idx) { if (this->Index != nullptr) { diff --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h --- a/clang-tools-extra/clangd/index/Background.h +++ b/clang-tools-extra/clangd/index/Background.h @@ -139,7 +139,8 @@ // In production an explicit value is passed. size_t ThreadPoolSize = 4, std::function OnProgress = nullptr, - std::function ContextProvider = nullptr); + std::function ContextProvider = nullptr, + bool CollectMainFileRefs = true); ~BackgroundIndex(); // Blocks while the current task finishes. // Enqueue translation units for indexing. @@ -185,6 +186,7 @@ const GlobalCompilationDatabase &CDB; Context BackgroundContext; std::function ContextProvider; + bool CollectMainFileRefs; llvm::Error index(tooling::CompileCommand); diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -95,10 +95,11 @@ const GlobalCompilationDatabase &CDB, BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize, std::function OnProgress, - std::function ContextProvider) + std::function ContextProvider, bool CollectMainFileRefs) : SwapIndex(std::make_unique()), TFS(TFS), CDB(CDB), BackgroundContext(std::move(BackgroundContext)), ContextProvider(std::move(ContextProvider)), + CollectMainFileRefs(CollectMainFileRefs), Rebuilder(this, &IndexedSymbols, ThreadPoolSize), IndexStorageFactory(std::move(IndexStorageFactory)), Queue(std::move(OnProgress)), @@ -304,6 +305,7 @@ return false; // Skip files that haven't changed, without errors. return true; }; + IndexOpts.CollectMainFileRefs = CollectMainFileRefs; IndexFileIn Index; auto Action = createStaticIndexingAction( diff --git a/clang-tools-extra/clangd/index/FileIndex.h b/clang-tools-extra/clangd/index/FileIndex.h --- a/clang-tools-extra/clangd/index/FileIndex.h +++ b/clang-tools-extra/clangd/index/FileIndex.h @@ -110,11 +110,13 @@ /// and macros in \p PP. void updatePreamble(PathRef Path, llvm::StringRef Version, ASTContext &AST, std::shared_ptr PP, - const CanonicalIncludes &Includes); + const CanonicalIncludes &Includes, + bool CollectMainFileRefs = true); /// Update symbols and references from main file \p Path with /// `indexMainDecls`. - void updateMain(PathRef Path, ParsedAST &AST); + void updateMain(PathRef Path, ParsedAST &AST, + bool CollectMainFileRefs = true); private: bool UseDex; // FIXME: this should be always on. @@ -152,13 +154,14 @@ /// Retrieves symbols and refs of local top level decls in \p AST (i.e. /// `AST.getLocalTopLevelDecls()`). /// Exposed to assist in unit tests. -SlabTuple indexMainDecls(ParsedAST &AST); +SlabTuple indexMainDecls(ParsedAST &AST, bool CollectMainFileRefs = true); /// Index declarations from \p AST and macros from \p PP that are declared in /// included headers. SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST, std::shared_ptr PP, - const CanonicalIncludes &Includes); + const CanonicalIncludes &Includes, + bool CollectMainFileRefs = true); /// Takes slabs coming from a TU (multiple files) and shards them per /// declaration location. diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp --- a/clang-tools-extra/clangd/index/FileIndex.cpp +++ b/clang-tools-extra/clangd/index/FileIndex.cpp @@ -47,12 +47,13 @@ llvm::ArrayRef DeclsToIndex, const MainFileMacros *MacroRefsToIndex, const CanonicalIncludes &Includes, bool IsIndexMainAST, - llvm::StringRef Version) { + llvm::StringRef Version, bool CollectMainFileRefs) { SymbolCollector::Options CollectorOpts; CollectorOpts.CollectIncludePath = true; CollectorOpts.Includes = &Includes; CollectorOpts.CountReferences = false; CollectorOpts.Origin = SymbolOrigin::Dynamic; + CollectorOpts.CollectMainFileRefs = CollectMainFileRefs; index::IndexingOptions IndexOpts; // We only need declarations, because we don't count references. @@ -205,22 +206,23 @@ return std::move(IF); } -SlabTuple indexMainDecls(ParsedAST &AST) { - return indexSymbols(AST.getASTContext(), AST.getPreprocessorPtr(), - AST.getLocalTopLevelDecls(), &AST.getMacros(), - AST.getCanonicalIncludes(), - /*IsIndexMainAST=*/true, AST.version()); +SlabTuple indexMainDecls(ParsedAST &AST, bool CollectMainFileRefs) { + return indexSymbols( + AST.getASTContext(), AST.getPreprocessorPtr(), + AST.getLocalTopLevelDecls(), &AST.getMacros(), AST.getCanonicalIncludes(), + /*IsIndexMainAST=*/true, AST.version(), CollectMainFileRefs); } SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST, std::shared_ptr PP, - const CanonicalIncludes &Includes) { + const CanonicalIncludes &Includes, + bool CollectMainFileRefs) { std::vector DeclsToIndex( AST.getTranslationUnitDecl()->decls().begin(), AST.getTranslationUnitDecl()->decls().end()); return indexSymbols(AST, std::move(PP), DeclsToIndex, /*MainFileMacros=*/nullptr, Includes, - /*IsIndexMainAST=*/false, Version); + /*IsIndexMainAST=*/false, Version, CollectMainFileRefs); } void FileSymbols::update(llvm::StringRef Key, @@ -379,10 +381,11 @@ void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version, ASTContext &AST, std::shared_ptr PP, - const CanonicalIncludes &Includes) { + const CanonicalIncludes &Includes, + bool CollectMainFileRefs) { IndexFileIn IF; - std::tie(IF.Symbols, std::ignore, IF.Relations) = - indexHeaderSymbols(Version, AST, std::move(PP), Includes); + std::tie(IF.Symbols, std::ignore, IF.Relations) = indexHeaderSymbols( + Version, AST, std::move(PP), Includes, CollectMainFileRefs); FileShardedIndex ShardedIndex(std::move(IF)); for (auto Uri : ShardedIndex.getAllSources()) { auto IF = ShardedIndex.getShard(Uri); @@ -414,8 +417,9 @@ } } -void FileIndex::updateMain(PathRef Path, ParsedAST &AST) { - auto Contents = indexMainDecls(AST); +void FileIndex::updateMain(PathRef Path, ParsedAST &AST, + bool CollectMainFileRefs) { + auto Contents = indexMainDecls(AST, CollectMainFileRefs); MainFileSymbols.update( Path, std::make_unique(std::move(std::get<0>(Contents))), std::make_unique(std::move(std::get<1>(Contents))), diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -78,6 +78,8 @@ /// Collect symbols local to main-files, such as static functions /// and symbols inside an anonymous namespace. bool CollectMainFileSymbols = true; + /// Collect references to main-file symbols. + bool CollectMainFileRefs = false; /// If set to true, SymbolCollector will collect doc for all symbols. /// Note that documents of symbols being indexed for completion will always /// be collected regardless of this option. diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -309,12 +309,13 @@ if (IsOnlyRef && !CollectRef) return true; - // Do not store references to main-file symbols. // Unlike other fields, e.g. Symbols (which use spelling locations), we use // file locations for references (as it aligns the behavior of clangd's // AST-based xref). // FIXME: we should try to use the file locations for other fields. - if (CollectRef && (!IsMainFileOnly || ND->isExternallyVisible()) && + if (CollectRef && + (!IsMainFileOnly || Opts.CollectMainFileRefs || + ND->isExternallyVisible()) && !isa(ND) && (Opts.RefsInHeaders || SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID())) diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -450,6 +450,13 @@ init(true), }; +opt CollectMainFileRefs{ + "collect-main-file-refs", + cat(Misc), + desc("Store references to main-file-only symbols in the index"), + init(false), +}; + #if CLANGD_ENABLE_REMOTE opt RemoteIndexAddress{ "remote-index-address", @@ -683,6 +690,7 @@ Opts.ResourceDir = ResourceDir; Opts.BuildDynamicSymbolIndex = EnableIndex; Opts.BackgroundIndex = EnableBackgroundIndex; + Opts.CollectMainFileRefs = CollectMainFileRefs; std::unique_ptr StaticIdx; std::future AsyncIndexLoad; // Block exit while loading the index. if (EnableIndex && !IndexFile.empty()) { diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -199,7 +199,7 @@ EXPECT_THAT(runFuzzyFind(Idx, ""), UnorderedElementsAre(AllOf(Named("common"), NumReferences(1U)), AllOf(Named("A_CC"), NumReferences(0U)), - AllOf(Named("g"), NumReferences(0U)), + AllOf(Named("g"), NumReferences(1U)), AllOf(Named("f_b"), Declared(), Not(Defined()), NumReferences(0U)))); @@ -212,7 +212,7 @@ EXPECT_THAT(runFuzzyFind(Idx, ""), UnorderedElementsAre(AllOf(Named("common"), NumReferences(5U)), AllOf(Named("A_CC"), NumReferences(0U)), - AllOf(Named("g"), NumReferences(0U)), + AllOf(Named("g"), NumReferences(1U)), AllOf(Named("f_b"), Declared(), Defined(), NumReferences(1U)))); diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -613,6 +613,7 @@ )"); CollectorOpts.RefFilter = RefKind::All; CollectorOpts.CollectMacro = true; + CollectorOpts.CollectMainFileRefs = true; runSymbolCollector(Header.code(), (Main.code() + SymbolsOnlyInMainCode.code()).str()); EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID, @@ -624,15 +625,13 @@ EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _)))); EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID, HaveRanges(Main.ranges("macro"))))); - // Symbols *only* in the main file: - // - (a, b) externally visible and should have refs. - // - (c, FUNC) externally invisible and had no refs collected. - auto MainSymbols = - TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols(); - EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "a").ID, _))); - EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "b").ID, _))); - EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _)))); - EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _)))); + // Symbols *only* in the main file (a, b, c) should have refs collected + // as well. + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "a").ID, _))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "b").ID, _))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "c").ID, _))); + // However, references to main-file macros are not collected. + EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "FUNC").ID, _)))); } TEST_F(SymbolCollectorTest, MacroRefInHeader) { @@ -708,12 +707,12 @@ llvm::StringRef Main; llvm::StringRef TargetSymbolName; } TestCases[] = { - { - R"cpp( + { + R"cpp( struct Foo; #define MACRO Foo )cpp", - R"cpp( + R"cpp( struct $spelled[[Foo]] { $spelled[[Foo]](); ~$spelled[[Foo]](); @@ -721,24 +720,24 @@ $spelled[[Foo]] Variable1; $implicit[[MACRO]] Variable2; )cpp", - "Foo", - }, - { - R"cpp( + "Foo", + }, + { + R"cpp( class Foo { public: Foo() = default; }; )cpp", - R"cpp( + R"cpp( void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();} )cpp", - "Foo::Foo" /// constructor. - }, + "Foo::Foo" /// constructor. + }, }; CollectorOpts.RefFilter = RefKind::All; CollectorOpts.RefsInHeaders = false; - for (const auto& T : TestCases) { + for (const auto &T : TestCases) { Annotations Header(T.Header); Annotations Main(T.Main); // Reset the file system. @@ -818,8 +817,9 @@ $Foo[[Foo]] fo; } )"); - // The main file is normal .cpp file, we should collect the refs - // for externally visible symbols. + // We should collect refs to main-file symbols in all cases: + + // 1. The main file is normal .cpp file. TestFileName = testPath("foo.cpp"); runSymbolCollector("", Header.code()); EXPECT_THAT(Refs, @@ -828,7 +828,7 @@ Pair(findSymbol(Symbols, "Func").ID, HaveRanges(Header.ranges("Func"))))); - // Run the .h file as main file, we should collect the refs. + // 2. Run the .h file as main file. TestFileName = testPath("foo.h"); runSymbolCollector("", Header.code(), /*ExtraArgs=*/{"-xobjective-c++-header"}); @@ -839,8 +839,7 @@ Pair(findSymbol(Symbols, "Func").ID, HaveRanges(Header.ranges("Func"))))); - // Run the .hh file as main file (without "-x c++-header"), we should collect - // the refs as well. + // 3. Run the .hh file as main file (without "-x c++-header"). TestFileName = testPath("foo.hh"); runSymbolCollector("", Header.code()); EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));