diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 3ee04f03179512..0cfa53558114f3 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/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 @@ SymbolLocation getPreferredLocation(const Location &ASTLoc, return Merged.CanonicalDeclaration; } -/// Finds declarations locations that a given source location refers to. -class DeclarationFinder : public index::IndexDataConsumer { - llvm::DenseSet Decls; - const SourceLocation &SearchedLocation; - -public: - DeclarationFinder(const SourceLocation &SearchedLocation) - : SearchedLocation(SearchedLocation) {} - - // The results are sorted by declaration location. - std::vector getFoundDecls() const { - std::vector 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; - } - - bool - handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles, - llvm::ArrayRef Relations, - SourceLocation Loc, - index::IndexDataConsumer::ASTNodeInfo ASTNode) override { - // Skip non-semantic references. - if (Roles & static_cast(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(E)) - return CtorExpr->getParenOrBraceRange().isInvalid(); - // Ignore implicit conversion-operator AST node. - if (const auto *ME = dyn_cast(E)) { - if (isa(ME->getMemberDecl())) - return ME->getMemberLoc().isInvalid(); - } - return isa(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 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 Result; + if (const SelectionTree::Node *N = Selection.commonAncestor()) { + auto Decls = targetDecl(N->ASTNode, Relations); + Result.assign(Decls.begin(), Decls.end()); } -}; - -std::vector 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 makeLocation(ASTContext &AST, SourceLocation TokLoc, @@ -258,14 +194,13 @@ std::vector locateSymbolAt(ParsedAST &AST, Position Pos, } } - SourceLocation SourceLocationBeg = - SM.getMacroArgExpandedLocation(getBeginningOfIdentifier( - Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts())); - // Macros are simple: there's no declaration/definition distinction. // As a consequence, there's no need to look them up in the index either. + SourceLocation MaybeMacroLocation = + SM.getMacroArgExpandedLocation(getBeginningOfIdentifier( + Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts())); std::vector Result; - if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) { + if (auto M = locateMacroAt(MaybeMacroLocation, AST.getPreprocessor())) { if (auto Loc = makeLocation(AST.getASTContext(), M->Info->getDefinitionLoc(), *MainFilePath)) { LocatedSymbol Macro; @@ -273,6 +208,11 @@ std::vector locateSymbolAt(ParsedAST &AST, Position Pos, Macro.PreferredDeclaration = *Loc; Macro.Definition = Loc; Result.push_back(std::move(Macro)); + + // Don't look at the AST or index if we have a macro result. + // (We'd just return declarations referenced from the macro's + // expansion.) + return Result; } } @@ -285,24 +225,37 @@ std::vector locateSymbolAt(ParsedAST &AST, Position Pos, // Keep track of SymbolID -> index mapping, to fill in index data later. llvm::DenseMap ResultIndex; + SourceLocation SourceLoc; + if (auto L = sourceLocationInMainFile(SM, Pos)) { + SourceLoc = *L; + } else { + elog("locateSymbolAt failed to convert position to source location: {0}", + L.takeError()); + return Result; + } + // 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); + 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(D)) + if (auto *ND = dyn_cast(Preferred)) 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) + // 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(D)) + if (auto ID = getSymbolID(Preferred)) ResultIndex[*ID] = Result.size() - 1; } @@ -413,11 +366,14 @@ std::vector findDocumentHighlights(ParsedAST &AST, 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. @@ -887,19 +843,25 @@ llvm::Optional getHover(ParsedAST &AST, Position Pos, 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; @@ -930,7 +892,11 @@ std::vector findReferences(ParsedAST &AST, Position Pos, auto Loc = SM.getMacroArgExpandedLocation( getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts())); // TODO: should we handle macros, too? - auto Decls = getDeclAtPosition(AST, Loc); + // We also show references to 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); @@ -989,7 +955,11 @@ std::vector getSymbolInfo(ParsedAST &AST, Position Pos) { std::vector 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(D)) { std::string QName = printQualifiedName(*ND); @@ -1158,7 +1128,9 @@ const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) { 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::Underlying; + auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations); if (Decls.empty()) return nullptr; diff --git a/clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp b/clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp index 1d009f6339bc6c..1f9976b51ed00b 100644 --- a/clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp @@ -24,7 +24,7 @@ namespace clang { namespace clangd { namespace { -using ::testing::ElementsAreArray; +using ::testing::UnorderedElementsAreArray; auto CreateExpectedSymbolDetails = [](const std::string &name, const std::string &container, @@ -329,7 +329,7 @@ TEST(SymbolInfoTests, All) { auto AST = TestTU::withCode(TestInput.code()).build(); EXPECT_THAT(getSymbolInfo(AST, TestInput.point()), - ElementsAreArray(T.second)) + UnorderedElementsAreArray(T.second)) << T.first; } } diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp index 252c421ef615f2..0247e1e9fed754 100644 --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -60,6 +60,8 @@ struct Ch^ild2 { int c; }; +using A^lias = Child2; + int main() { Ch^ild2 ch^ild2; ch^ild2.c = 1; diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index c8b0055394660a..9330c6d366dd51 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -490,7 +490,7 @@ TEST(LocateSymbol, Ambiguous) { struct Foo { Foo(); Foo(Foo&&); - Foo(const char*); + $ConstructorLoc[[Foo]](const char*); }; Foo f(); @@ -507,6 +507,8 @@ TEST(LocateSymbol, Ambiguous) { 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(); @@ -517,12 +519,16 @@ TEST(LocateSymbol, Ambiguous) { EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g"))); EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f"))); EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str"))); + // FIXME: Target the constructor as well. 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"))); + // FIXME: Target the constructor as well. + EXPECT_THAT(locateSymbolAt(AST, T.point("8")), ElementsAre(Sym("abcd"))); + // FIXME: Target the constructor as well. + EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo"))); + 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 +2198,7 @@ TEST(GetDeducedType, KwAutoExpansion) { 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);