[clangd] Catch more symbols in SymbolCollector.

[hokein]

Summary

We currently only collect external-linkage symbols in the collector,
which results in missing some typical symbols (like no-linkage type alias symbols).

This patch relaxes the constraint a bit to allow collecting more symbols.

[ilya-biryukov]

clangd/index/SymbolCollector.cpp

89

Why don't we include symbols from anonymous namespaces too?
They are very similar to static symbols.

[hokein]

clangd/index/SymbolCollector.cpp

89

Yeah, these symbols need a special handling (the qualified name is like foo::<anonymous>::bar), we don't support them well enough. I think it is fine to ignore them at the moment.

[ilya-biryukov]

LGTM (see the review comment about adding a comment in the code too)

clangd/index/SymbolCollector.cpp

89

Makes sense.
Let's add a comment on why we skip them here.

[hokein]

Add a comment for symbols in anonymous namespace.

clangd/index/SymbolCollector.cpp

bool SymbolCollector::handleDeclOccurence(
     ArrayRef<index::SymbolRelation> Relations, FileID FID, unsigned Offset,
     index::IndexDataConsumer::ASTNodeInfo ASTNode) {
   // FIXME: collect all symbol references.
   if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
         Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
     return true;
 
   if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
-    // FIXME: Should we include the internal linkage symbols?
-    if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
+    // FIXME: figure out a way to handle internal linkage symbols (e.g. static
+    // variables, function) defined in the .cc files. Also we skip the symbols
+    // in anonymous namespace as the qualifier names of these symbols are like
+    // `foo::<anonymous>::bar`, which need a special handling.
+    // In real world projects, we have a relatively large set of header files
+    // that define static variables (like "static const int A = 1;"), we still
+    // want to collect these symbols, although they cause potential ODR
+    // violations.
+    if (ND->isInAnonymousNamespace())
       return true;
 
     llvm::SmallString<128> USR;
     if (index::generateUSRForDecl(ND, USR))
       return true;
 
     auto ID = SymbolID(USR);
     if (Symbols.find(ID) != nullptr)

unittests/clangd/SymbolCollectorTests.cpp

 #include "gtest/gtest.h"
 
 #include <memory>
 #include <string>
 
 using testing::Eq;
 using testing::Field;
 using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
 
 // GMock helpers for matching Symbol.
 MATCHER_P(QName, Name, "") {
   return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
 }
 
 namespace clang {
 namespace clangd {

 
 TEST_F(SymbolCollectorTest, CollectSymbol) {
   const std::string Header = R"(
     class Foo {
       void f();
     };
     void f1();
     inline void f2() {}
+    static const int KInt = 2;
+    const char* kStr = "123";
   )";
   const std::string Main = R"(
     namespace {
     void ff() {} // ignore
     }
+
     void f1() {}
+
+    namespace foo {
+    // Type alias
+    typedef int int32;
+    using int32_t = int32;
+
+    // Variable
+    int v1;
+
+    // Namespace
+    namespace bar {
+    int v2;
+    }
+    // Namespace alias
+    namespace baz = bar;
+
+    // FIXME: using declaration is not supported as the IndexAction will ignore
+    // implicit declarations (the implicit using shadow declaration) by default,
+    // and there is no way to customize this behavior at the moment.
+    using bar::v2;
+    } // namespace foo
   )";
   runSymbolCollector(Header, Main);
-  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
-                                            QName("f1"), QName("f2")));
+  EXPECT_THAT(Symbols,
+              UnorderedElementsAreArray(
+                  {QName("Foo"),
+                   QName("Foo::f"),
+                   QName("f1"),
+                   QName("f2"),
+                   QName("KInt"),
+                   QName("kStr"),
+                   QName("foo"),
+                   QName("foo::bar"),
+                   QName("foo::int32"),
+                   QName("foo::int32_t"),
+                   QName("foo::v1"),
+                   QName("foo::bar::v2"),
+                   QName("foo::baz")}));
 }
 
 TEST_F(SymbolCollectorTest, YAMLConversions) {
   const std::string YAML1 = R"(
 ---
 ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
 Name:   'Foo1'
 Scope:   'clang'