llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Nathan Ridge (HighCommander4) <details> <summary>Changes</summary> Previously reviewed at https://github.com/llvm/llvm-project/pull/77556 but was backed out due to build failures. --- Patch is 48.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117673.diff 23 Files Affected: - (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+7) - (modified) clang-tools-extra/clangd/ClangdLSPServer.h (+3) - (modified) clang-tools-extra/clangd/ClangdServer.cpp (+9) - (modified) clang-tools-extra/clangd/ClangdServer.h (+4) - (modified) clang-tools-extra/clangd/XRefs.cpp (+59) - (modified) clang-tools-extra/clangd/XRefs.h (+3) - (modified) clang-tools-extra/clangd/index/Index.cpp (+5) - (modified) clang-tools-extra/clangd/index/Index.h (+35) - (modified) clang-tools-extra/clangd/index/MemIndex.cpp (+20) - (modified) clang-tools-extra/clangd/index/MemIndex.h (+4) - (modified) clang-tools-extra/clangd/index/Merge.cpp (+34) - (modified) clang-tools-extra/clangd/index/Merge.h (+3) - (modified) clang-tools-extra/clangd/index/ProjectAware.cpp (+13) - (modified) clang-tools-extra/clangd/index/Ref.h (+3) - (modified) clang-tools-extra/clangd/index/SymbolCollector.cpp (+17-3) - (modified) clang-tools-extra/clangd/index/SymbolCollector.h (+1) - (modified) clang-tools-extra/clangd/index/dex/Dex.cpp (+42) - (modified) clang-tools-extra/clangd/index/dex/Dex.h (+20) - (modified) clang-tools-extra/clangd/test/type-hierarchy-ext.test (+2) - (modified) clang-tools-extra/clangd/test/type-hierarchy.test (+2) - (modified) clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp (+204-73) - (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+6) - (modified) clang-tools-extra/clangd/unittests/RenameTests.cpp (+12) ``````````diff diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 05dd313d0a0d35..1391dcaa0c81a4 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1415,6 +1415,12 @@ void ClangdLSPServer::onInlayHint(const InlayHintsParams &Params, std::move(Reply)); } +void ClangdLSPServer::onCallHierarchyOutgoingCalls( + const CallHierarchyOutgoingCallsParams &Params, + Callback<std::vector<CallHierarchyOutgoingCall>> Reply) { + Server->outgoingCalls(Params.item, std::move(Reply)); +} + void ClangdLSPServer::applyConfiguration( const ConfigurationSettings &Settings) { // Per-file update to the compilation database. @@ -1693,6 +1699,7 @@ void ClangdLSPServer::bindMethods(LSPBinder &Bind, Bind.method("typeHierarchy/subtypes", this, &ClangdLSPServer::onSubTypes); Bind.method("textDocument/prepareCallHierarchy", this, &ClangdLSPServer::onPrepareCallHierarchy); Bind.method("callHierarchy/incomingCalls", this, &ClangdLSPServer::onCallHierarchyIncomingCalls); + Bind.method("callHierarchy/outgoingCalls", this, &ClangdLSPServer::onCallHierarchyOutgoingCalls); Bind.method("textDocument/selectionRange", this, &ClangdLSPServer::onSelectionRange); Bind.method("textDocument/documentLink", this, &ClangdLSPServer::onDocumentLink); Bind.method("textDocument/semanticTokens/full", this, &ClangdLSPServer::onSemanticTokens); diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 0b8e4720f53236..597fd9de7ff688 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -156,6 +156,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks, void onCallHierarchyIncomingCalls( const CallHierarchyIncomingCallsParams &, Callback<std::vector<CallHierarchyIncomingCall>>); + void onCallHierarchyOutgoingCalls( + const CallHierarchyOutgoingCallsParams &, + Callback<std::vector<CallHierarchyOutgoingCall>>); void onClangdInlayHints(const InlayHintsParams &, Callback<llvm::json::Value>); void onInlayHint(const InlayHintsParams &, Callback<std::vector<InlayHint>>); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 9b38be04e7ddd7..63f83bc36f0c69 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -912,6 +912,15 @@ void ClangdServer::inlayHints(PathRef File, std::optional<Range> RestrictRange, WorkScheduler->runWithAST("InlayHints", File, std::move(Action), Transient); } +void ClangdServer::outgoingCalls( + const CallHierarchyItem &Item, + Callback<std::vector<CallHierarchyOutgoingCall>> CB) { + WorkScheduler->run("Outgoing Calls", "", + [CB = std::move(CB), Item, this]() mutable { + CB(clangd::outgoingCalls(Item, Index)); + }); +} + void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { // FIXME: Do nothing for now. This will be used for indexing and potentially // invalidating other caches. diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index a653cdb56b751b..8b6618cf96ccfc 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -292,6 +292,10 @@ class ClangdServer { void incomingCalls(const CallHierarchyItem &Item, Callback<std::vector<CallHierarchyIncomingCall>>); + /// Resolve outgoing calls for a given call hierarchy item. + void outgoingCalls(const CallHierarchyItem &Item, + Callback<std::vector<CallHierarchyOutgoingCall>>); + /// Resolve inlay hints for a given document. void inlayHints(PathRef File, std::optional<Range> RestrictRange, Callback<std::vector<InlayHint>>); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 61fa66180376cd..d237d95b3eb655 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1702,6 +1702,7 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { HierarchyItem HI; HI.name = printName(Ctx, ND); + // FIXME: Populate HI.detail the way we do in symbolToHierarchyItem? HI.kind = SK; HI.range = Range{sourceLocToPosition(SM, DeclRange->getBegin()), sourceLocToPosition(SM, DeclRange->getEnd())}; @@ -1753,6 +1754,7 @@ static std::optional<HierarchyItem> symbolToHierarchyItem(const Symbol &S, } HierarchyItem HI; HI.name = std::string(S.Name); + HI.detail = (S.Scope + S.Name).str(); HI.kind = indexSymbolKindToSymbolKind(S.SymInfo.Kind); HI.selectionRange = Loc->range; // FIXME: Populate 'range' correctly @@ -2319,6 +2321,63 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { return Results; } +std::vector<CallHierarchyOutgoingCall> +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { + std::vector<CallHierarchyOutgoingCall> Results; + if (!Index || Item.data.empty()) + return Results; + auto ID = SymbolID::fromStr(Item.data); + if (!ID) { + elog("outgoingCalls failed to find symbol: {0}", ID.takeError()); + return Results; + } + // In this function, we find outgoing calls based on the index only. + ContainedRefsRequest Request; + Request.ID = *ID; + // Initially store the ranges in a map keyed by SymbolID of the callee. + // This allows us to group different calls to the same function + // into the same CallHierarchyOutgoingCall. + llvm::DenseMap<SymbolID, std::vector<Range>> CallsOut; + // We can populate the ranges based on a refs request only. As we do so, we + // also accumulate the callee IDs into a lookup request. + LookupRequest CallsOutLookup; + Index->containedRefs(Request, [&](const auto &R) { + auto Loc = indexToLSPLocation(R.Location, Item.uri.file()); + if (!Loc) { + elog("outgoingCalls failed to convert location: {0}", Loc.takeError()); + return; + } + auto It = CallsOut.try_emplace(R.Symbol, std::vector<Range>{}).first; + It->second.push_back(Loc->range); + + CallsOutLookup.IDs.insert(R.Symbol); + }); + // Perform the lookup request and combine its results with CallsOut to + // get complete CallHierarchyOutgoingCall objects. + Index->lookup(CallsOutLookup, [&](const Symbol &Callee) { + // The containedRefs request should only return symbols which are + // function-like, i.e. symbols for which references to them can be "calls". + using SK = index::SymbolKind; + auto Kind = Callee.SymInfo.Kind; + assert(Kind == SK::Function || Kind == SK::InstanceMethod || + Kind == SK::ClassMethod || Kind == SK::StaticMethod || + Kind == SK::Constructor || Kind == SK::Destructor || + Kind == SK::ConversionFunction); + + auto It = CallsOut.find(Callee.ID); + assert(It != CallsOut.end()); + if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file())) + Results.push_back( + CallHierarchyOutgoingCall{std::move(*CHI), std::move(It->second)}); + }); + // Sort results by name of the callee. + llvm::sort(Results, [](const CallHierarchyOutgoingCall &A, + const CallHierarchyOutgoingCall &B) { + return A.to.name < B.to.name; + }); + return Results; +} + llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST, const FunctionDecl *FD) { if (!FD->hasBody()) diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index df91dd15303c18..247e52314c3f94 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -150,6 +150,9 @@ prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath); std::vector<CallHierarchyIncomingCall> incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); +std::vector<CallHierarchyOutgoingCall> +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); + /// Returns all decls that are referenced in the \p FD except local symbols. llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST, const FunctionDecl *FD); diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp index 7a0c23287db225..86dc6ed7633449 100644 --- a/clang-tools-extra/clangd/index/Index.cpp +++ b/clang-tools-extra/clangd/index/Index.cpp @@ -66,6 +66,11 @@ bool SwapIndex::refs(const RefsRequest &R, llvm::function_ref<void(const Ref &)> CB) const { return snapshot()->refs(R, CB); } +bool SwapIndex::containedRefs( + const ContainedRefsRequest &R, + llvm::function_ref<void(const ContainedRefsResult &)> CB) const { + return snapshot()->containedRefs(R, CB); +} void SwapIndex::relations( const RelationsRequest &R, llvm::function_ref<void(const SymbolID &, const Symbol &)> CB) const { diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index 047ce08e93e3ab..a193b1a191216a 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -77,6 +77,19 @@ struct RefsRequest { bool WantContainer = false; }; +struct ContainedRefsRequest { + /// Note that RefKind::Call just restricts the matched SymbolKind to + /// functions, not the form of the reference (e.g. address-of-function, + /// which can indicate an indirect call, should still be caught). + static const RefKind SupportedRefKinds = RefKind::Call; + + SymbolID ID; + /// If set, limit the number of refers returned from the index. The index may + /// choose to return less than this, e.g. it tries to avoid returning stale + /// results. + std::optional<uint32_t> Limit; +}; + struct RelationsRequest { llvm::DenseSet<SymbolID> Subjects; RelationKind Predicate; @@ -84,6 +97,14 @@ struct RelationsRequest { std::optional<uint32_t> Limit; }; +struct ContainedRefsResult { + /// The source location where the symbol is named. + SymbolLocation Location; + RefKind Kind = RefKind::Unknown; + /// The ID of the symbol which is referred to + SymbolID Symbol; +}; + /// Describes what data is covered by an index. /// /// Indexes may contain symbols but not references from a file, etc. @@ -141,6 +162,17 @@ class SymbolIndex { virtual bool refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const = 0; + /// Find all symbols that are referenced by a symbol and apply + /// \p Callback on each result. + /// + /// Results should be returned in arbitrary order. + /// The returned result must be deep-copied if it's used outside Callback. + /// + /// Returns true if there will be more results (limited by Req.Limit); + virtual bool containedRefs( + const ContainedRefsRequest &Req, + llvm::function_ref<void(const ContainedRefsResult &)> Callback) const = 0; + /// Finds all relations (S, P, O) stored in the index such that S is among /// Req.Subjects and P is Req.Predicate, and invokes \p Callback for (S, O) in /// each. @@ -175,6 +207,9 @@ class SwapIndex : public SymbolIndex { llvm::function_ref<void(const Symbol &)>) const override; bool refs(const RefsRequest &, llvm::function_ref<void(const Ref &)>) const override; + bool containedRefs( + const ContainedRefsRequest &, + llvm::function_ref<void(const ContainedRefsResult &)>) const override; void relations(const RelationsRequest &, llvm::function_ref<void(const SymbolID &, const Symbol &)>) const override; diff --git a/clang-tools-extra/clangd/index/MemIndex.cpp b/clang-tools-extra/clangd/index/MemIndex.cpp index 2665d46b97d839..9c9d3942bdee63 100644 --- a/clang-tools-extra/clangd/index/MemIndex.cpp +++ b/clang-tools-extra/clangd/index/MemIndex.cpp @@ -9,6 +9,7 @@ #include "MemIndex.h" #include "FuzzyMatch.h" #include "Quality.h" +#include "index/Index.h" #include "support/Trace.h" namespace clang { @@ -85,6 +86,25 @@ bool MemIndex::refs(const RefsRequest &Req, return false; // We reported all refs. } +bool MemIndex::containedRefs( + const ContainedRefsRequest &Req, + llvm::function_ref<void(const ContainedRefsResult &)> Callback) const { + trace::Span Tracer("MemIndex refersTo"); + uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max()); + for (const auto &Pair : Refs) { + for (const auto &R : Pair.second) { + if (!static_cast<int>(ContainedRefsRequest::SupportedRefKinds & R.Kind) || + Req.ID != R.Container) + continue; + if (Remaining == 0) + return true; // More refs were available. + --Remaining; + Callback({R.Location, R.Kind, Pair.first}); + } + } + return false; // We reported all refs. +} + void MemIndex::relations( const RelationsRequest &Req, llvm::function_ref<void(const SymbolID &, const Symbol &)> Callback) const { diff --git a/clang-tools-extra/clangd/index/MemIndex.h b/clang-tools-extra/clangd/index/MemIndex.h index fba2c1a7120a2b..8f390c5028dc4d 100644 --- a/clang-tools-extra/clangd/index/MemIndex.h +++ b/clang-tools-extra/clangd/index/MemIndex.h @@ -72,6 +72,10 @@ class MemIndex : public SymbolIndex { bool refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const override; + bool containedRefs(const ContainedRefsRequest &Req, + llvm::function_ref<void(const ContainedRefsResult &)> + Callback) const override; + void relations(const RelationsRequest &Req, llvm::function_ref<void(const SymbolID &, const Symbol &)> Callback) const override; diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index 8221d4b1f44405..aecca38a885b66 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -155,6 +155,40 @@ bool MergedIndex::refs(const RefsRequest &Req, return More || StaticHadMore; } +bool MergedIndex::containedRefs( + const ContainedRefsRequest &Req, + llvm::function_ref<void(const ContainedRefsResult &)> Callback) const { + trace::Span Tracer("MergedIndex refersTo"); + bool More = false; + uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max()); + // We don't want duplicated refs from the static/dynamic indexes, + // and we can't reliably deduplicate them because offsets may differ slightly. + // We consider the dynamic index authoritative and report all its refs, + // and only report static index refs from other files. + More |= Dynamic->containedRefs(Req, [&](const auto &O) { + Callback(O); + assert(Remaining != 0); + --Remaining; + }); + if (Remaining == 0 && More) + return More; + auto DynamicContainsFile = Dynamic->indexedFiles(); + // We return less than Req.Limit if static index returns more refs for dirty + // files. + bool StaticHadMore = Static->containedRefs(Req, [&](const auto &O) { + if ((DynamicContainsFile(O.Location.FileURI) & IndexContents::References) != + IndexContents::None) + return; // ignore refs that have been seen from dynamic index. + if (Remaining == 0) { + More = true; + return; + } + --Remaining; + Callback(O); + }); + return More || StaticHadMore; +} + llvm::unique_function<IndexContents(llvm::StringRef) const> MergedIndex::indexedFiles() const { return [DynamicContainsFile{Dynamic->indexedFiles()}, diff --git a/clang-tools-extra/clangd/index/Merge.h b/clang-tools-extra/clangd/index/Merge.h index b8a562b0df5d92..7441be6e57e854 100644 --- a/clang-tools-extra/clangd/index/Merge.h +++ b/clang-tools-extra/clangd/index/Merge.h @@ -38,6 +38,9 @@ class MergedIndex : public SymbolIndex { llvm::function_ref<void(const Symbol &)>) const override; bool refs(const RefsRequest &, llvm::function_ref<void(const Ref &)>) const override; + bool containedRefs( + const ContainedRefsRequest &, + llvm::function_ref<void(const ContainedRefsResult &)>) const override; void relations(const RelationsRequest &, llvm::function_ref<void(const SymbolID &, const Symbol &)>) const override; diff --git a/clang-tools-extra/clangd/index/ProjectAware.cpp b/clang-tools-extra/clangd/index/ProjectAware.cpp index 2c6f8273b35d0e..9836f0130362a0 100644 --- a/clang-tools-extra/clangd/index/ProjectAware.cpp +++ b/clang-tools-extra/clangd/index/ProjectAware.cpp @@ -35,6 +35,10 @@ class ProjectAwareIndex : public SymbolIndex { /// Query all indexes while prioritizing the associated one (if any). bool refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const override; + /// Query all indexes while prioritizing the associated one (if any). + bool containedRefs(const ContainedRefsRequest &Req, + llvm::function_ref<void(const ContainedRefsResult &)> + Callback) const override; /// Queries only the associates index when Req.RestrictForCodeCompletion is /// set, otherwise queries all. @@ -94,6 +98,15 @@ bool ProjectAwareIndex::refs( return false; } +bool ProjectAwareIndex::containedRefs( + const ContainedRefsRequest &Req, + llvm::function_ref<void(const ContainedRefsResult &)> Callback) const { + trace::Span Tracer("ProjectAwareIndex::refersTo"); + if (auto *Idx = getIndex()) + return Idx->containedRefs(Req, Callback); + return false; +} + bool ProjectAwareIndex::fuzzyFind( const FuzzyFindRequest &Req, llvm::function_ref<void(const Symbol &)> Callback) const { diff --git a/clang-tools-extra/clangd/index/Ref.h b/clang-tools-extra/clangd/index/Ref.h index 6e383e2ade3d25..870f77f56e6cb3 100644 --- a/clang-tools-extra/clangd/index/Ref.h +++ b/clang-tools-extra/clangd/index/Ref.h @@ -63,6 +63,9 @@ enum class RefKind : uint8_t { // ^ this references Foo, but does not explicitly spell out its name // }; Spelled = 1 << 3, + // A reference which is a call. Used as a filter for which references + // to store in data structures used for computing outgoing calls. + Call = 1 << 4, All = Declaration | Definition | Reference | Spelled, }; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 91ae9d3003a971..81125dbb1aeafc 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -18,6 +18,7 @@ #include "clang-include-cleaner/Record.h" #include "clang-include-cleaner/Types.h" #include "index/CanonicalIncludes.h" +#include "index/Ref.h" #include "index/Relation.h" #include "index/Symbol.h" #include "index/SymbolID.h" @@ -660,7 +661,7 @@ bool SymbolCollector::handleDeclOccurrence( auto FileLoc = SM.getFileLoc(Loc); auto FID = SM.getFileID(FileLoc); if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { - addRef(ID, SymbolRef{FileLoc, FID, Roles, + addRef(ID, SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, getRefContainer(ASTNode.Parent, Opts), isSpelled(FileLoc, *ND)}); } @@ -774,8 +775,10 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name, // FIXME: Populate container information for macro references. // FIXME: All MacroRefs are marked as Spelled now, but this should be // checked. - addRef(ID, SymbolRef{Loc, SM.getFileID(Loc), Roles, /*Container=*/nullptr, - /*Spelled=*/true}); + addRef(ID, + SymbolRef{Loc, SM.getFileID(Loc), Roles, index::SymbolKind::Macro, + /*Container=*/nullptr, + /*Spelled=*/true}); } // Collect symbols. @@ -1166,6 +1169,14 @@ bool SymbolCollector::shouldIndexFile(FileID FID) { return I.first->second; } +static bool refIsCall(index::SymbolKind Kind) { + using SK = index::SymbolKind; + return Kind == SK::Function || Kind == SK::InstanceMethod || + Kind == SK::ClassMethod || Kind == SK::StaticMethod || + Kind == SK::Constructor || Kind == SK::Destructor || + ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/117673 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
