diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- 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,43 @@ 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; +// If a constructor is being called at the location indicated by N, return +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) { + if (const VarDecl *VD = N->ASTNode.get()) { + if (const Expr *Init = VD->getInit()) { + if (const CXXConstructExpr *CE = dyn_cast(Init)) + return CE->getConstructor(); + } } + while ((N = N->Parent)) { + if (N->ASTNode.get() || N->ASTNode.get() || + N->ASTNode.get()) + break; + if (const CXXConstructExpr *CE = N->ASTNode.get()) + return CE->getConstructor(); + } + return nullptr; +} - 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); - } +std::vector +getDeclAtPosition(ParsedAST &AST, SourceLocation Pos, + DeclRelationSet Relations = DeclRelation::TemplatePattern | + DeclRelation::Alias) { + 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); + if (auto *Constructor = findCalledConstructor(N)) { + Decls.push_back(Constructor); } - return true; + 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, @@ -265,6 +226,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 Result; + bool HaveMacro = false; if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) { if (auto Loc = makeLocation(AST.getASTContext(), M->Info->getDefinitionLoc(), *MainFilePath)) { @@ -273,6 +235,7 @@ Macro.PreferredDeclaration = *Loc; Macro.Definition = Loc; Result.push_back(std::move(Macro)); + HaveMacro = true; } } @@ -285,25 +248,31 @@ // Keep track of SymbolID -> index mapping, to fill in index data later. llvm::DenseMap 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(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; + // 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. + for (const Decl *D : getDeclAtPosition(AST, SourceLocationBeg)) { + 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(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. @@ -892,7 +861,10 @@ if (!Decls.empty()) HI = getHoverContents(Decls.front(), Index); } - if (!HI && hasDeducedType(AST, SourceLocationBeg)) { + // Check for hasDeducedType() even if getDeclAtPosition() found something. + // This is needed because it will find something for "operator auto", + // but we want to show the deduced target type there. + if (hasDeducedType(AST, SourceLocationBeg)) { DeducedTypeVisitor V(SourceLocationBeg); V.TraverseAST(AST.getASTContext()); if (!V.DeducedType.isNull()) @@ -928,7 +900,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 +963,11 @@ 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); diff --git a/clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp b/clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp --- a/clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp +++ b/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; } } diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -517,9 +517,10 @@ 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"))); - EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc"))); + EXPECT_THAT(locateSymbolAt(AST, T.point("7")), + ElementsAre(Sym("abc"), Sym("Foo"))); EXPECT_THAT(locateSymbolAt(AST, T.point("8")), - ElementsAre(Sym("Foo"), Sym("abcd"))); + ElementsAre(Sym("abcd"), Sym("Foo"))); EXPECT_THAT(locateSymbolAt(AST, T.point("9")), // First one is class definition, second is the constructor. ElementsAre(Sym("Foo"), Sym("Foo"))); @@ -2190,10 +2191,7 @@ struct Test { StringRef AnnotatedCode; const char *DeducedType; - } Tests[] = { - {"^auto i = 0;", "int"}, - {"^auto f(){ return 1;};", "int"} - }; + } Tests[] = {{"^auto i = 0;", "int"}, {"^auto f(){ return 1;};", "int"}}; for (Test T : Tests) { Annotations File(T.AnnotatedCode); auto AST = TestTU::withCode(File.code()).build();