[clang-tools-extra] ca9fd22 - [clangd] Set "spelled" flag for constructor references.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-02-06T16:59:45+01:00
New Revision: ca9fd22adb5a633429ea85d2d62e5414ca35ab11

URL: https://github.com/llvm/llvm-project/commit/ca9fd22adb5a633429ea85d2d62e5414ca35ab11
DIFF: https://github.com/llvm/llvm-project/commit/ca9fd22adb5a633429ea85d2d62e5414ca35ab11.diff

LOG: [clangd] Set "spelled" flag for constructor references.

Summary:
DeclarationName for cxx constructor is special, it is not an identifier.
thus the "Spelled" flag are not set for all ctor references, this patch
fixes it.

Reviewers: kbobyrev

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/index/SymbolCollector.cpp
    clang-tools-extra/clangd/index/SymbolID.h
    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 8a08ae894526..06190914ee8d 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -574,7 +574,7 @@ void SymbolCollector::finish() {
   // FIXME: All MacroRefs are marked as Spelled now, but this should be checked.
   for (const auto &IDAndRefs : MacroRefs)
     for (const auto &LocAndRole : IDAndRefs.second)
-      CollectRef(IDAndRefs.first, LocAndRole);
+      CollectRef(IDAndRefs.first, LocAndRole, /*Spelled=*/true);
   // Populate Refs slab from DeclRefs.
   llvm::DenseMap<FileID, std::vector<syntax::Token>> FilesToTokensCache;
   for (auto &DeclAndRef : DeclRefs) {
@@ -592,7 +592,10 @@ void SymbolCollector::finish() {
         const auto *IdentifierToken =
             spelledIdentifierTouching(LocAndRole.first, Tokens);
         DeclarationName Name = DeclAndRef.first->getDeclName();
-        bool Spelled = IdentifierToken && Name.isIdentifier() &&
+        const auto NameKind = Name.getNameKind();
+        bool IsTargetKind = NameKind == DeclarationName::Identifier ||
+                            NameKind == DeclarationName::CXXConstructorName;
+        bool Spelled = IdentifierToken && IsTargetKind &&
                        Name.getAsString() == IdentifierToken->text(SM);
         CollectRef(*ID, LocAndRole, Spelled);
       }

diff  --git a/clang-tools-extra/clangd/index/SymbolID.h b/clang-tools-extra/clangd/index/SymbolID.h
index d715f4d0266c..ec7e73301a68 100644
--- a/clang-tools-extra/clangd/index/SymbolID.h
+++ b/clang-tools-extra/clangd/index/SymbolID.h
@@ -36,6 +36,9 @@ class SymbolID {
   bool operator==(const SymbolID &Sym) const {
     return HashValue == Sym.HashValue;
   }
+  bool operator!=(const SymbolID &Sym) const {
+    return !(*this == Sym);
+  }
   bool operator<(const SymbolID &Sym) const {
     return HashValue < Sym.HashValue;
   }

diff  --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 773260c883c0..d3cd9f542de3 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -700,39 +700,70 @@ TEST_F(SymbolCollectorTest, MacrosWithRefFilter) {
   EXPECT_THAT(Refs, IsEmpty());
 }
 
-TEST_F(SymbolCollectorTest, SpelledReference) {
-  Annotations Header(R"cpp(
-  struct Foo;
-  #define MACRO Foo
-  )cpp");
-  Annotations Main(R"cpp(
-  struct $spelled[[Foo]] {
-    $spelled[[Foo]]();
-    ~$spelled[[Foo]]();
+TEST_F(SymbolCollectorTest, SpelledReferences) {
+  struct {
+    llvm::StringRef Header;
+    llvm::StringRef Main;
+    llvm::StringRef TargetSymbolName;
+  } TestCases[] = {
+    {
+      R"cpp(
+        struct Foo;
+        #define MACRO Foo
+      )cpp",
+      R"cpp(
+        struct $spelled[[Foo]] {
+          $spelled[[Foo]]();
+          ~$spelled[[Foo]]();
+        };
+        $spelled[[Foo]] Variable1;
+        $implicit[[MACRO]] Variable2;
+      )cpp",
+      "Foo",
+    },
+    {
+      R"cpp(
+        class Foo {
+        public:
+          Foo() = default;
+        };
+      )cpp",
+      R"cpp(
+        void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();}
+      )cpp",
+      "Foo::Foo" /// constructor.
+    },
   };
-  $spelled[[Foo]] Variable1;
-  $implicit[[MACRO]] Variable2;
-  )cpp");
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = false;
-  runSymbolCollector(Header.code(), Main.code());
-  const auto SpelledRanges = Main.ranges("spelled");
-  const auto ImplicitRanges = Main.ranges("implicit");
-  RefSlab::Builder SpelledSlabBuilder, ImplicitSlabBuilder;
-  for (const auto &SymbolAndRefs : Refs) {
-    const auto Symbol = SymbolAndRefs.first;
-    for (const auto &Ref : SymbolAndRefs.second)
-      if ((Ref.Kind & RefKind::Spelled) != RefKind::Unknown)
-        SpelledSlabBuilder.insert(Symbol, Ref);
-      else
-        ImplicitSlabBuilder.insert(Symbol, Ref);
+  for (const auto& T : TestCases) {
+    Annotations Header(T.Header);
+    Annotations Main(T.Main);
+    // Reset the file system.
+    InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem;
+    runSymbolCollector(Header.code(), Main.code());
+
+    const auto SpelledRanges = Main.ranges("spelled");
+    const auto ImplicitRanges = Main.ranges("implicit");
+    RefSlab::Builder SpelledSlabBuilder, ImplicitSlabBuilder;
+    const auto TargetID = findSymbol(Symbols, T.TargetSymbolName).ID;
+    for (const auto &SymbolAndRefs : Refs) {
+      const auto ID = SymbolAndRefs.first;
+      if (ID != TargetID)
+        continue;
+      for (const auto &Ref : SymbolAndRefs.second)
+        if ((Ref.Kind & RefKind::Spelled) != RefKind::Unknown)
+          SpelledSlabBuilder.insert(ID, Ref);
+        else
+          ImplicitSlabBuilder.insert(ID, Ref);
+    }
+    const auto SpelledRefs = std::move(SpelledSlabBuilder).build(),
+               ImplicitRefs = std::move(ImplicitSlabBuilder).build();
+    EXPECT_THAT(SpelledRefs,
+                Contains(Pair(TargetID, HaveRanges(SpelledRanges))));
+    EXPECT_THAT(ImplicitRefs,
+                Contains(Pair(TargetID, HaveRanges(ImplicitRanges))));
   }
-  const auto SpelledRefs = std::move(SpelledSlabBuilder).build(),
-             ImplicitRefs = std::move(ImplicitSlabBuilder).build();
-  EXPECT_THAT(SpelledRefs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
-                                         HaveRanges(SpelledRanges))));
-  EXPECT_THAT(ImplicitRefs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
-                                          HaveRanges(ImplicitRanges))));
 }
 
 TEST_F(SymbolCollectorTest, NameReferences) {