nridge updated this revision to Diff 226561.
nridge added a comment.
Updated not to use the beginning-of-identifier location as input to
SelectionTree
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69237/new/
https://reviews.llvm.org/D69237
Files:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -490,7 +490,7 @@
struct Foo {
Foo();
Foo(Foo&&);
- Foo(const char*);
+ $ConstructorLoc[[Foo]](const char*);
};
Foo f();
@@ -507,6 +507,8 @@
Foo ab$7^c;
Foo ab$8^cd("asdf");
Foo foox = Fo$9^o("asdf");
+ Foo abcde$10^("asdf");
+ Foo foox2 = Foo$11^("asdf");
}
)cpp");
auto AST = TestTU::withCode(T.code()).build();
@@ -518,11 +520,17 @@
EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
- EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
- ElementsAre(Sym("Foo"), Sym("abcd")));
- EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
- // First one is class definition, second is the constructor.
- ElementsAre(Sym("Foo"), Sym("Foo")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("8")), ElementsAre(Sym("abcd")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo")));
+
+ // Test constructor targeting. This currently works if the cursor
+ // is just before the opening parenthesis.
+ // FIXME: Ideally, we would like this to work for the whole
+ // identifier, and in cases where there is no parenthesis (e.g. case #7).
+ EXPECT_THAT(locateSymbolAt(AST, T.point("10")),
+ ElementsAre(Sym("Foo", T.range("ConstructorLoc"))));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
+ ElementsAre(Sym("Foo", T.range("ConstructorLoc"))));
}
TEST(LocateSymbol, TemplateTypedefs) {
@@ -2192,7 +2200,7 @@
const char *DeducedType;
} Tests[] = {
{"^auto i = 0;", "int"},
- {"^auto f(){ return 1;};", "int"}
+ {"^auto f(){ return 1;};", "int"},
};
for (Test T : Tests) {
Annotations File(T.AnnotatedCode);
Index: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
@@ -24,7 +24,7 @@
namespace clangd {
namespace {
-using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
auto CreateExpectedSymbolDetails = [](const std::string &name,
const std::string &container,
@@ -329,7 +329,7 @@
auto AST = TestTU::withCode(TestInput.code()).build();
EXPECT_THAT(getSymbolInfo(AST, TestInput.point()),
- ElementsAreArray(T.second))
+ UnorderedElementsAreArray(T.second))
<< T.first;
}
}
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
#include "Logger.h"
#include "ParsedAST.h"
#include "Protocol.h"
+#include "Selection.h"
#include "SourceCode.h"
#include "URI.h"
#include "index/Index.h"
@@ -133,83 +134,18 @@
return Merged.CanonicalDeclaration;
}
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
- llvm::DenseSet<const Decl *> Decls;
- const SourceLocation &SearchedLocation;
-
-public:
- DeclarationFinder(const SourceLocation &SearchedLocation)
- : SearchedLocation(SearchedLocation) {}
-
- // The results are sorted by declaration location.
- std::vector<const Decl *> getFoundDecls() const {
- std::vector<const Decl *> Result;
- for (const Decl *D : Decls)
- Result.push_back(D);
-
- llvm::sort(Result, [](const Decl *L, const Decl *R) {
- return L->getBeginLoc() < R->getBeginLoc();
- });
- return Result;
+std::vector<const Decl *> getDeclAtPosition(ParsedAST &AST, SourceLocation Pos,
+ DeclRelationSet Relations) {
+ FileID FID;
+ unsigned Offset;
+ std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+ SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+ std::vector<const Decl *> Result;
+ if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+ auto Decls = targetDecl(N->ASTNode, Relations);
+ Result.assign(Decls.begin(), Decls.end());
}
-
- bool
- handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
- llvm::ArrayRef<index::SymbolRelation> Relations,
- SourceLocation Loc,
- index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
- // Skip non-semantic references.
- if (Roles & static_cast<unsigned>(index::SymbolRole::NameReference))
- return true;
-
- if (Loc == SearchedLocation) {
- auto IsImplicitExpr = [](const Expr *E) {
- if (!E)
- return false;
- // We assume that a constructor expression is implict (was inserted by
- // clang) if it has an invalid paren/brace location, since such
- // experssion is impossible to write down.
- if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(E))
- return CtorExpr->getParenOrBraceRange().isInvalid();
- // Ignore implicit conversion-operator AST node.
- if (const auto *ME = dyn_cast<MemberExpr>(E)) {
- if (isa<CXXConversionDecl>(ME->getMemberDecl()))
- return ME->getMemberLoc().isInvalid();
- }
- return isa<ImplicitCastExpr>(E);
- };
-
- if (IsImplicitExpr(ASTNode.OrigE))
- return true;
- // Find and add definition declarations (for GoToDefinition).
- // We don't use parameter `D`, as Parameter `D` is the canonical
- // declaration, which is the first declaration of a redeclarable
- // declaration, and it could be a forward declaration.
- if (const auto *Def = getDefinition(D)) {
- Decls.insert(Def);
- } else {
- // Couldn't find a definition, fall back to use `D`.
- Decls.insert(D);
- }
- }
- return true;
- }
-};
-
-std::vector<const Decl *> getDeclAtPosition(ParsedAST &AST,
- SourceLocation Pos) {
- DeclarationFinder Finder(Pos);
- index::IndexingOptions IndexOpts;
- IndexOpts.SystemSymbolFilter =
- index::IndexingOptions::SystemSymbolFilterKind::All;
- IndexOpts.IndexFunctionLocals = true;
- IndexOpts.IndexParametersInDeclarations = true;
- IndexOpts.IndexTemplateParameters = true;
- indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
- AST.getLocalTopLevelDecls(), Finder, IndexOpts);
-
- return Finder.getFoundDecls();
+ return Result;
}
llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
@@ -265,6 +201,7 @@
// Macros are simple: there's no declaration/definition distinction.
// As a consequence, there's no need to look them up in the index either.
std::vector<LocatedSymbol> Result;
+ bool HaveMacro = false;
if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) {
if (auto Loc = makeLocation(AST.getASTContext(),
M->Info->getDefinitionLoc(), *MainFilePath)) {
@@ -273,6 +210,7 @@
Macro.PreferredDeclaration = *Loc;
Macro.Definition = Loc;
Result.push_back(std::move(Macro));
+ HaveMacro = true;
}
}
@@ -285,25 +223,43 @@
// Keep track of SymbolID -> index mapping, to fill in index data later.
llvm::DenseMap<SymbolID, size_t> ResultIndex;
- // Emit all symbol locations (declaration or definition) from AST.
- for (const Decl *D : getDeclAtPosition(AST, SourceLocationBeg)) {
- auto Loc =
- makeLocation(AST.getASTContext(), spellingLocIfSpelled(findName(D), SM),
- *MainFilePath);
- if (!Loc)
- continue;
-
- Result.emplace_back();
- if (auto *ND = dyn_cast<NamedDecl>(D))
- Result.back().Name = printName(AST.getASTContext(), *ND);
- Result.back().PreferredDeclaration = *Loc;
- // DeclInfo.D is always a definition if possible, so this check works.
- if (getDefinition(D) == D)
- Result.back().Definition = *Loc;
-
- // Record SymbolID for index lookup later.
- if (auto ID = getSymbolID(D))
- ResultIndex[*ID] = Result.size() - 1;
+ // For getDeclAtPosition(), use the actual input location, not the location
+ // of the beginning of the identifier, because SelectionTree may select
+ // different nodes for the two locations.
+ SourceLocation SourceLoc;
+ if (auto L = sourceLocationInMainFile(SM, Pos)) {
+ SourceLoc = *L;
+ } else {
+ log("locateSymbolAt: {0}", L.takeError());
+ }
+
+ // If we found a macro, don't bother calling getDeclAtPosition(). It would
+ // just return declarations referenced from the macro's expansion.
+ if (!HaveMacro) {
+ // Emit all symbol locations (declaration or definition) from AST.
+ DeclRelationSet Relations =
+ DeclRelation::TemplatePattern | DeclRelation::Alias;
+ for (const Decl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
+ const Decl *Def = getDefinition(D);
+ const Decl *Preferred = Def ? Def : D;
+ auto Loc = makeLocation(AST.getASTContext(),
+ spellingLocIfSpelled(findName(Preferred), SM),
+ *MainFilePath);
+ if (!Loc)
+ continue;
+
+ Result.emplace_back();
+ if (auto *ND = dyn_cast<NamedDecl>(Preferred))
+ Result.back().Name = printName(AST.getASTContext(), *ND);
+ Result.back().PreferredDeclaration = *Loc;
+ // Preferred is always a definition if possible, so this check works.
+ if (Def == Preferred)
+ Result.back().Definition = *Loc;
+
+ // Record SymbolID for index lookup later.
+ if (auto ID = getSymbolID(Preferred))
+ ResultIndex[*ID] = Result.size() - 1;
+ }
}
// Now query the index for all Symbol IDs we found in the AST.
@@ -413,11 +369,14 @@
Position Pos) {
const SourceManager &SM = AST.getSourceManager();
// FIXME: show references to macro within file?
- auto References =
- findRefs(getDeclAtPosition(
- AST, SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
- Pos, SM, AST.getASTContext().getLangOpts()))),
- AST);
+ DeclRelationSet Relations =
+ DeclRelation::TemplatePattern | DeclRelation::Alias;
+ auto References = findRefs(
+ getDeclAtPosition(AST,
+ SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
+ Pos, SM, AST.getASTContext().getLangOpts())),
+ Relations),
+ AST);
// FIXME: we may get multiple DocumentHighlights with the same location and
// different kinds, deduplicate them.
@@ -885,19 +844,25 @@
SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
- if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) {
- HI = getHoverContents(*M, AST);
- } else {
- auto Decls = getDeclAtPosition(AST, SourceLocationBeg);
- if (!Decls.empty())
- HI = getHoverContents(Decls.front(), Index);
- }
- if (!HI && hasDeducedType(AST, SourceLocationBeg)) {
+ if (hasDeducedType(AST, SourceLocationBeg)) {
DeducedTypeVisitor V(SourceLocationBeg);
V.TraverseAST(AST.getASTContext());
if (!V.DeducedType.isNull())
HI = getHoverContents(V.DeducedType, V.D, AST.getASTContext(), Index);
}
+
+ if (!HI) {
+ if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) {
+ HI = getHoverContents(*M, AST);
+ } else {
+ DeclRelationSet Relations =
+ DeclRelation::TemplatePattern | DeclRelation::Alias;
+ auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);
+ if (!Decls.empty())
+ HI = getHoverContents(Decls.front(), Index);
+ }
+ }
+
if (!HI)
return llvm::None;
@@ -928,7 +893,11 @@
auto Loc = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
// TODO: should we handle macros, too?
- auto Decls = getDeclAtPosition(AST, Loc);
+ // We also want the targets of using-decls, so we include
+ // DeclRelation::Underlying.
+ DeclRelationSet Relations = DeclRelation::TemplatePattern |
+ DeclRelation::Alias | DeclRelation::Underlying;
+ auto Decls = getDeclAtPosition(AST, Loc, Relations);
// We traverse the AST to find references in the main file.
auto MainFileRefs = findRefs(Decls, AST);
@@ -987,7 +956,11 @@
std::vector<SymbolDetails> Results;
- for (const Decl *D : getDeclAtPosition(AST, Loc)) {
+ // We also want the targets of using-decls, so we include
+ // DeclRelation::Underlying.
+ DeclRelationSet Relations = DeclRelation::TemplatePattern |
+ DeclRelation::Alias | DeclRelation::Underlying;
+ for (const Decl *D : getDeclAtPosition(AST, Loc, Relations)) {
SymbolDetails NewSymbol;
if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
std::string QName = printQualifiedName(*ND);
@@ -1156,7 +1129,9 @@
const SourceManager &SM = AST.getSourceManager();
SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
- auto Decls = getDeclAtPosition(AST, SourceLocationBeg);
+ DeclRelationSet Relations =
+ DeclRelation::TemplatePattern | DeclRelation::Alias;
+ auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);
if (Decls.empty())
return nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
