[clang-tools-extra] 230cae8 - [clangd] Enable textual fallback for go-to-definition on dependent names
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 25 21:38:57 PDT 2020
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) {
More information about the cfe-commits
mailing list