Author: Aleksandr Platonov Date: 2020-07-27T15:35:53+03:00 New Revision: 90684d1545167ee4e0c93d8eaf6ba4a3c7ab710e
URL: https://github.com/llvm/llvm-project/commit/90684d1545167ee4e0c93d8eaf6ba4a3c7ab710e DIFF: https://github.com/llvm/llvm-project/commit/90684d1545167ee4e0c93d8eaf6ba4a3c7ab710e.diff LOG: [clangd] Collect references for externally visible main-file symbols Summary: Without this patch clangd does not collect references for main-file symbols if there is no public declaration in preamble. Example: `test1.c` ``` void f1() {} ``` `test2.c` ``` extern void f1(); void f2() { f^1(); } ``` `Find all references` does not show definition of f1() in the result, but GTD works OK. Reviewers: sammccall, kadircet Reviewed By: kadircet Subscribers: ilya-golovenko, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D84513 Added: Modified: clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 6c11399c87b6..c163951aff9b 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -314,7 +314,8 @@ bool SymbolCollector::handleDeclOccurrence( // 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 && !isa<NamespaceDecl>(ND) && + if (CollectRef && (!IsMainFileOnly || ND->isExternallyVisible()) && + !isa<NamespaceDecl>(ND) && (Opts.RefsInHeaders || SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID())) DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles); diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp index 70d5156b1072..f1c582ef1abe 100644 --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -171,7 +171,7 @@ TEST_F(BackgroundIndexTest, IndexTwoFiles) { #endif )cpp"; FS.Files[testPath("root/A.cc")] = - "#include \"A.h\"\nvoid g() { (void)common; }"; + "#include \"A.h\"\nstatic void g() { (void)common; }"; FS.Files[testPath("root/B.cc")] = R"cpp( #define A 0 diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp index 9e4f75b5cca3..3614ab2c5cb9 100644 --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -624,11 +624,13 @@ TEST_F(SymbolCollectorTest, Refs) { 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, c, FUNC) had no refs collected. + // 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, Not(Contains(Pair(findSymbol(MainSymbols, "a").ID, _)))); - EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "b").ID, _)))); + 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, _)))); } @@ -816,11 +818,15 @@ TEST_F(SymbolCollectorTest, HeaderAsMainFile) { $Foo[[Foo]] fo; } )"); - // The main file is normal .cpp file, we shouldn't collect any refs of symbols - // which are not declared in the preamble. + // The main file is normal .cpp file, we should collect the refs + // for externally visible symbols. TestFileName = testPath("foo.cpp"); runSymbolCollector("", Header.code()); - EXPECT_THAT(Refs, UnorderedElementsAre()); + EXPECT_THAT(Refs, + UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID, + HaveRanges(Header.ranges("Foo"))), + Pair(findSymbol(Symbols, "Func").ID, + HaveRanges(Header.ranges("Func"))))); // Run the .h file as main file, we should collect the refs. TestFileName = testPath("foo.h"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
