diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 0dcff2eae05e7..0b2c6c2e721d3 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1040,5 +1040,48 @@ bool isExpandedFromParameterPack(const ParmVarDecl *D) { return getUnderlyingPackType(D) != nullptr; } +bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) { + const auto *FD = FT->getTemplatedDecl(); + const auto NumParams = FD->getNumParams(); + // Check whether its last parameter is a parameter pack... + if (NumParams > 0) { + const auto *LastParam = FD->getParamDecl(NumParams - 1); + if (const auto *PET = dyn_cast(LastParam->getType())) { + // ... of the type T&&... or T... + const auto BaseType = PET->getPattern().getNonReferenceType(); + if (const auto *TTPT = + dyn_cast(BaseType.getTypePtr())) { + // ... whose template parameter comes from the function directly + if (FT->getTemplateParameters()->getDepth() == TTPT->getDepth()) { + return true; + } + } + } + } + return false; +} + +bool ForwardingToConstructorVisitor::VisitCallExpr(CallExpr *E) { + if (auto *FD = E->getDirectCallee()) { + if (auto *PT = FD->getPrimaryTemplate(); + PT && isLikelyForwardingFunction(PT)) { + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + std::move(Visitor.Constructors.begin(), Visitor.Constructors.end(), + std::back_inserter(Constructors)); + } + } + return true; +} + +bool ForwardingToConstructorVisitor::VisitCXXNewExpr(CXXNewExpr *E) { + if (auto *CE = E->getConstructExpr()) { + if (auto *Callee = CE->getConstructor()) { + Constructors.push_back(Callee); + } + } + return true; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 2b83595e5b8e9..3a31dff458421 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -19,6 +19,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/NestedNameSpecifier.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/MacroInfo.h" @@ -253,6 +254,23 @@ resolveForwardingParameters(const FunctionDecl *D, unsigned MaxDepth = 10); /// reference to one (e.g. `Args&...` or `Args&&...`). bool isExpandedFromParameterPack(const ParmVarDecl *D); +/// Heuristic that checks if FT is forwarding a parameter pack to another +/// function (e.g. `make_unique`). +bool isLikelyForwardingFunction(FunctionTemplateDecl *FT); + +class ForwardingToConstructorVisitor + : public RecursiveASTVisitor { +public: + ForwardingToConstructorVisitor() {} + + bool VisitCallExpr(CallExpr *E); + + bool VisitCXXNewExpr(CXXNewExpr *E); + + // Output of this visitor + std::vector Constructors{}; +}; + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index 82fac96360488..bc9f61cb935a9 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -123,6 +123,10 @@ class ParsedAST { return Resolver.get(); } + /// Cache for constructors called through forwarding, e.g. make_unique + llvm::DenseMap> + ForwardingToConstructorCache; + private: ParsedAST(PathRef TUPath, llvm::StringRef Version, std::shared_ptr Preamble, diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 8af9e4649218d..09aaf3290b585 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Preamble.h" +#include "AST.h" #include "CollectMacros.h" #include "Compiler.h" #include "Config.h" @@ -166,27 +167,6 @@ class CppFilePreambleCallbacks : public PreambleCallbacks { collectPragmaMarksCallback(*SourceMgr, Marks)); } - static bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) { - const auto *FD = FT->getTemplatedDecl(); - const auto NumParams = FD->getNumParams(); - // Check whether its last parameter is a parameter pack... - if (NumParams > 0) { - const auto *LastParam = FD->getParamDecl(NumParams - 1); - if (const auto *PET = dyn_cast(LastParam->getType())) { - // ... of the type T&&... or T... - const auto BaseType = PET->getPattern().getNonReferenceType(); - if (const auto *TTPT = - dyn_cast(BaseType.getTypePtr())) { - // ... whose template parameter comes from the function directly - if (FT->getTemplateParameters()->getDepth() == TTPT->getDepth()) { - return true; - } - } - } - } - return false; - } - bool shouldSkipFunctionBody(Decl *D) override { // Usually we don't need to look inside the bodies of header functions // to understand the program. However when forwarding function like diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index ef45acf501612..dd79c59b7b0bf 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -929,12 +929,16 @@ class ReferenceFinder : public index::IndexDataConsumer { } }; - ReferenceFinder(const ParsedAST &AST, + ReferenceFinder(ParsedAST &AST, const llvm::ArrayRef Targets, bool PerToken) : PerToken(PerToken), AST(AST) { - for (const NamedDecl *ND : Targets) + for (const NamedDecl *ND : Targets) { TargetDecls.insert(ND->getCanonicalDecl()); + if (auto *Constructor = llvm::dyn_cast(ND)) { + TargetConstructors.insert(Constructor); + } + } } std::vector take() && { @@ -955,13 +959,51 @@ class ReferenceFinder : public index::IndexDataConsumer { return std::move(References); } + bool forwardsToConstructor(const Decl *D) { + if (TargetConstructors.empty()) { + return false; + } + auto *FD = llvm::dyn_cast(D); + if (FD == nullptr || !FD->isTemplateInstantiation()) { + return false; + } + if (auto *PT = FD->getPrimaryTemplate(); + PT == nullptr || !isLikelyForwardingFunction(PT)) { + return false; + } + std::vector *Constructors = nullptr; + if (auto Entry = AST.ForwardingToConstructorCache.find(FD); + Entry != AST.ForwardingToConstructorCache.end()) { + Constructors = &Entry->getSecond(); + } + if (Constructors == nullptr) { + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + auto Iter = AST.ForwardingToConstructorCache.try_emplace( + FD, std::move(Visitor.Constructors)); + if (Iter.second) { + Constructors = &Iter.first->getSecond(); + } + } + if (Constructors != nullptr) { + for (auto *Constructor : *Constructors) { + if (TargetConstructors.contains(Constructor)) { + return true; + } + } + } + return false; + } + bool handleDeclOccurrence(const Decl *D, index::SymbolRoleSet Roles, llvm::ArrayRef Relations, SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { - if (!TargetDecls.contains(D->getCanonicalDecl())) + if (!TargetDecls.contains(D->getCanonicalDecl()) && + !forwardsToConstructor(ASTNode.OrigD)) { return true; + } const SourceManager &SM = AST.getSourceManager(); if (!isInsideMainFile(Loc, SM)) return true; @@ -1000,8 +1042,10 @@ class ReferenceFinder : public index::IndexDataConsumer { private: bool PerToken; // If true, report 3 references for split ObjC selector names. std::vector References; - const ParsedAST * + ParsedAST * llvm::DenseSet TargetDecls; + // Constructors need special handling since they can be hidden behind forwards + llvm::DenseSet TargetConstructors; }; std::vector diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 39c479b5f4d5b..517932f153798 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -25,6 +25,7 @@ #include "index/SymbolLocation.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclarationName.h" @@ -51,6 +52,7 @@ #include #include #include +#include namespace clang { namespace clangd { @@ -576,6 +578,30 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, return Enclosing; } +std::vector +SymbolCollector::findIndirectConstructors(const Decl *D) { + auto *FD = llvm::dyn_cast(D); + if (FD == nullptr || !FD->isTemplateInstantiation()) { + return {}; + } + if (auto *PT = FD->getPrimaryTemplate(); + PT == nullptr || !isLikelyForwardingFunction(PT)) { + return {}; + } + if (auto Entry = ForwardingToConstructorCache.find(FD); + Entry != ForwardingToConstructorCache.end()) { + return Entry->getSecond(); + } + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + auto Iter = ForwardingToConstructorCache.try_emplace( + FD, std::move(Visitor.Constructors)); + if (Iter.second) { + return Iter.first->getSecond(); + } + return {}; +} + // Always return true to continue indexing. bool SymbolCollector::handleDeclOccurrence( const Decl *D, index::SymbolRoleSet Roles, @@ -666,9 +692,21 @@ bool SymbolCollector::handleDeclOccurrence( auto FileLoc = SM.getFileLoc(Loc); auto FID = SM.getFileID(FileLoc); if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { + auto *Container = getRefContainer(ASTNode.Parent, Opts); addRef(ID, SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, - getRefContainer(ASTNode.Parent, Opts), - isSpelled(FileLoc, *ND)}); + Container, isSpelled(FileLoc, *ND)}); + // Also collect indirect constructor calls like `make_unique` + for (auto *Constructor : findIndirectConstructors(ASTNode.OrigD)) { + if (!shouldCollectSymbol(*Constructor, *ASTCtx, Opts, IsMainFileOnly)) { + continue; + } + if (auto ConstructorID = getSymbolIDCached(Constructor)) { + addRef(ConstructorID, + SymbolRef{FileLoc, FID, Roles, + index::getSymbolInfo(Constructor).Kind, Container, + isSpelled(FileLoc, *Constructor)}); + } + } } } // Don't continue indexing if this is a mere reference. diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index e9eb27fd0f664..54e8ada0249f3 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -159,6 +159,8 @@ class SymbolCollector : public index::IndexDataConsumer { void finish() override; private: + std::vector findIndirectConstructors(const Decl *D); + const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol); void addDefinition(const NamedDecl &, const Symbol &DeclSymbol, @@ -230,6 +232,8 @@ class SymbolCollector : public index::IndexDataConsumer { std::unique_ptr HeaderFileURIs; llvm::DenseMap DeclToIDCache; llvm::DenseMap MacroToIDCache; + llvm::DenseMap> + ForwardingToConstructorCache; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 7ed08d7cce3d3..cdcdaa9e3a18b 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -5,14 +5,15 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// -#include "Annotations.h" #include "AST.h" +#include "Annotations.h" #include "ParsedAST.h" #include "Protocol.h" #include "SourceCode.h" #include "SyncAPI.h" #include "TestFS.h" #include "TestTU.h" +#include "TestWorkspace.h" #include "XRefs.h" #include "index/MemIndex.h" #include "clang/AST/Decl.h" @@ -2713,6 +2714,91 @@ TEST(FindReferences, NoQueryForLocalSymbols) { } } +TEST(FindReferences, ForwardingInAST) { + Annotations Main(R"cpp( + namespace std { + template T &&forward(T &t); + template T *make_unique(Args &&...args) { + return new T(std::forward(args)...); + } + } + + struct Test { + $Constructor[[T^est]](){} + }; + + int main() { + auto a = std::$Caller[[make_unique]](); + } + )cpp"); + TestTU TU; + TU.Code = std::string(Main.code()); + auto AST = TU.build(); + + EXPECT_THAT(findReferences(AST, Main.point(), 0).References, + ElementsAre(rangeIs(Main.range("Constructor")), + rangeIs(Main.range("Caller")))); +} + +TEST(FindReferences, ForwardingInASTTwice) { + Annotations Main(R"cpp( + namespace std { + template T &&forward(T &t); + template T *make_unique(Args &&...args) { + return new T(forward(args)...); + } + template T *make_unique2(Args &&...args) { + return make_unique(forward(args)...); + } + } + + struct Test { + $Constructor[[T^est]](){} + }; + + int main() { + auto a = std::$Caller[[make_unique2]](); + } + )cpp"); + TestTU TU; + TU.Code = std::string(Main.code()); + auto AST = TU.build(); + + EXPECT_THAT(findReferences(AST, Main.point(), 0).References, + ElementsAre(rangeIs(Main.range("Constructor")), + rangeIs(Main.range("Caller")))); +} + +TEST(FindReferences, ForwardingInIndex) { + Annotations Header(R"cpp( + namespace std { + template T &&forward(T &t); + template T *make_unique(Args &&...args) { + return new T(std::forward(args)...); + } + } + struct Test { + [[T^est]](){} + }; + )cpp"); + Annotations Main(R"cpp( + #include "header.hpp" + int main() { + auto a = std::[[make_unique]](); + } + )cpp"); + TestWorkspace TW; + TW.addMainFile("header.hpp", Header.code()); + TW.addMainFile("main.cpp", Main.code()); + auto AST = TW.openFile("header.hpp").value(); + auto Index = TW.index(); + + EXPECT_THAT(findReferences(AST, Header.point(), 0, Index.get(), + /*AddContext*/ true) + .References, + ElementsAre(rangeIs(Header.range()), rangeIs(Main.range()))); +} + TEST(GetNonLocalDeclRefs, All) { struct Case { llvm::StringRef AnnotatedCode;