[clang-tools-extra] r322067 - [clangd] Catch more symbols in SymbolCollector.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 9 02:44:09 PST 2018


Author: hokein
Date: Tue Jan  9 02:44:09 2018
New Revision: 322067

URL: http://llvm.org/viewvc/llvm-project?rev=322067&view=rev
Log:
[clangd] Catch more symbols in SymbolCollector.

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.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D41759

Modified:
    clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
    clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=322067&r1=322066&r2=322067&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Tue Jan  9 02:44:09 2018
@@ -80,8 +80,15 @@ bool SymbolCollector::handleDeclOccurenc
     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;

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=322067&r1=322066&r2=322067&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Tue Jan  9 02:44:09 2018
@@ -29,6 +29,7 @@
 using testing::Eq;
 using testing::Field;
 using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
 
 // GMock helpers for matching Symbol.
 MATCHER_P(QName, Name, "") {
@@ -97,16 +98,53 @@ TEST_F(SymbolCollectorTest, CollectSymbo
     };
     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) {




More information about the cfe-commits mailing list