Author: Nathan Ridge Date: 2020-04-26T00:38:38-04:00 New Revision: 230cae89db3f8619e2b5383ae797462434f69d50
URL: https://github.com/llvm/llvm-project/commit/230cae89db3f8619e2b5383ae797462434f69d50 DIFF: https://github.com/llvm/llvm-project/commit/230cae89db3f8619e2b5383ae797462434f69d50.diff LOG: [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 Added: Modified: clang-tools-extra/clangd/XRefs.cpp clang-tools-extra/clangd/XRefs.h clang-tools-extra/clangd/unittests/XRefsTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index d17fa52bd82c..7eb3c4fcc7a3 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/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; } diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index af78ec780c5a..4645d32c763c 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/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. diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 027939e15f77..09304ecdaf73 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
