[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

[Aleksandr Platonov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG90684d154516: [clangd] Collect references for externally visible main-file symbols (authored by ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84513

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -624,11 +624,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, 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 @@
     $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");
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -171,7 +171,7 @@
       #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
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -314,7 +314,8 @@
   // 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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84513.280878.patch
Type: text/x-patch
Size: 3435 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200727/0ffd783d/attachment.bin>