[clangd] Add SymbolID to LocatedSymbol.

[usaxena95]

Summary

This is useful for running in batch mode.
Getting the SymbolID from via getSymbolInfo may give SymbolID
of a symbol different from that located by LocateSymbolAt (they
have different semantics of choosing the symbol.)

[sammccall]

LG, but can you amend the description a bit?

We do not have to both locate the symbol and also query the SymbolID (using getSymbolInfo).

IIRC this isn't really the reason, it's because getSymbolInfo and locateSymbolAt don't have the same semantics *for choosing symbols* and we want the ID of the symbol that matches the latter.

clang-tools-extra/clangd/XRefs.cpp

260

note that this populates SymbolID even if there's no ID available, so it's incorrect if the type is Optional<SymbolID>

clang-tools-extra/clangd/XRefs.h

51

There are other cases too, maybe just "if available"

52

SymbolID is already inherently optional (it has a zero value for a whil enow)

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

923

nit, just EXPECT_TRUE(Results[0].ID), and remove the matcher?

[usaxena95]

Addressed comments.

clang-tools-extra/clangd/XRefs.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H
 
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "index/Index.h"
+#include "index/SymbolID.h"
 #include "index/SymbolLocation.h"
 #include "support/Path.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Type.h"
 #include "clang/Format/Format.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/raw_ostream.h"

 // For some types of symbol, e.g. macros, definition == declaration always.
 struct LocatedSymbol {
   // The (unqualified) name of the symbol.
   std::string Name;
   // The canonical or best declaration: where most users find its interface.
   Location PreferredDeclaration;
   // Where the symbol is defined, if known. May equal PreferredDeclaration.
   llvm::Optional<Location> Definition;
+  // SymbolID of the located symbol if available.
+  SymbolID ID;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const LocatedSymbol &);
 /// Get definition of symbol at a specified \p Pos.
 /// Multiple locations may be returned, corresponding to distinct symbols.
 std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
                                           const SymbolIndex *Index = nullptr);
 
 // Tries to provide a textual fallback for locating a symbol by looking up the

clang-tools-extra/clangd/XRefs.cpp

locateMacroReferent(const syntax::Token &TouchedIdentifier, ParsedAST &AST,
                     llvm::StringRef MainFilePath) {
   if (auto M = locateMacroAt(TouchedIdentifier, AST.getPreprocessor())) {
     if (auto Loc =
             makeLocation(AST.getASTContext(), M->NameLoc, MainFilePath)) {
       LocatedSymbol Macro;
       Macro.Name = std::string(M->Name);
       Macro.PreferredDeclaration = *Loc;
       Macro.Definition = Loc;
+      Macro.ID = getSymbolID(M->Name, M->Info, AST.getSourceManager());
       return Macro;
     }
   }
   return llvm::None;
 }
 
 // A wrapper around `Decl::getCanonicalDecl` to support cases where Clang's
 // definition of a canonical declaration doesn't match up to what a programmer
auto AddResultDecl = [&](const NamedDecl *D) {
     auto Loc =
         makeLocation(AST.getASTContext(), nameLocation(*D, SM), MainFilePath);
     if (!Loc)
       return;
 
     Result.emplace_back();
     Result.back().Name = printName(AST.getASTContext(), *D);
     Result.back().PreferredDeclaration = *Loc;
+    Result.back().ID = getSymbolID(D);
     if (const NamedDecl *Def = getDefinition(D))
       Result.back().Definition = makeLocation(
           AST.getASTContext(), nameLocation(*Def, SM), MainFilePath);
 
     // Record SymbolID for index lookup later.
     if (auto ID = getSymbolID(D))
       ResultIndex[ID] = Result.size() - 1;
   };
for (const NamedDecl *D : Decls) {
 
     auto Loc = makeLocation(ASTContext, nameLocation(*D, SM), *MainFilePath);
     if (!Loc)
       continue;
 
     Results.emplace_back();
     Results.back().Name = printName(ASTContext, *D);
     Results.back().PreferredDeclaration = *Loc;
+    Results.back().ID = getSymbolID(D);
     if (const NamedDecl *Def = getDefinition(D))
       Results.back().Definition =
           makeLocation(ASTContext, nameLocation(*Def, SM), *MainFilePath);
   }
 
   return Results;
 }
 
auto MaybeDeclLoc =
         indexToLSPLocation(Sym.CanonicalDeclaration, MainFilePath);
     if (!MaybeDeclLoc) {
       log("locateSymbolNamedTextuallyAt: {0}", MaybeDeclLoc.takeError());
       return;
     }
     LocatedSymbol Located;
     Located.PreferredDeclaration = *MaybeDeclLoc;
     Located.Name = (Sym.Name + Sym.TemplateSpecializationArgs).str();
+    Located.ID = Sym.ID;
     if (Sym.Definition) {
       auto MaybeDefLoc = indexToLSPLocation(Sym.Definition, MainFilePath);
       if (!MaybeDefLoc) {
         log("locateSymbolNamedTextuallyAt: {0}", MaybeDefLoc.takeError());
         return;
       }
       Located.PreferredDeclaration = *MaybeDefLoc;
       Located.Definition = *MaybeDefLoc;
findExplicitReferences(
         }
       },
       AST.getHeuristicResolver());
   return DeclRefs;
 }
 
 } // namespace clangd
 } // namespace clang
 No newline at end of file

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

MATCHER_P3(Sym, Name, Decl, DefOrNone, "") {
   }
   return true;
 }
 
 MATCHER_P(Sym, Name, "") { return arg.Name == Name; }
 
 MATCHER_P(RangeIs, R, "") { return arg.Loc.range == R; }
 MATCHER_P(AttrsAre, A, "") { return arg.Attributes == A; }
+MATCHER_P(HasID, ID, "") { return arg.ID == ID; }
 
 TEST(LocateSymbol, WithIndex) {
   Annotations SymbolHeader(R"cpp(
         class $forward[[Forward]];
         class $foo[[Foo]] {};
 
         void $f1[[f1]]();
 
for (const char *Test : Tests) {
     auto AST = TU.build();
     auto Results = locateSymbolAt(AST, T.point());
 
     if (!WantDecl) {
       EXPECT_THAT(Results, IsEmpty()) << Test;
     } else {
       ASSERT_THAT(Results, ::testing::SizeIs(1)) << Test;
       EXPECT_EQ(Results[0].PreferredDeclaration.range, *WantDecl) << Test;
+      EXPECT_TRUE(Results[0].ID) << Test;
       llvm::Optional<Range> GotDef;
       if (Results[0].Definition)
         GotDef = Results[0].Definition->range;
       EXPECT_EQ(WantDef, GotDef) << Test;
     }
   }
 }
+TEST(LocateSymbol, ValidSymbolID) {
+  auto T = Annotations(R"cpp(
+    #define MACRO(x, y) ((x) + (y))
+    int add(int x, int y) { return $MACRO^MACRO(x, y); }
+    int sum = $add^add(1, 2);
+  )cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  EXPECT_THAT(locateSymbolAt(AST, T.point("add"), Index.get()),
+              ElementsAre(AllOf(Sym("add"),
+                                HasID(getSymbolID(&findDecl(AST, "add"))))));
+  EXPECT_THAT(
+      locateSymbolAt(AST, T.point("MACRO"), Index.get()),
+      ElementsAre(AllOf(Sym("MACRO"),
+                        HasID(findSymbol(TU.headerSymbols(), "MACRO").ID))));
+}
 
 TEST(LocateSymbol, AllMulti) {
   // Ranges in tests:
   //   $declN is the declaration location
   //   $defN is the definition location (if absent, symbol has no definition)
   //
   // NOTE:
   //   N starts at 0.
auto T = Annotations(
       R"cpp(
         struct [[MyClass]] {};
         // Comment mentioning M^yClass
       )cpp");
 
   auto TU = TestTU::withCode(T.code());
   auto AST = TU.build();
   auto Index = TU.index();
-  EXPECT_THAT(locateSymbolAt(AST, T.point(), Index.get()),
-              ElementsAre(Sym("MyClass", T.range(), T.range())));
+  EXPECT_THAT(
+      locateSymbolAt(AST, T.point(), Index.get()),
+      ElementsAre(AllOf(Sym("MyClass", T.range(), T.range()),
+                        HasID(getSymbolID(&findDecl(AST, "MyClass"))))));
 }
 
 TEST(LocateSymbol, Textual) {
   const char *Tests[] = {
       R"cpp(// Comment
         struct [[MyClass]] {};
         // Comment mentioning M^yClass
       )cpp",