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 @@ -173,7 +173,8 @@ Callbacks *Callbacks) : ConfigProvider(Opts.ConfigProvider), TFS(TFS), DynamicIdx(Opts.BuildDynamicSymbolIndex - ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex) + ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex, + Opts.CollectMainFileRefs) : nullptr), GetClangTidyOptions(Opts.GetClangTidyOptions), SuggestMissingIncludes(Opts.SuggestMissingIncludes), 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 @@ -137,6 +137,8 @@ // file. Called with the empty string for other tasks. // (When called, the context from BackgroundIndex construction is active). std::function ContextProvider = nullptr; + // Whether to collect references to main-file-only symbols. + bool CollectMainFileRefs = false; }; /// Creates a new background index and starts its threads. @@ -188,6 +190,7 @@ const ThreadsafeFS &TFS; const GlobalCompilationDatabase &CDB; 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,6 +95,7 @@ BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts) : SwapIndex(std::make_unique()), TFS(TFS), CDB(CDB), ContextProvider(std::move(Opts.ContextProvider)), + CollectMainFileRefs(Opts.CollectMainFileRefs), Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize), IndexStorageFactory(std::move(IndexStorageFactory)), Queue(std::move(Opts.OnProgress)), @@ -301,6 +302,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 @@ -104,7 +104,7 @@ /// FIXME: Expose an interface to remove files that are closed. class FileIndex : public MergedIndex { public: - FileIndex(bool UseDex = true); + FileIndex(bool UseDex = true, bool CollectMainFileRefs = false); /// Update preamble symbols of file \p Path with all declarations in \p AST /// and macros in \p PP. @@ -118,6 +118,7 @@ private: bool UseDex; // FIXME: this should be always on. + bool CollectMainFileRefs; // Contains information from each file's preamble only. Symbols and relations // are sharded per declaration file to deduplicate multiple symbols and reduce @@ -152,7 +153,7 @@ /// 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 = false); /// Index declarations from \p AST and macros from \p PP that are declared in /// included headers. 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,11 +206,11 @@ 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, @@ -220,7 +221,8 @@ AST.getTranslationUnitDecl()->decls().end()); return indexSymbols(AST, std::move(PP), DeclsToIndex, /*MainFileMacros=*/nullptr, Includes, - /*IsIndexMainAST=*/false, Version); + /*IsIndexMainAST=*/false, Version, + /*CollectMainFileRefs=*/false); } void FileSymbols::update(llvm::StringRef Key, @@ -371,8 +373,9 @@ llvm_unreachable("Unknown clangd::IndexType"); } -FileIndex::FileIndex(bool UseDex) +FileIndex::FileIndex(bool UseDex, bool CollectMainFileRefs) : MergedIndex(&MainFileIndex, &PreambleIndex), UseDex(UseDex), + CollectMainFileRefs(CollectMainFileRefs), PreambleIndex(std::make_unique()), MainFileIndex(std::make_unique()) {} @@ -415,7 +418,7 @@ } void FileIndex::updateMain(PathRef Path, ParsedAST &AST) { - auto Contents = indexMainDecls(AST); + 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 @@ -334,12 +334,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", @@ -682,6 +689,7 @@ if (!ResourceDir.empty()) Opts.ResourceDir = ResourceDir; Opts.BuildDynamicSymbolIndex = EnableIndex; + 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 @@ -229,6 +229,61 @@ FileURI("unittest:///root/B.cc")})); } +TEST_F(BackgroundIndexTest, MainFileRefs) { + MockFS FS; + FS.Files[testPath("root/A.h")] = R"cpp( + void header_sym(); + )cpp"; + FS.Files[testPath("root/A.cc")] = + "#include \"A.h\"\nstatic void main_sym() { (void)header_sym; }"; + + // Check the behaviour with CollectMainFileRefs = false (the default). + { + llvm::StringMap Storage; + size_t CacheHits = 0; + MemoryShardStorage MSS(Storage, CacheHits); + OverlayCDB CDB(/*Base=*/nullptr); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); + + tooling::CompileCommand Cmd; + Cmd.Filename = testPath("root/A.cc"); + Cmd.Directory = testPath("root"); + Cmd.CommandLine = {"clang++", testPath("root/A.cc")}; + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); + + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + EXPECT_THAT( + runFuzzyFind(Idx, ""), + UnorderedElementsAre(AllOf(Named("header_sym"), NumReferences(1U)), + AllOf(Named("main_sym"), NumReferences(0U)))); + } + + // Check the behaviour with CollectMainFileRefs = true. + { + llvm::StringMap Storage; + size_t CacheHits = 0; + MemoryShardStorage MSS(Storage, CacheHits); + OverlayCDB CDB(/*Base=*/nullptr); + BackgroundIndex::Options Opts; + Opts.CollectMainFileRefs = true; + BackgroundIndex Idx( + FS, CDB, [&](llvm::StringRef) { return &MSS; }, Opts); + + tooling::CompileCommand Cmd; + Cmd.Filename = testPath("root/A.cc"); + Cmd.Directory = testPath("root"); + Cmd.CommandLine = {"clang++", testPath("root/A.cc")}; + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); + + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + EXPECT_THAT( + runFuzzyFind(Idx, ""), + UnorderedElementsAre(AllOf(Named("header_sym"), NumReferences(1U)), + AllOf(Named("main_sym"), NumReferences(1U)))); + } +} + TEST_F(BackgroundIndexTest, ShardStorageTest) { MockFS FS; FS.Files[testPath("root/A.h")] = R"cpp( 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 @@ -714,7 +714,6 @@ 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 = @@ -723,6 +722,20 @@ 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, _)))); + + // Run the collector again with CollectMainFileRefs = true. + // We need to recreate InMemoryFileSystem because runSymbolCollector() + // calls MemoryBuffer::getMemBuffer(), which makes the buffers unusable + // after runSymbolCollector() exits. + InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem(); + CollectorOpts.CollectMainFileRefs = true; + runSymbolCollector(Header.code(), + (Main.code() + SymbolsOnlyInMainCode.code()).str()); + 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) { @@ -908,8 +921,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, @@ -918,7 +932,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"}); @@ -929,8 +943,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")));