[PATCH] D41759: [clangd] Catch more symbols in SymbolCollector.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 129053.
hokein added a comment.

Add a comment for symbols in anonymous namespace.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41759

Files:
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -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 @@
     };
     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) {
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -80,8 +80,15 @@
     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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41759.129053.patch
Type: text/x-patch
Size: 3049 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180109/fbc5c20e/attachment.bin>