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

[hokein]

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.

[hokein]

update.

[kadircet]

clang-tools-extra/clangd/index/SymbolCollector.cpp

597

drive-by: what about destructors?

[kbobyrev]

clang-tools-extra/clangd/index/SymbolCollector.cpp

595–598

nit: const auto?

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

752

nit: maybe add operator != to SymbolKind instead?

[hokein]

clang-tools-extra/clangd/index/SymbolCollector.cpp

597

we aren't interested in destructors (particularly the ~) as there is a Foo typeloc in "~[[Foo]]".

clang-tools-extra/clangd/index/SymbolCollector.cpp

auto CollectRef =
           R.Kind = toRefKind(LocAndRole.second, Spelled);
           Refs.insert(ID, R);
         }
       };
   // Populate Refs slab from MacroRefs.
   // 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) {
     if (auto ID = getSymbolID(DeclAndRef.first)) {
       for (auto &LocAndRole : DeclAndRef.second) {
         const auto FileID = SM.getFileID(LocAndRole.first);
         // FIXME: It's better to use TokenBuffer by passing spelled tokens from
         // the caller of SymbolCollector.
         if (!FilesToTokensCache.count(FileID))
           FilesToTokensCache[FileID] =
               syntax::tokenize(FileID, SM, ASTCtx->getLangOpts());
         llvm::ArrayRef<syntax::Token> Tokens = FilesToTokensCache[FileID];
         // Check if the referenced symbol is spelled exactly the same way the
         // corresponding NamedDecl is. If it is, mark this reference as spelled.
         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);
       }
     }
   }
 
   ReferencedDecls.clear();
   ReferencedMacros.clear();

clang-tools-extra/clangd/index/SymbolID.h

 class SymbolID {
 public:
   SymbolID() = default;
   explicit SymbolID(llvm::StringRef USR);
 
   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;
   }
 
   // The stored hash is truncated to RawSize bytes.
   // This trades off memory against the number of symbols we can handle.
   constexpr static size_t RawSize = 8;
   llvm::StringRef raw() const;

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

 TEST_F(SymbolCollectorTest, MacrosWithRefFilter) {
   Annotations Header("#define $macro[[MACRO]](X) (X + 1)");
   Annotations Main("void foo() { int x = $macro[[MACRO]](1); }");
   CollectorOpts.RefFilter = RefKind::Unknown;
   runSymbolCollector(Header.code(), Main.code());
   EXPECT_THAT(Refs, IsEmpty());
 }
 
-TEST_F(SymbolCollectorTest, SpelledReference) {
-  Annotations Header(R"cpp(
+TEST_F(SymbolCollectorTest, SpelledReferences) {
+  struct {
+    llvm::StringRef Header;
+    llvm::StringRef Main;
+    llvm::StringRef TargetSymbolName;
+  } TestCases[] = {
+    {
+      R"cpp(
         struct Foo;
         #define MACRO Foo
-  )cpp");
-  Annotations Main(R"cpp(
+      )cpp",
+      R"cpp(
         struct $spelled[[Foo]] {
           $spelled[[Foo]]();
           ~$spelled[[Foo]]();
         };
         $spelled[[Foo]] Variable1;
         $implicit[[MACRO]] Variable2;
-  )cpp");
+      )cpp",
+      "Foo",
+    },
+    {
+      R"cpp(
+        class Foo {
+        public:
+          Foo() = default;
+        };
+      )cpp",
+      R"cpp(
+        void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();}
+      )cpp",
+      "Foo::Foo" /// constructor.
+    },
+  };
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = false;
+  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 Symbol = SymbolAndRefs.first;
+      const auto ID = SymbolAndRefs.first;
+      if (ID != TargetID)
+        continue;
       for (const auto &Ref : SymbolAndRefs.second)
         if ((Ref.Kind & RefKind::Spelled) != RefKind::Unknown)
-        SpelledSlabBuilder.insert(Symbol, Ref);
+          SpelledSlabBuilder.insert(ID, Ref);
         else
-        ImplicitSlabBuilder.insert(Symbol, Ref);
+          ImplicitSlabBuilder.insert(ID, Ref);
     }
     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))));
+    EXPECT_THAT(SpelledRefs,
+                Contains(Pair(TargetID, HaveRanges(SpelledRanges))));
+    EXPECT_THAT(ImplicitRefs,
+                Contains(Pair(TargetID, HaveRanges(ImplicitRanges))));
+  }
 }
 
 TEST_F(SymbolCollectorTest, NameReferences) {
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
   Annotations Header(R"(
     class [[Foo]] {
     public: