[clangd] Enable textual fallback for go-to-definition on dependent names

Reviewers: sammccall

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

Tags: #clang

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

clang-tools-extra/clangd/XRefs.cpp

@@ -22,6 +22,7 @@
 #include "index/Relation.h"
 #include "index/SymbolLocation.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Attrs.inc"
 #include "clang/AST/Decl.h"
@@ -139,17 +140,20 @@ SymbolLocation getPreferredLocation(const Location &ASTLoc,
   return Merged.CanonicalDeclaration;
 }
 
-std::vector<const NamedDecl *> getDeclAtPosition(ParsedAST &AST,
-                                                 SourceLocation Pos,
-                                                 DeclRelationSet Relations) {
+std::vector<const NamedDecl *>
+getDeclAtPosition(ParsedAST &AST, SourceLocation Pos, DeclRelationSet Relations,
+                  ASTNodeKind *NodeKind = nullptr) {
   unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(Pos).second;
   std::vector<const NamedDecl *> Result;
   SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Offset,
                             Offset, [&](SelectionTree ST) {
                               if (const SelectionTree::Node *N =
-                                      ST.commonAncestor())
+                                      ST.commonAncestor()) {
+                                if (NodeKind)
+                                  *NodeKind = N->ASTNode.getNodeKind();
                                 llvm::copy(targetDecl(N->ASTNode, Relations),
                                            std::back_inserter(Result));
+                              }
                               return !Result.empty();
                             });
   return Result;
@@ -221,7 +225,7 @@ locateMacroReferent(const syntax::Token &TouchedIdentifier, ParsedAST &AST,
 std::vector<LocatedSymbol>
 locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
                   ParsedAST &AST, llvm::StringRef MainFilePath,
-                  const SymbolIndex *Index) {
+                  const SymbolIndex *Index, ASTNodeKind *NodeKind) {
   const SourceManager &SM = AST.getSourceManager();
   // Results follow the order of Symbols.Decls.
   std::vector<LocatedSymbol> Result;
@@ -250,7 +254,8 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
   // Emit all symbol locations (declaration or definition) from AST.
   DeclRelationSet Relations =
       DeclRelation::TemplatePattern | DeclRelation::Alias;
-  for (const NamedDecl *D : getDeclAtPosition(AST, CurLoc, Relations)) {
+  for (const NamedDecl *D :
+       getDeclAtPosition(AST, CurLoc, Relations, NodeKind)) {
     // Special case: void foo() ^override: jump to the overridden method.
     if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
       const InheritableAttr *Attr = D->getAttr<OverrideAttr>();
@@ -332,14 +337,26 @@ llvm::StringRef sourcePrefix(SourceLocation Loc, const SourceManager &SM) {
   return Buf.substr(0, D.second);
 }
 
+bool isDependentName(ASTNodeKind NodeKind) {
+  return NodeKind.isSame(ASTNodeKind::getFromNodeKind<OverloadExpr>()) ||
+         NodeKind.isSame(
+             ASTNodeKind::getFromNodeKind<CXXDependentScopeMemberExpr>()) ||
+         NodeKind.isSame(
+             ASTNodeKind::getFromNodeKind<DependentScopeDeclRefExpr>());
+}
+
 } // namespace
 
 std::vector<LocatedSymbol>
 locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
-                      const SymbolIndex *Index,
-                      const std::string &MainFilePath) {
-  // Don't use heuristics if this is a real identifier, or not an identifier.
-  if (Word.ExpandedToken || !Word.LikelyIdentifier || !Index)
+                      const SymbolIndex *Index, const std::string &MainFilePath,
+                      ASTNodeKind NodeKind) {
+  // Don't use heuristics if this is a real identifier, or not an
+  // identifier.
+  // Exception: dependent names, because those may have useful textual
+  // matches that AST-based heuristics cannot find.
+  if ((Word.ExpandedToken && !isDependentName(NodeKind)) ||
+      !Word.LikelyIdentifier || !Index)
     return {};
   // We don't want to handle words in string literals. It'd be nice to whitelist
   // comments instead, but they're not retained in TokenBuffer.
@@ -540,8 +557,9 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
       // expansion.)
       return {*std::move(Macro)};
 
-  auto ASTResults =
-      locateASTReferent(*CurLoc, TouchedIdentifier, AST, *MainFilePath, Index);
+  ASTNodeKind NodeKind;
+  auto ASTResults = locateASTReferent(*CurLoc, TouchedIdentifier, AST,
+                                      *MainFilePath, Index, &NodeKind);
   if (!ASTResults.empty())
     return ASTResults;
 
@@ -554,14 +572,15 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
             findNearbyIdentifier(*Word, AST.getTokens())) {
       if (auto Macro = locateMacroReferent(*NearbyIdent, AST, *MainFilePath))
         return {*std::move(Macro)};
-      ASTResults = locateASTReferent(NearbyIdent->location(), NearbyIdent, AST,
-                                     *MainFilePath, Index);
+      ASTResults =
+          locateASTReferent(NearbyIdent->location(), NearbyIdent, AST,
+                            *MainFilePath, Index, /*NodeKind=*/nullptr);
       if (!ASTResults.empty())
         return ASTResults;
     }
     // No nearby word, or it didn't refer to anything either. Try the index.
     auto TextualResults =
-        locateSymbolTextually(*Word, AST, Index, *MainFilePath);
+        locateSymbolTextually(*Word, AST, Index, *MainFilePath, NodeKind);
     if (!TextualResults.empty())
       return TextualResults;
   }

clang-tools-extra/clangd/XRefs.h

@@ -19,6 +19,7 @@
 #include "SourceCode.h"
 #include "index/Index.h"
 #include "index/SymbolLocation.h"
+#include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Type.h"
 #include "clang/Format/Format.h"
 #include "clang/Index/IndexSymbol.h"
@@ -62,8 +63,8 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
 // (This is for internal use by locateSymbolAt, and is exposed for testing).
 std::vector<LocatedSymbol>
 locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
-                      const SymbolIndex *Index,
-                      const std::string &MainFilePath);
+                      const SymbolIndex *Index, const std::string &MainFilePath,
+                      ASTNodeKind NodeKind);
 
 // Try to find a proximate occurrence of `Word` as an identifier, which can be
 // used to resolve it.

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

@@ -663,16 +663,6 @@ TEST(LocateSymbol, Textual) {
         int myFunction(int);
         // Not triggered for token which survived preprocessing.
         int var = m^yFunction();
-      )cpp",
-      R"cpp(// Dependent type
-        struct Foo {
-          void uniqueMethodName();
-        };
-        template <typename T>
-        void f(T t) {
-          // Not triggered for token which survived preprocessing.
-          t->u^niqueMethodName();
-        }
       )cpp"};
 
   for (const char *Test : Tests) {
@@ -692,8 +682,8 @@ TEST(LocateSymbol, Textual) {
       ADD_FAILURE() << "No word touching point!" << Test;
       continue;
     }
-    auto Results =
-        locateSymbolTextually(*Word, AST, Index.get(), testPath(TU.Filename));
+    auto Results = locateSymbolTextually(*Word, AST, Index.get(),
+                                         testPath(TU.Filename), ASTNodeKind());
 
     if (!WantDecl) {
       EXPECT_THAT(Results, IsEmpty()) << Test;
@@ -778,30 +768,36 @@ TEST(LocateSymbol, Ambiguous) {
                                    Sym("baz", T.range("StaticOverload2"))));
 }
 
-TEST(LocateSymbol, TextualAmbiguous) {
-  auto T = Annotations(R"cpp(
+TEST(LocateSymbol, TextualDependent) {
+  // Put the declarations in the header to make sure we are
+  // finding them via the index heuristic and not the
+  // nearby-ident heuristic.
+  Annotations Header(R"cpp(
         struct Foo {
           void $FooLoc[[uniqueMethodName]]();
         };
         struct Bar {
           void $BarLoc[[uniqueMethodName]]();
         };
-        // Will call u^niqueMethodName() on t.
+        )cpp");
+  Annotations Source(R"cpp(
         template <typename T>
-        void f(T t);
+        void f(T t) {
+          t.u^niqueMethodName();
+        }
       )cpp");
-  auto TU = TestTU::withCode(T.code());
+  TestTU TU;
+  TU.Code = std::string(Source.code());
+  TU.HeaderCode = std::string(Header.code());
   auto AST = TU.build();
   auto Index = TU.index();
-  auto Word = SpelledWord::touching(
-      cantFail(sourceLocationInMainFile(AST.getSourceManager(), T.point())),
-      AST.getTokens(), AST.getLangOpts());
-  ASSERT_TRUE(Word);
-  auto Results =
-      locateSymbolTextually(*Word, AST, Index.get(), testPath(TU.Filename));
-  EXPECT_THAT(Results,
-              UnorderedElementsAre(Sym("uniqueMethodName", T.range("FooLoc")),
-                                   Sym("uniqueMethodName", T.range("BarLoc"))));
+  // Need to use locateSymbolAt() since we are testing an
+  // interaction between locateASTReferent() and
+  // locateSymbolNamedTextuallyAt().
+  auto Results = locateSymbolAt(AST, Source.point(), Index.get());
+  EXPECT_THAT(Results, UnorderedElementsAre(
+                           Sym("uniqueMethodName", Header.range("FooLoc")),
+                           Sym("uniqueMethodName", Header.range("BarLoc"))));
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {