From 910a890e96e231d19a5833c02c280a03a2283271 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Fri, 28 Nov 2025 14:01:25 +0100 Subject: [PATCH 01/29] Fill symbol tags for the response TypeHierarchyItem of prepareTypeHierarchy. --- clang-tools-extra/clangd/FindSymbols.cpp | 14 +++++++++++++ clang-tools-extra/clangd/FindSymbols.h | 3 +++ clang-tools-extra/clangd/Protocol.cpp | 2 ++ clang-tools-extra/clangd/XRefs.cpp | 21 ++++++++++--------- .../clangd/test/type-hierarchy.test | 9 ++++++++ 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 147dd38db8a8a..40e31af5b8ce3 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -228,6 +228,20 @@ std::vector getSymbolTags(const NamedDecl &ND) { return Tags; } +std::vector getSymbolTags(const Symbol &S) { + std::vector Tags; + + if (S.Flags & Symbol::Deprecated) + Tags.push_back(SymbolTag::Deprecated); + + if (S.Definition) + Tags.push_back(SymbolTag::Definition); + else + Tags.push_back(SymbolTag::Declaration); + + return Tags; +} + namespace { using ScoredSymbolInfo = std::pair; struct ScoredSymbolGreater { diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 97b99af4f35e6..4847ea359e9ef 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -69,6 +69,9 @@ SymbolTags computeSymbolTags(const NamedDecl &ND); /// \p ND The declaration to get tags for. std::vector getSymbolTags(const NamedDecl &ND); +/// Returns the symbol tags for an index `Symbol`. +std::vector getSymbolTags(const Symbol &S); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 793db7b052990..81ee6c9287ee1 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -1445,6 +1445,8 @@ llvm::json::Value toJSON(const TypeHierarchyItem &I) { if (I.detail) Result["detail"] = I.detail; + if(!I.tags.empty()) + Result["tags"] = I.tags; return std::move(Result); } diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 5b9ba1baa0705..5a9a9f3eab770 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1872,6 +1872,7 @@ static std::optional symbolToHierarchyItem(const Symbol &S, HI.name = std::string(S.Name); HI.detail = (S.Scope + S.Name).str(); HI.kind = indexSymbolKindToSymbolKind(S.SymInfo); + HI.tags = getSymbolTags(S); HI.selectionRange = Loc->range; // FIXME: Populate 'range' correctly // (https://github.com/clangd/clangd/issues/59). @@ -2136,15 +2137,15 @@ static QualType typeForNode(const ASTContext &Ctx, const HeuristicResolver *H, return QualType(); } -// Given a type targeted by the cursor, return one or more types that are more interesting -// to target. -static void unwrapFindType( - QualType T, const HeuristicResolver* H, llvm::SmallVector& Out) { +// Given a type targeted by the cursor, return one or more types that are more +// interesting to target. +static void unwrapFindType(QualType T, const HeuristicResolver *H, + llvm::SmallVector &Out) { if (T.isNull()) return; // If there's a specific type alias, point at that rather than unwrapping. - if (const auto* TDT = T->getAs()) + if (const auto *TDT = T->getAs()) return Out.push_back(QualType(TDT, 0)); // Pointers etc => pointee type. @@ -2178,8 +2179,8 @@ static void unwrapFindType( } // Convenience overload, to allow calling this without the out-parameter -static llvm::SmallVector unwrapFindType( - QualType T, const HeuristicResolver* H) { +static llvm::SmallVector unwrapFindType(QualType T, + const HeuristicResolver *H) { llvm::SmallVector Result; unwrapFindType(T, H, Result); return Result; @@ -2201,9 +2202,9 @@ std::vector findType(ParsedAST &AST, Position Pos, std::vector LocatedSymbols; // NOTE: unwrapFindType might return duplicates for something like - // unique_ptr>. Let's *not* remove them, because it gives you some - // information about the type you may have not known before - // (since unique_ptr> != unique_ptr). + // unique_ptr>. Let's *not* remove them, because it gives you + // some information about the type you may have not known before (since + // unique_ptr> != unique_ptr). for (const QualType &Type : unwrapFindType( typeForNode(AST.getASTContext(), AST.getHeuristicResolver(), N), AST.getHeuristicResolver())) diff --git a/clang-tools-extra/clangd/test/type-hierarchy.test b/clang-tools-extra/clangd/test/type-hierarchy.test index a5f13ab13d0b3..918a37a74098c 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy.test +++ b/clang-tools-extra/clangd/test/type-hierarchy.test @@ -44,6 +44,9 @@ # CHECK-NEXT: "line": 2 # CHECK-NEXT: } # CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 19 +# CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" # CHECK-NEXT: } # CHECK-NEXT: ] @@ -85,6 +88,9 @@ # CHECK-NEXT: "line": 1 # CHECK-NEXT: } # CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 19 +# CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" # CHECK-NEXT: } # CHECK-NEXT: ] @@ -136,6 +142,9 @@ # CHECK-NEXT: "line": 3 # CHECK-NEXT: } # CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 19 +# CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" # CHECK-NEXT: } # CHECK-NEXT: ] From 01a022e6ea330ec0b3bcbae3997644cac733cab1 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 1 Dec 2025 11:34:51 +0100 Subject: [PATCH 02/29] Fill symbol tags into the object SymbolInformation, and on getting workspace symbols. --- clang-tools-extra/clangd/FindSymbols.cpp | 1 + clang-tools-extra/clangd/Protocol.cpp | 2 ++ clang-tools-extra/clangd/test/symbols.test | 5 ++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 40e31af5b8ce3..175645d4d7c39 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -373,6 +373,7 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, Info.score = Relevance.NameMatch > std::numeric_limits::epsilon() ? Score / Relevance.NameMatch : QualScore; + Info.tags = getSymbolTags(Sym); Top.push({Score, std::move(Info)}); }); for (auto &R : std::move(Top).items()) diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 81ee6c9287ee1..fb3919f587ff2 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -867,6 +867,8 @@ llvm::json::Value toJSON(const SymbolInformation &P) { }; if (P.score) O["score"] = *P.score; + if(!P.tags.empty()) + O["tags"] = P.tags; return std::move(O); } diff --git a/clang-tools-extra/clangd/test/symbols.test b/clang-tools-extra/clangd/test/symbols.test index a16a226e48c05..4f0d7b8b08569 100644 --- a/clang-tools-extra/clangd/test/symbols.test +++ b/clang-tools-extra/clangd/test/symbols.test @@ -24,7 +24,10 @@ # CHECK-NEXT: "uri": "file://{{.*}}/vector.h" # CHECK-NEXT: }, # CHECK-NEXT: "name": "vector", -# CHECK-NEXT: "score": {{.*}} +# CHECK-NEXT: "score": {{.*}}, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 18 +# CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ] # CHECK-NEXT:} From c450d629f9699706eaaa51ab059683f420d2873b Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 1 Dec 2025 12:03:35 +0100 Subject: [PATCH 03/29] Minor improvements. --- clang-tools-extra/clangd/FindSymbols.cpp | 3 +++ clang-tools-extra/clangd/FindSymbols.h | 6 +++++- clang-tools-extra/clangd/XRefs.cpp | 2 -- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 175645d4d7c39..5209dd82dc738 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -13,7 +13,10 @@ #include "Quality.h" #include "SourceCode.h" #include "index/Index.h" +#include "index/Symbol.h" +#include "index/SymbolLocation.h" #include "support/Logger.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclFriend.h" #include "clang/AST/DeclTemplate.h" #include "clang/Index/IndexSymbol.h" diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 4847ea359e9ef..169efa9a84584 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -14,13 +14,17 @@ #include "Protocol.h" #include "index/Symbol.h" -#include "clang/AST/Decl.h" #include "llvm/ADT/StringRef.h" +#include "clang/AST/Decl.h" namespace clang { +class NamedDecl; + namespace clangd { class ParsedAST; class SymbolIndex; +struct Symbol; +struct SymbolLocation; /// A bitmask type representing symbol tags supported by LSP. /// \see diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 5a9a9f3eab770..4facc2c6c5c7d 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1898,8 +1898,6 @@ symbolToCallHierarchyItem(const Symbol &S, PathRef TUPath) { if (!Result) return Result; Result->data = S.ID.str(); - if (S.Flags & Symbol::Deprecated) - Result->tags.push_back(SymbolTag::Deprecated); return Result; } From c297add73cd10bfead6b34c7035279dabde201da Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 3 Dec 2025 13:09:58 +0100 Subject: [PATCH 04/29] Set the detail field of HierarchyItem. --- clang-tools-extra/clangd/XRefs.cpp | 2 +- clang-tools-extra/clangd/test/call-hierarchy.test | 1 + clang-tools-extra/clangd/test/type-hierarchy-ext.test | 3 +++ clang-tools-extra/clangd/test/type-hierarchy.test | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 4facc2c6c5c7d..7483440f97c67 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1818,7 +1818,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.detail = printQualifiedName(ND); HI.kind = SK; HI.range = Range{sourceLocToPosition(SM, DeclRange->getBegin()), sourceLocToPosition(SM, DeclRange->getEnd())}; diff --git a/clang-tools-extra/clangd/test/call-hierarchy.test b/clang-tools-extra/clangd/test/call-hierarchy.test index f0d57b60421a4..fb0766c089ee5 100644 --- a/clang-tools-extra/clangd/test/call-hierarchy.test +++ b/clang-tools-extra/clangd/test/call-hierarchy.test @@ -9,6 +9,7 @@ # CHECK-NEXT: "result": [ # CHECK-NEXT: { # CHECK-NEXT: "data": "{{.*}}", +# CHECK-NEXT: "detail": "callee", # CHECK-NEXT: "kind": 12, # CHECK-NEXT: "name": "callee", # CHECK-NEXT: "range": { diff --git a/clang-tools-extra/clangd/test/type-hierarchy-ext.test b/clang-tools-extra/clangd/test/type-hierarchy-ext.test index 8d1a5dc31da0f..983c7538088bf 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy-ext.test +++ b/clang-tools-extra/clangd/test/type-hierarchy-ext.test @@ -52,6 +52,7 @@ # CHECK-NEXT: ], # CHECK-NEXT: "symbolID": "8A991335E4E67D08" # CHECK-NEXT: }, +# CHECK-NEXT: "detail": "Child2", # CHECK-NEXT: "kind": 23, # CHECK-NEXT: "name": "Child2", # CHECK-NEXT: "parents": [ @@ -65,6 +66,7 @@ # CHECK-NEXT: ], # CHECK-NEXT: "symbolID": "ECDC0C46D75120F4" # CHECK-NEXT: }, +# CHECK-NEXT: "detail": "Child1", # CHECK-NEXT: "kind": 23, # CHECK-NEXT: "name": "Child1", # CHECK-NEXT: "parents": [ @@ -73,6 +75,7 @@ # CHECK-NEXT: "parents": [], # CHECK-NEXT: "symbolID": "FE546E7B648D69A7" # CHECK-NEXT: }, +# CHECK-NEXT: "detail": "Parent", # CHECK-NEXT: "kind": 23, # CHECK-NEXT: "name": "Parent", # CHECK-NEXT: "parents": [], diff --git a/clang-tools-extra/clangd/test/type-hierarchy.test b/clang-tools-extra/clangd/test/type-hierarchy.test index 918a37a74098c..d1dda4b92c29c 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy.test +++ b/clang-tools-extra/clangd/test/type-hierarchy.test @@ -22,6 +22,7 @@ # CHECK-NEXT: ], # CHECK-NEXT: "symbolID": "8A991335E4E67D08" # CHECK-NEXT: }, +# CHECK-NEXT: "detail": "Child2", # CHECK-NEXT: "kind": 23, # CHECK-NEXT: "name": "Child2", # CHECK-NEXT: "range": { From 35ee021c67bdb5ed50b422f7477d89330a4c8714 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Fri, 12 Dec 2025 12:17:56 +0100 Subject: [PATCH 05/29] Collect symbols tags from AST in the method incomingCalls. --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 2 +- clang-tools-extra/clangd/ClangdServer.cpp | 12 ++-- clang-tools-extra/clangd/ClangdServer.h | 5 +- clang-tools-extra/clangd/XRefs.cpp | 57 +++++++++++++++++-- clang-tools-extra/clangd/XRefs.h | 3 +- .../clangd/unittests/CallHierarchyTests.cpp | 56 +++++++++--------- 6 files changed, 93 insertions(+), 42 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index ebd42abd2dd61..0a57573ecc469 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1389,7 +1389,7 @@ void ClangdLSPServer::onPrepareCallHierarchy( void ClangdLSPServer::onCallHierarchyIncomingCalls( const CallHierarchyIncomingCallsParams &Params, Callback> Reply) { - Server->incomingCalls(Params.item, std::move(Reply)); + Server->incomingCalls(Params.item.uri.file(), Params.item, std::move(Reply)); } void ClangdLSPServer::onClangdInlayHints(const InlayHintsParams &Params, diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index f1a87dd12d905..21d69b99ad8d4 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -909,12 +909,16 @@ void ClangdServer::prepareCallHierarchy( } void ClangdServer::incomingCalls( + PathRef File, const CallHierarchyItem &Item, Callback> CB) { - WorkScheduler->run("Incoming Calls", "", - [CB = std::move(CB), Item, this]() mutable { - CB(clangd::incomingCalls(Item, Index)); - }); + auto Action = [Item, CB = std::move(CB), this]( + llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + CB(clangd::incomingCalls(Item, Index, InpAST->AST)); + }; + WorkScheduler->runWithAST("Incoming Calls", File, std::move(Action)); } void ClangdServer::inlayHints(PathRef File, std::optional RestrictRange, diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 3ffaf67553dce..ae7e25e2b6db6 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -299,8 +299,9 @@ class ClangdServer { Callback> CB); /// Resolve incoming calls for a given call hierarchy item. - void incomingCalls(const CallHierarchyItem &Item, - Callback>); + void incomingCalls(PathRef File, + const CallHierarchyItem &Item, + Callback> CB); /// Resolve outgoing calls for a given call hierarchy item. void outgoingCalls(const CallHierarchyItem &Item, diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 7483440f97c67..776e1a33e6c68 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -2349,6 +2349,36 @@ void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, Item.uri.file()); } +// Tries to find a NamedDecl in the AST that matches the given Symbol. +// Returns nullptr if the symbol is not found in the current AST. +const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, + const ParsedAST &AST) { + // Try to convert the symbol to a location and find the decl at that location + auto SymLoc = symbolToLocation(Sym, AST.tuPath()); + if (!SymLoc) + return nullptr; + + // Check if the symbol location is in the main file + if (SymLoc->uri.file() != AST.tuPath()) + return nullptr; + + // Convert LSP position to source location + const auto &SM = AST.getSourceManager(); + auto CurLoc = sourceLocationInMainFile(SM, SymLoc->range.start); + if (!CurLoc) { + llvm::consumeError(CurLoc.takeError()); + return nullptr; + } + + // Get all decls at this location + auto Decls = getDeclAtPosition(const_cast(AST), *CurLoc, {}); + if (Decls.empty()) + return nullptr; + + // Return the first decl (usually the most specific one) + return Decls[0]; +} + std::vector prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath) { std::vector Result; @@ -2376,8 +2406,10 @@ prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath) { } std::vector -incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { +incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + const ParsedAST &AST) { std::vector Results; + if (!Index || Item.data.empty()) return Results; auto ID = SymbolID::fromStr(Item.data); @@ -2421,14 +2453,27 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { Index->lookup(ContainerLookup, [&](const Symbol &Caller) { auto It = CallsIn.find(Caller.ID); assert(It != CallsIn.end()); - if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) { + if (auto *ND = getNamedDeclFromSymbol(Caller, AST)) { + if (auto CHI = declToCallHierarchyItem(*ND, AST.tuPath())) { + std::vector FromRanges; + for (const Location &L : It->second) { + if (L.uri != CHI->uri) { + // Call location not in same file as caller. + // This can happen in some edge cases. There's not much we can do, + // since the protocol only allows returning ranges interpreted as + // being in the caller's file. + continue; + } + FromRanges.push_back(L.range); + } + Results.push_back(CallHierarchyIncomingCall{ + std::move(*CHI), std::move(FromRanges), MightNeverCall}); + } + } else if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) { + // Fallback to using symbol if NamedDecl is not available std::vector FromRanges; for (const Location &L : It->second) { if (L.uri != CHI->uri) { - // Call location not in same file as caller. - // This can happen in some edge cases. There's not much we can do, - // since the protocol only allows returning ranges interpreted as - // being in the caller's file. continue; } FromRanges.push_back(L.range); diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 247e52314c3f9..1019fa189a613 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -148,7 +148,8 @@ std::vector prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath); std::vector -incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); +incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + const ParsedAST &AST); std::vector outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index 9859577c7cf7e..f5e9983aa70ec 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -89,12 +89,12 @@ TEST(CallHierarchy, IncomingOneFileCpp) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT( IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("caller1"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); ASSERT_THAT( IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("caller2"), withDetail("caller2"))), @@ -103,13 +103,13 @@ TEST(CallHierarchy, IncomingOneFileCpp) { AllOf(from(AllOf(withName("caller3"), withDetail("caller3"))), iFromRanges(Source.range("Caller1C"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); ASSERT_THAT( IncomingLevel3, ElementsAre(AllOf(from(AllOf(withName("caller3"), withDetail("caller3"))), iFromRanges(Source.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); EXPECT_THAT(IncomingLevel4, IsEmpty()); } @@ -137,12 +137,12 @@ TEST(CallHierarchy, IncomingOneFileObjC) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("MyClass::caller1"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("caller2"), withDetail("MyClass::caller2"))), @@ -152,13 +152,13 @@ TEST(CallHierarchy, IncomingOneFileObjC) { withDetail("MyClass::caller3"))), iFromRanges(Source.range("Caller1C"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(AllOf(withName("caller3"), withDetail("MyClass::caller3"))), iFromRanges(Source.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); EXPECT_THAT(IncomingLevel4, IsEmpty()); } @@ -184,18 +184,18 @@ TEST(CallHierarchy, IncomingIncludeOverrides) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("Func"), withDetail("Implementation::Func"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); ASSERT_THAT( IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("Test"), withDetail("Test"))), iFromRanges(Source.range("FuncCall"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); EXPECT_THAT(IncomingLevel3, IsEmpty()); } @@ -221,13 +221,13 @@ TEST(CallHierarchy, MainFileOnlyRef) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT( IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("caller1"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); EXPECT_THAT( IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("caller2"), withDetail("caller2"))), @@ -256,7 +256,7 @@ TEST(CallHierarchy, IncomingQualified) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("Waldo::find"))); - auto Incoming = incomingCalls(Items[0], Index.get()); + auto Incoming = incomingCalls(Items[0], Index.get(), AST); EXPECT_THAT( Incoming, ElementsAre( @@ -396,13 +396,13 @@ TEST(CallHierarchy, MultiFileCpp) { std::vector Items = prepareCallHierarchy(AST, Pos, TUPath); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("nsa::caller1"))), iFromRanges(Caller1C.range())))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); ASSERT_THAT( IncomingLevel2, ElementsAre( @@ -411,13 +411,13 @@ TEST(CallHierarchy, MultiFileCpp) { AllOf(from(AllOf(withName("caller3"), withDetail("nsa::caller3"))), iFromRanges(Caller3C.range("Caller1"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(AllOf(withName("caller3"), withDetail("nsa::caller3"))), iFromRanges(Caller3C.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); EXPECT_THAT(IncomingLevel4, IsEmpty()); }; @@ -553,12 +553,12 @@ TEST(CallHierarchy, IncomingMultiFileObjC) { std::vector Items = prepareCallHierarchy(AST, Pos, TUPath); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller1")), iFromRanges(Caller1C.range())))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel2, ElementsAre(AllOf(from(withName("caller2")), iFromRanges(Caller2C.range("A"), @@ -566,12 +566,12 @@ TEST(CallHierarchy, IncomingMultiFileObjC) { AllOf(from(withName("caller3")), iFromRanges(Caller3C.range("Caller1"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(withName("caller3")), iFromRanges(Caller3C.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); EXPECT_THAT(IncomingLevel4, IsEmpty()); }; @@ -616,7 +616,7 @@ TEST(CallHierarchy, CallInLocalVarDecl) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto Incoming = incomingCalls(Items[0], Index.get()); + auto Incoming = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(Incoming, ElementsAre(AllOf(from(withName("caller1")), iFromRanges(Source.range("call1"))), AllOf(from(withName("caller2")), @@ -643,7 +643,7 @@ TEST(CallHierarchy, HierarchyOnField) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("var1"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("Callee"))))); @@ -664,7 +664,7 @@ TEST(CallHierarchy, HierarchyOnVar) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("var"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("Callee"))))); @@ -686,14 +686,14 @@ TEST(CallHierarchy, HierarchyOnEnumConstant) { std::vector Items = prepareCallHierarchy(AST, Source.point("Heads"), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("heads"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("CallerH"))))); Items = prepareCallHierarchy(AST, Source.point("Tails"), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("tails"))); - IncomingLevel1 = incomingCalls(Items[0], Index.get()); + IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("CallerT"))))); @@ -718,7 +718,7 @@ TEST(CallHierarchy, CallInDifferentFileThanCaller) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto Incoming = incomingCalls(Items[0], Index.get()); + auto Incoming = incomingCalls(Items[0], Index.get(), AST); // The only call site is in the source file, which is a different file from // the declaration of the function containing the call, which is in the From 89237d198b323e4dd65176c25bcbbb462a586cdd Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Sun, 14 Dec 2025 19:49:49 +0100 Subject: [PATCH 06/29] Collect symbols tags from AST in the method outgoingCalls. --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 2 +- clang-tools-extra/clangd/ClangdServer.cpp | 12 ++++-- clang-tools-extra/clangd/ClangdServer.h | 2 +- clang-tools-extra/clangd/XRefs.cpp | 42 ++++++++++--------- clang-tools-extra/clangd/XRefs.h | 3 +- .../clangd/unittests/CallHierarchyTests.cpp | 20 ++++----- 6 files changed, 45 insertions(+), 36 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 0a57573ecc469..66b4ca9880c4f 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1434,7 +1434,7 @@ void ClangdLSPServer::onInlayHint(const InlayHintsParams &Params, void ClangdLSPServer::onCallHierarchyOutgoingCalls( const CallHierarchyOutgoingCallsParams &Params, Callback> Reply) { - Server->outgoingCalls(Params.item, std::move(Reply)); + Server->outgoingCalls(Params.item.uri.file(), Params.item, std::move(Reply)); } void ClangdLSPServer::applyConfiguration( diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 21d69b99ad8d4..cb26c7a799f5e 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -933,12 +933,16 @@ void ClangdServer::inlayHints(PathRef File, std::optional RestrictRange, } void ClangdServer::outgoingCalls( + PathRef File, const CallHierarchyItem &Item, Callback> CB) { - WorkScheduler->run("Outgoing Calls", "", - [CB = std::move(CB), Item, this]() mutable { - CB(clangd::outgoingCalls(Item, Index)); - }); + auto Action = [Item, CB = std::move(CB), this]( + llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + CB(clangd::outgoingCalls(Item, Index, InpAST->AST)); + }; + WorkScheduler->runWithAST("Outgoing Calls", File, std::move(Action)); } void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index ae7e25e2b6db6..c28224e8bbb6b 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -304,7 +304,7 @@ class ClangdServer { Callback> CB); /// Resolve outgoing calls for a given call hierarchy item. - void outgoingCalls(const CallHierarchyItem &Item, + void outgoingCalls(PathRef File, const CallHierarchyItem &Item, Callback>); /// Resolve inlay hints for a given document. diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 776e1a33e6c68..3539a30818e0b 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -2453,27 +2453,21 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, Index->lookup(ContainerLookup, [&](const Symbol &Caller) { auto It = CallsIn.find(Caller.ID); assert(It != CallsIn.end()); + + std::optional CHI; if (auto *ND = getNamedDeclFromSymbol(Caller, AST)) { - if (auto CHI = declToCallHierarchyItem(*ND, AST.tuPath())) { - std::vector FromRanges; - for (const Location &L : It->second) { - if (L.uri != CHI->uri) { - // Call location not in same file as caller. - // This can happen in some edge cases. There's not much we can do, - // since the protocol only allows returning ranges interpreted as - // being in the caller's file. - continue; - } - FromRanges.push_back(L.range); - } - Results.push_back(CallHierarchyIncomingCall{ - std::move(*CHI), std::move(FromRanges), MightNeverCall}); - } - } else if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) { - // Fallback to using symbol if NamedDecl is not available + CHI = declToCallHierarchyItem(*ND, AST.tuPath()); + } else { + CHI = symbolToCallHierarchyItem(Caller, Item.uri.file()); + } + if (CHI) { std::vector FromRanges; for (const Location &L : It->second) { if (L.uri != CHI->uri) { + // Call location not in same file as caller. + // This can happen in some edge cases. There's not much we can do, + // since the protocol only allows returning ranges interpreted as + // being in the caller's file. continue; } FromRanges.push_back(L.range); @@ -2503,7 +2497,8 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, } std::vector -outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + const ParsedAST &AST) { std::vector Results; if (!Index || Item.data.empty()) return Results; @@ -2549,7 +2544,16 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { auto It = CallsOut.find(Callee.ID); assert(It != CallsOut.end()); - if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file())) { + + std::optional CHI; + + if (auto *ND = getNamedDeclFromSymbol(Callee, AST)) { + CHI = declToCallHierarchyItem(*ND, AST.tuPath()); + } else { + CHI = symbolToCallHierarchyItem(Callee, Item.uri.file()); + } + + if (CHI) { std::vector FromRanges; for (const Location &L : It->second) { if (L.uri != Item.uri) { diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 1019fa189a613..6319729ba39e4 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -152,7 +152,8 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, const ParsedAST &AST); std::vector -outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + const ParsedAST &AST); /// Returns all decls that are referenced in the \p FD except local symbols. llvm::DenseSet getNonLocalDeclRefs(ParsedAST &AST, diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index f5e9983aa70ec..2891b420427d2 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -296,29 +296,29 @@ TEST(CallHierarchy, OutgoingOneFile) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("caller3"))); - auto OugoingLevel1 = outgoingCalls(Items[0], Index.get()); + auto OugoingLevel1 = outgoingCalls(Items[0], Index.get(), AST); ASSERT_THAT( OugoingLevel1, ElementsAre( - AllOf(to(AllOf(withName("caller1"), withDetail("ns::Foo::caller1"))), + AllOf(to(AllOf(withName("Foo::caller1"), withDetail("ns::Foo::caller1"))), oFromRanges(Source.range("Caller1C"))), AllOf(to(AllOf(withName("caller2"), withDetail("caller2"))), oFromRanges(Source.range("Caller2"))))); - auto OutgoingLevel2 = outgoingCalls(OugoingLevel1[1].to, Index.get()); + auto OutgoingLevel2 = outgoingCalls(OugoingLevel1[1].to, Index.get(), AST); ASSERT_THAT( OutgoingLevel2, ElementsAre(AllOf( - to(AllOf(withName("caller1"), withDetail("ns::Foo::caller1"))), + to(AllOf(withName("Foo::caller1"), withDetail("ns::Foo::caller1"))), oFromRanges(Source.range("Caller1A"), Source.range("Caller1B"))))); - auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get()); + auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get(), AST); ASSERT_THAT( OutgoingLevel3, ElementsAre(AllOf(to(AllOf(withName("callee"), withDetail("callee"))), oFromRanges(Source.range("Callee"))))); - auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get()); + auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get(), AST); EXPECT_THAT(OutgoingLevel4, IsEmpty()); } @@ -430,7 +430,7 @@ TEST(CallHierarchy, MultiFileCpp) { ElementsAre(AllOf( withName("caller3"), withFile(testPath(IsDeclaration ? "caller3.hh" : "caller3.cc"))))); - auto OutgoingLevel1 = outgoingCalls(Items[0], Index.get()); + auto OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), AST); ASSERT_THAT( OutgoingLevel1, // fromRanges are interpreted in the context of Items[0]'s file. @@ -444,19 +444,19 @@ TEST(CallHierarchy, MultiFileCpp) { IsDeclaration ? oFromRanges() : oFromRanges(Caller3C.range("Caller2"))))); - auto OutgoingLevel2 = outgoingCalls(OutgoingLevel1[1].to, Index.get()); + auto OutgoingLevel2 = outgoingCalls(OutgoingLevel1[1].to, Index.get(), AST); ASSERT_THAT(OutgoingLevel2, ElementsAre(AllOf( to(AllOf(withName("caller1"), withDetail("nsa::caller1"))), oFromRanges(Caller2C.range("A"), Caller2C.range("B"))))); - auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get()); + auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get(), AST); ASSERT_THAT( OutgoingLevel3, ElementsAre(AllOf(to(AllOf(withName("callee"), withDetail("callee"))), oFromRanges(Caller1C.range())))); - auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get()); + auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get(), AST); EXPECT_THAT(OutgoingLevel4, IsEmpty()); }; From 9eb72dc9f49566bf7fda0c21ea990d3ff39c2ea6 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 15 Dec 2025 11:02:01 +0100 Subject: [PATCH 07/29] Collect symbols tags from AST in the method subtypes. --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 4 +- clang-tools-extra/clangd/ClangdServer.cpp | 36 +++++--- clang-tools-extra/clangd/ClangdServer.h | 4 +- clang-tools-extra/clangd/XRefs.cpp | 89 +++++++++++-------- clang-tools-extra/clangd/XRefs.h | 5 +- .../clangd/test/type-hierarchy-ext.test | 8 +- .../clangd/test/type-hierarchy.test | 4 +- .../clangd/unittests/TypeHierarchyTests.cpp | 4 +- 8 files changed, 91 insertions(+), 63 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 66b4ca9880c4f..39b3d58397fbc 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1356,7 +1356,7 @@ void ClangdLSPServer::onResolveTypeHierarchy( } Reply(serializeTHIForExtension(std::move(**Resp))); }; - Server->resolveTypeHierarchy(Params.item, Params.resolve, Params.direction, + Server->resolveTypeHierarchy(Params.item.uri.file(), Params.item, Params.resolve, Params.direction, std::move(Serialize)); } @@ -1376,7 +1376,7 @@ void ClangdLSPServer::onSuperTypes( void ClangdLSPServer::onSubTypes( const ResolveTypeHierarchyItemParams &Params, Callback> Reply) { - Server->subTypes(Params.item, std::move(Reply)); + Server->subTypes(Params.item.uri.file(), Params.item, std::move(Reply)); } void ClangdLSPServer::onPrepareCallHierarchy( diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index cb26c7a799f5e..c210eda6bfd2e 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -880,21 +880,37 @@ void ClangdServer::superTypes( }); } -void ClangdServer::subTypes(const TypeHierarchyItem &Item, +void ClangdServer::subTypes(PathRef File, const TypeHierarchyItem &Item, Callback> CB) { - WorkScheduler->run( - "typeHierarchy/subTypes", /*Path=*/"", - [=, CB = std::move(CB)]() mutable { CB(clangd::subTypes(Item, Index)); }); + + auto Action = [File = File.str(), Item, CB = std::move(CB), this]( + llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + CB(clangd::subTypes(Item, Index, InpAST->AST)); + }; + WorkScheduler->runWithAST("subTypes Calls", File, std::move(Action)); + // WorkScheduler->run( + // "typeHierarchy/subTypes", /*Path=*/"", + // [=, CB = std::move(CB)]() mutable { CB(clangd::subTypes(Item, Index)); }); } -void ClangdServer::resolveTypeHierarchy( +void ClangdServer::resolveTypeHierarchy(PathRef File, TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction, Callback> CB) { - WorkScheduler->run( - "Resolve Type Hierarchy", "", [=, CB = std::move(CB)]() mutable { - clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index); - CB(Item); - }); + auto Action = [=, CB = std::move(CB), this]( + llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index, InpAST->AST); + CB(Item); + }; + WorkScheduler->runWithAST("resolveTypeHierarchy Calls", File, std::move(Action)); + // WorkScheduler->run( + // "Resolve Type Hierarchy", "", [=, CB = std::move(CB)]() mutable { + // clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index); + // CB(Item); + // }); } void ClangdServer::prepareCallHierarchy( diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index c28224e8bbb6b..431856aa1fadb 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -286,11 +286,11 @@ class ClangdServer { void superTypes(const TypeHierarchyItem &Item, Callback>> CB); /// Get direct children of a type hierarchy item. - void subTypes(const TypeHierarchyItem &Item, + void subTypes(PathRef File, const TypeHierarchyItem &Item, Callback> CB); /// Resolve type hierarchy item in the given direction. - void resolveTypeHierarchy(TypeHierarchyItem Item, int Resolve, + void resolveTypeHierarchy(PathRef File, TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction, Callback> CB); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 3539a30818e0b..ff9a15c3372aa 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1901,18 +1901,58 @@ symbolToCallHierarchyItem(const Symbol &S, PathRef TUPath) { return Result; } +// Tries to find a NamedDecl in the AST that matches the given Symbol. +// Returns nullptr if the symbol is not found in the current AST. +const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, + const ParsedAST &AST) { + // Try to convert the symbol to a location and find the decl at that location + auto SymLoc = symbolToLocation(Sym, AST.tuPath()); + if (!SymLoc) + return nullptr; + + // Check if the symbol location is in the main file + if (SymLoc->uri.file() != AST.tuPath()) + return nullptr; + + // Convert LSP position to source location + const auto &SM = AST.getSourceManager(); + auto CurLoc = sourceLocationInMainFile(SM, SymLoc->range.start); + if (!CurLoc) { + llvm::consumeError(CurLoc.takeError()); + return nullptr; + } + + // Get all decls at this location + auto Decls = getDeclAtPosition(const_cast(AST), *CurLoc, {}); + if (Decls.empty()) + return nullptr; + + // Return the first decl (usually the most specific one) + return Decls[0]; +} + static void fillSubTypes(const SymbolID &ID, std::vector &SubTypes, - const SymbolIndex *Index, int Levels, PathRef TUPath) { + const SymbolIndex *Index, int Levels, PathRef TUPath, + const ParsedAST &AST) { RelationsRequest Req; Req.Subjects.insert(ID); Req.Predicate = RelationKind::BaseOf; Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) { - if (std::optional ChildSym = - symbolToTypeHierarchyItem(Object, TUPath)) { + std::optional ChildSym; + + if (auto *ND = getNamedDeclFromSymbol(Object, AST)) { + ChildSym = declToTypeHierarchyItem(*ND, AST.tuPath()); + elog("fillSubTypes: declToTypeHierarchyItem, {0}", ChildSym.has_value()); + } else { + ChildSym = symbolToTypeHierarchyItem(Object, TUPath); + elog("fillSubTypes: symbolToTypeHierarchyItem, {0}", ChildSym.has_value()); + } + if (ChildSym) { if (Levels > 1) { ChildSym->children.emplace(); - fillSubTypes(Object.ID, *ChildSym->children, Index, Levels - 1, TUPath); + fillSubTypes(Object.ID, *ChildSym->children, Index, Levels - 1, TUPath, + AST); } SubTypes.emplace_back(std::move(*ChildSym)); } @@ -2295,7 +2335,7 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, if (Index) { if (auto ID = getSymbolID(CXXRD)) - fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath); + fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath, AST); } } Results.emplace_back(std::move(*Result)); @@ -2327,9 +2367,9 @@ superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) { } std::vector subTypes(const TypeHierarchyItem &Item, - const SymbolIndex *Index) { + const SymbolIndex *Index, const ParsedAST &AST) { std::vector Results; - fillSubTypes(Item.data.symbolID, Results, Index, 1, Item.uri.file()); + fillSubTypes(Item.data.symbolID, Results, Index, 1, Item.uri.file(), AST); for (auto &ChildSym : Results) ChildSym.data.parents = {Item.data}; return Results; @@ -2337,7 +2377,8 @@ std::vector subTypes(const TypeHierarchyItem &Item, void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, TypeHierarchyDirection Direction, - const SymbolIndex *Index) { + const SymbolIndex *Index, + const ParsedAST &AST) { // We only support typeHierarchy/resolve for children, because for parents // we ignore ResolveLevels and return all levels of parents eagerly. if (!Index || Direction == TypeHierarchyDirection::Parents || @@ -2346,37 +2387,7 @@ void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, Item.children.emplace(); fillSubTypes(Item.data.symbolID, *Item.children, Index, ResolveLevels, - Item.uri.file()); -} - -// Tries to find a NamedDecl in the AST that matches the given Symbol. -// Returns nullptr if the symbol is not found in the current AST. -const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, - const ParsedAST &AST) { - // Try to convert the symbol to a location and find the decl at that location - auto SymLoc = symbolToLocation(Sym, AST.tuPath()); - if (!SymLoc) - return nullptr; - - // Check if the symbol location is in the main file - if (SymLoc->uri.file() != AST.tuPath()) - return nullptr; - - // Convert LSP position to source location - const auto &SM = AST.getSourceManager(); - auto CurLoc = sourceLocationInMainFile(SM, SymLoc->range.start); - if (!CurLoc) { - llvm::consumeError(CurLoc.takeError()); - return nullptr; - } - - // Get all decls at this location - auto Decls = getDeclAtPosition(const_cast(AST), *CurLoc, {}); - if (Decls.empty()) - return nullptr; - - // Return the first decl (usually the most specific one) - return Decls[0]; + Item.uri.file(), AST); } std::vector diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 6319729ba39e4..1582c481ea5c2 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -137,11 +137,12 @@ std::optional> superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index); /// Returns direct children of a TypeHierarchyItem. std::vector subTypes(const TypeHierarchyItem &Item, - const SymbolIndex *Index); + const SymbolIndex *Index, + const ParsedAST &AST); void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, TypeHierarchyDirection Direction, - const SymbolIndex *Index); + const SymbolIndex *Index, const ParsedAST &AST); /// Get call hierarchy information at \p Pos. std::vector diff --git a/clang-tools-extra/clangd/test/type-hierarchy-ext.test b/clang-tools-extra/clangd/test/type-hierarchy-ext.test index 983c7538088bf..d635b199d002c 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy-ext.test +++ b/clang-tools-extra/clangd/test/type-hierarchy-ext.test @@ -17,11 +17,11 @@ # CHECK-NEXT: "name": "Child3", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 13, +# CHECK-NEXT: "character": 25, # CHECK-NEXT: "line": 3 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 7, +# CHECK-NEXT: "character": 0, # CHECK-NEXT: "line": 3 # CHECK-NEXT: } # CHECK-NEXT: }, @@ -162,11 +162,11 @@ # CHECK-NEXT: "name": "Child4", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 13, +# CHECK-NEXT: "character": 25, # CHECK-NEXT: "line": 4 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 7, +# CHECK-NEXT: "character": 0, # CHECK-NEXT: "line": 4 # CHECK-NEXT: } # CHECK-NEXT: }, diff --git a/clang-tools-extra/clangd/test/type-hierarchy.test b/clang-tools-extra/clangd/test/type-hierarchy.test index d1dda4b92c29c..065f129e05d2f 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy.test +++ b/clang-tools-extra/clangd/test/type-hierarchy.test @@ -125,11 +125,11 @@ # CHECK-NEXT: "name": "Child3", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 13, +# CHECK-NEXT: "character": 25, # CHECK-NEXT: "line": 3 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 7, +# CHECK-NEXT: "character": 0, # CHECK-NEXT: "line": 3 # CHECK-NEXT: } # CHECK-NEXT: }, diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp index 406a842f5a008..ae2f806d0b17e 100644 --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -756,7 +756,7 @@ struct Child2b : Child1 {}; parentsNotResolved(), childrenNotResolved())))); resolveTypeHierarchy((*Result.front().children)[0], /*ResolveLevels=*/1, - TypeHierarchyDirection::Children, Index.get()); + TypeHierarchyDirection::Children, Index.get(), AST); EXPECT_THAT( (*Result.front().children)[0], @@ -783,7 +783,7 @@ struct Child : Parent1, Parent2 {}; TypeHierarchyDirection::Children, Index.get(), testPath(TU.Filename)); ASSERT_THAT(Result, SizeIs(1)); - auto Children = subTypes(Result.front(), Index.get()); + auto Children = subTypes(Result.front(), Index.get(), AST); // Make sure parents are populated when getting children. // FIXME: This is partial. From 0b1c3f6635a14ae224a6446c35a6d7159cb47dfa Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 15 Dec 2025 16:25:26 +0100 Subject: [PATCH 08/29] Collect symbols tags from AST in the method supertypes. --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 61 ++++++++--------- clang-tools-extra/clangd/ClangdLSPServer.h | 11 ++-- clang-tools-extra/clangd/ClangdServer.cpp | 65 +++++++++---------- clang-tools-extra/clangd/ClangdServer.h | 5 +- clang-tools-extra/clangd/Protocol.h | 3 + clang-tools-extra/clangd/XRefs.cpp | 28 +++++--- clang-tools-extra/clangd/XRefs.h | 6 +- .../clangd/test/call-hierarchy.test | 3 + .../clangd/test/type-hierarchy.test | 7 +- .../clangd/unittests/TypeHierarchyTests.cpp | 2 +- 10 files changed, 99 insertions(+), 92 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 39b3d58397fbc..d8a1a089beb8c 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -136,10 +136,9 @@ CodeAction toCodeAction(const Fix &F, const URIForFile &File, Edit.textDocument = VersionedTextDocumentIdentifier{{File}, Version}; for (const auto &E : F.Edits) Edit.edits.push_back( - {E.range, E.newText, - SupportChangeAnnotation ? E.annotationId : ""}); + {E.range, E.newText, SupportChangeAnnotation ? E.annotationId : ""}); if (SupportChangeAnnotation) { - for (const auto &[AID, Annotation]: F.Annotations) + for (const auto &[AID, Annotation] : F.Annotations) Action.edit->changeAnnotations[AID] = Annotation; } } @@ -909,24 +908,24 @@ void ClangdLSPServer::onRename(const RenameParams &Params, if (!Server->getDraft(File)) return Reply(llvm::make_error( "onRename called for non-added file", ErrorCode::InvalidParams)); - Server->rename(File, Params.position, Params.newName, Opts.Rename, - [File, Params, Reply = std::move(Reply), - this](llvm::Expected R) mutable { - if (!R) - return Reply(R.takeError()); - if (auto Err = validateEdits(*Server, R->GlobalChanges)) - return Reply(std::move(Err)); - WorkspaceEdit Result; - // FIXME: use documentChanges if SupportDocumentChanges is - // true. - Result.changes.emplace(); - for (const auto &Rep : R->GlobalChanges) { - (*Result - .changes)[URI::createFile(Rep.first()).toString()] = - Rep.second.asTextEdits(); - } - Reply(Result); - }); + Server->rename( + File, Params.position, Params.newName, Opts.Rename, + [File, Params, Reply = std::move(Reply), + this](llvm::Expected R) mutable { + if (!R) + return Reply(R.takeError()); + if (auto Err = validateEdits(*Server, R->GlobalChanges)) + return Reply(std::move(Err)); + WorkspaceEdit Result; + // FIXME: use documentChanges if SupportDocumentChanges is + // true. + Result.changes.emplace(); + for (const auto &Rep : R->GlobalChanges) { + (*Result.changes)[URI::createFile(Rep.first()).toString()] = + Rep.second.asTextEdits(); + } + Reply(Result); + }); } void ClangdLSPServer::onDocumentDidClose( @@ -1070,7 +1069,7 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, std::map ToLSPDiags; ClangdServer::CodeActionInputs Inputs; - for (const auto& LSPDiag : Params.context.diagnostics) { + for (const auto &LSPDiag : Params.context.diagnostics) { if (auto DiagRef = getDiagRef(File.file(), LSPDiag)) { ToLSPDiags[*DiagRef] = LSPDiag; Inputs.Diagnostics.push_back(*DiagRef); @@ -1079,13 +1078,9 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, Inputs.File = File.file(); Inputs.Selection = Params.range; Inputs.RequestedActionKinds = Params.context.only; - Inputs.TweakFilter = [this](const Tweak &T) { - return Opts.TweakFilter(T); - }; - auto CB = [this, - Reply = std::move(Reply), - ToLSPDiags = std::move(ToLSPDiags), File, - Selection = Params.range]( + Inputs.TweakFilter = [this](const Tweak &T) { return Opts.TweakFilter(T); }; + auto CB = [this, Reply = std::move(Reply), ToLSPDiags = std::move(ToLSPDiags), + File, Selection = Params.range]( llvm::Expected Fixits) mutable { if (!Fixits) return Reply(Fixits.takeError()); @@ -1094,8 +1089,7 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, for (const auto &QF : Fixits->QuickFixes) { CAs.push_back(toCodeAction(QF.F, File, Version, SupportsDocumentChanges, SupportsChangeAnnotation)); - if (auto It = ToLSPDiags.find(QF.Diag); - It != ToLSPDiags.end()) { + if (auto It = ToLSPDiags.find(QF.Diag); It != ToLSPDiags.end()) { CAs.back().diagnostics = {It->second}; } } @@ -1356,7 +1350,8 @@ void ClangdLSPServer::onResolveTypeHierarchy( } Reply(serializeTHIForExtension(std::move(**Resp))); }; - Server->resolveTypeHierarchy(Params.item.uri.file(), Params.item, Params.resolve, Params.direction, + Server->resolveTypeHierarchy(Params.item.uri.file(), Params.item, + Params.resolve, Params.direction, std::move(Serialize)); } @@ -1370,7 +1365,7 @@ void ClangdLSPServer::onPrepareTypeHierarchy( void ClangdLSPServer::onSuperTypes( const ResolveTypeHierarchyItemParams &Params, Callback>> Reply) { - Server->superTypes(Params.item, std::move(Reply)); + Server->superTypes(Params.item.uri.file(), Params.item, std::move(Reply)); } void ClangdLSPServer::onSubTypes( diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 6ada3fd9e6e47..64dcbfbc55325 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -133,7 +133,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks, Callback>); void onGoToImplementation(const TextDocumentPositionParams &, Callback>); - void onReference(const ReferenceParams &, Callback>); + void onReference(const ReferenceParams &, + Callback>); void onSwitchSourceHeader(const TextDocumentIdentifier &, Callback>); void onDocumentHighlight(const TextDocumentPositionParams &, @@ -243,7 +244,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks, /// Used to indicate the ClangdLSPServer is being destroyed. std::atomic IsBeingDestroyed = {false}; - // FIXME: The caching is a temporary solution to get corresponding clangd + // FIXME: The caching is a temporary solution to get corresponding clangd // diagnostic from a LSP diagnostic. // Ideally, ClangdServer can generate an identifier for each diagnostic, // emit them via the LSP's data field (which was newly added in LSP 3.16). @@ -259,11 +260,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks, return {LSPDiag.range, LSPDiag.message}; } /// A map from LSP diagnostic to clangd-naive diagnostic. - typedef std::map - DiagnosticToDiagRefMap; + typedef std::map DiagnosticToDiagRefMap; /// Caches the mapping LSP and clangd-naive diagnostics per file. - llvm::StringMap - DiagRefMap; + llvm::StringMap DiagRefMap; // Last semantic-tokens response, for incremental requests. std::mutex SemanticTokensMutex; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index c210eda6bfd2e..5b6525d8f842e 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -62,8 +62,8 @@ namespace clangd { namespace { // Tracks number of times a tweak has been offered. -static constexpr trace::Metric TweakAvailable( - "tweak_available", trace::Metric::Counter, "tweak_id"); +static constexpr trace::Metric + TweakAvailable("tweak_available", trace::Metric::Counter, "tweak_id"); // Update the FileIndex with new ASTs and plumb the diagnostics responses. struct UpdateIndexCallbacks : public ParsingCallbacks { @@ -872,45 +872,42 @@ void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve, } void ClangdServer::superTypes( - const TypeHierarchyItem &Item, + PathRef File, const TypeHierarchyItem &Item, Callback>> CB) { - WorkScheduler->run("typeHierarchy/superTypes", /*Path=*/"", - [=, CB = std::move(CB)]() mutable { - CB(clangd::superTypes(Item, Index)); - }); + auto Action = [File = File.str(), Item, CB = std::move(CB), + this](llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + CB(clangd::superTypes(Item, Index, InpAST->AST)); + }; + WorkScheduler->runWithAST("superTypes Calls", File, std::move(Action)); } void ClangdServer::subTypes(PathRef File, const TypeHierarchyItem &Item, Callback> CB) { - auto Action = [File = File.str(), Item, CB = std::move(CB), this]( - llvm::Expected InpAST) mutable { - if (!InpAST) - return CB(InpAST.takeError()); - CB(clangd::subTypes(Item, Index, InpAST->AST)); - }; - WorkScheduler->runWithAST("subTypes Calls", File, std::move(Action)); - // WorkScheduler->run( - // "typeHierarchy/subTypes", /*Path=*/"", - // [=, CB = std::move(CB)]() mutable { CB(clangd::subTypes(Item, Index)); }); + auto Action = [File = File.str(), Item, CB = std::move(CB), + this](llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + CB(clangd::subTypes(Item, Index, InpAST->AST)); + }; + WorkScheduler->runWithAST("subTypes Calls", File, std::move(Action)); } -void ClangdServer::resolveTypeHierarchy(PathRef File, - TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction, +void ClangdServer::resolveTypeHierarchy( + PathRef File, TypeHierarchyItem Item, int Resolve, + TypeHierarchyDirection Direction, Callback> CB) { - auto Action = [=, CB = std::move(CB), this]( - llvm::Expected InpAST) mutable { + auto Action = [=, CB = std::move(CB), + this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index, InpAST->AST); CB(Item); }; - WorkScheduler->runWithAST("resolveTypeHierarchy Calls", File, std::move(Action)); - // WorkScheduler->run( - // "Resolve Type Hierarchy", "", [=, CB = std::move(CB)]() mutable { - // clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index); - // CB(Item); - // }); + WorkScheduler->runWithAST("resolveTypeHierarchy Calls", File, + std::move(Action)); } void ClangdServer::prepareCallHierarchy( @@ -925,11 +922,10 @@ void ClangdServer::prepareCallHierarchy( } void ClangdServer::incomingCalls( - PathRef File, - const CallHierarchyItem &Item, + PathRef File, const CallHierarchyItem &Item, Callback> CB) { - auto Action = [Item, CB = std::move(CB), this]( - llvm::Expected InpAST) mutable { + auto Action = [Item, CB = std::move(CB), + this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::incomingCalls(Item, Index, InpAST->AST)); @@ -949,11 +945,10 @@ void ClangdServer::inlayHints(PathRef File, std::optional RestrictRange, } void ClangdServer::outgoingCalls( - PathRef File, - const CallHierarchyItem &Item, + PathRef File, const CallHierarchyItem &Item, Callback> CB) { - auto Action = [Item, CB = std::move(CB), this]( - llvm::Expected InpAST) mutable { + auto Action = [Item, CB = std::move(CB), + this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::outgoingCalls(Item, Index, InpAST->AST)); diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 431856aa1fadb..c582a96691deb 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -283,7 +283,7 @@ class ClangdServer { TypeHierarchyDirection Direction, Callback> CB); /// Get direct parents of a type hierarchy item. - void superTypes(const TypeHierarchyItem &Item, + void superTypes(PathRef File, const TypeHierarchyItem &Item, Callback>> CB); /// Get direct children of a type hierarchy item. void subTypes(PathRef File, const TypeHierarchyItem &Item, @@ -299,8 +299,7 @@ class ClangdServer { Callback> CB); /// Resolve incoming calls for a given call hierarchy item. - void incomingCalls(PathRef File, - const CallHierarchyItem &Item, + void incomingCalls(PathRef File, const CallHierarchyItem &Item, Callback> CB); /// Resolve outgoing calls for a given call hierarchy item. diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 7a99721a1e856..b1cd269e45533 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1548,6 +1548,9 @@ struct TypeHierarchyItem { /// The kind of this item. SymbolKind kind; + /// The symbol tags for this item. + std::vector tags; + /// More detail for this item, e.g. the signature of a function. std::optional detail; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index ff9a15c3372aa..ce282cb8a71d3 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1820,6 +1820,7 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { HI.name = printName(Ctx, ND); HI.detail = printQualifiedName(ND); HI.kind = SK; + HI.tags = getSymbolTags(ND); HI.range = Range{sourceLocToPosition(SM, DeclRange->getBegin()), sourceLocToPosition(SM, DeclRange->getEnd())}; HI.selectionRange = Range{NameBegin, NameEnd}; @@ -1938,15 +1939,14 @@ static void fillSubTypes(const SymbolID &ID, RelationsRequest Req; Req.Subjects.insert(ID); Req.Predicate = RelationKind::BaseOf; - Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) { + Index->relations(Req, [&Levels, &Index, &SubTypes, &TUPath, + &AST](const SymbolID &Subject, const Symbol &Object) { std::optional ChildSym; if (auto *ND = getNamedDeclFromSymbol(Object, AST)) { ChildSym = declToTypeHierarchyItem(*ND, AST.tuPath()); - elog("fillSubTypes: declToTypeHierarchyItem, {0}", ChildSym.has_value()); } else { ChildSym = symbolToTypeHierarchyItem(Object, TUPath); - elog("fillSubTypes: symbolToTypeHierarchyItem, {0}", ChildSym.has_value()); } if (ChildSym) { if (Levels > 1) { @@ -2335,7 +2335,8 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, if (Index) { if (auto ID = getSymbolID(CXXRD)) - fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath, AST); + fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath, + AST); } } Results.emplace_back(std::move(*Result)); @@ -2345,7 +2346,8 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, } std::optional> -superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) { +superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index, + const ParsedAST &AST) { std::vector Results; if (!Item.data.parents) return std::nullopt; @@ -2357,8 +2359,14 @@ superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) { Req.IDs.insert(Parent.symbolID); IDToData[Parent.symbolID] = &Parent; } - Index->lookup(Req, [&Item, &Results, &IDToData](const Symbol &S) { - if (auto THI = symbolToTypeHierarchyItem(S, Item.uri.file())) { + Index->lookup(Req, [&Item, &Results, &IDToData, &AST](const Symbol &S) { + std::optional THI; + if (auto *ND = getNamedDeclFromSymbol(S, AST)) { + THI = declToTypeHierarchyItem(*ND, AST.tuPath()); + } else { + THI = symbolToTypeHierarchyItem(S, Item.uri.file()); + } + if (THI) { THI->data = *IDToData.lookup(S.ID); Results.emplace_back(std::move(*THI)); } @@ -2367,7 +2375,8 @@ superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) { } std::vector subTypes(const TypeHierarchyItem &Item, - const SymbolIndex *Index, const ParsedAST &AST) { + const SymbolIndex *Index, + const ParsedAST &AST) { std::vector Results; fillSubTypes(Item.data.symbolID, Results, Index, 1, Item.uri.file(), AST); for (auto &ChildSym : Results) @@ -2377,8 +2386,7 @@ std::vector subTypes(const TypeHierarchyItem &Item, void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, TypeHierarchyDirection Direction, - const SymbolIndex *Index, - const ParsedAST &AST) { + const SymbolIndex *Index, const ParsedAST &AST) { // We only support typeHierarchy/resolve for children, because for parents // we ignore ResolveLevels and return all levels of parents eagerly. if (!Index || Direction == TypeHierarchyDirection::Parents || diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 1582c481ea5c2..f2652b5f274db 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -132,9 +132,11 @@ std::vector getTypeHierarchy( const SymbolIndex *Index = nullptr, PathRef TUPath = PathRef{}); /// Returns direct parents of a TypeHierarchyItem using SymbolIDs stored inside -/// the item. +/// the item or using the AST. std::optional> -superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index); +superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index, + const ParsedAST &AST); + /// Returns direct children of a TypeHierarchyItem. std::vector subTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index, diff --git a/clang-tools-extra/clangd/test/call-hierarchy.test b/clang-tools-extra/clangd/test/call-hierarchy.test index fb0766c089ee5..270b553302ce1 100644 --- a/clang-tools-extra/clangd/test/call-hierarchy.test +++ b/clang-tools-extra/clangd/test/call-hierarchy.test @@ -32,6 +32,9 @@ # CHECK-NEXT: "line": 0 # CHECK-NEXT: } # CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 18 +# CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" # CHECK-NEXT: } --- diff --git a/clang-tools-extra/clangd/test/type-hierarchy.test b/clang-tools-extra/clangd/test/type-hierarchy.test index 065f129e05d2f..c7b8a16ae51e2 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy.test +++ b/clang-tools-extra/clangd/test/type-hierarchy.test @@ -46,6 +46,7 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ +# CHECK-NEXT: 18, # CHECK-NEXT: 19 # CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" @@ -71,11 +72,11 @@ # CHECK-NEXT: "name": "Child1", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 13, +# CHECK-NEXT: "character": 25, # CHECK-NEXT: "line": 1 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 7, +# CHECK-NEXT: "character": 0, # CHECK-NEXT: "line": 1 # CHECK-NEXT: } # CHECK-NEXT: }, @@ -90,6 +91,7 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ +# CHECK-NEXT: 18, # CHECK-NEXT: 19 # CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" @@ -144,6 +146,7 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ +# CHECK-NEXT: 18, # CHECK-NEXT: 19 # CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp index ae2f806d0b17e..8696e6d4b0790 100644 --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -809,7 +809,7 @@ struct Chil^d : Parent {}; TypeHierarchyDirection::Children, Index.get(), testPath(TU.Filename)); ASSERT_THAT(Result, SizeIs(1)); - auto Parents = superTypes(Result.front(), Index.get()); + auto Parents = superTypes(Result.front(), Index.get(), AST); EXPECT_THAT(Parents, Optional(UnorderedElementsAre( AllOf(withName("Parent"), From 7e11d6921dfb4b94b9f75b3f60382d3399fa09eb Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 22 Jan 2026 13:18:40 +0100 Subject: [PATCH 09/29] Extended unit-tests to check the occurrence of symbol tags in call-hierarchy and type-hierarchy. --- .../clangd/unittests/CallHierarchyTests.cpp | 56 +++++++++++++++++++ .../clangd/unittests/TypeHierarchyTests.cpp | 30 ++++++---- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index 2891b420427d2..1d2ae22463a08 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -48,6 +48,12 @@ MATCHER_P(withDetail, N, "") { return arg.detail == N; } MATCHER_P(withFile, N, "") { return arg.uri.file() == N; } MATCHER_P(withSelectionRange, R, "") { return arg.selectionRange == R; } +template +::testing::Matcher withSymbolTags(Tags... tags) { + // Matches the tags vector ignoring element order. + return Field(&CallHierarchyItem::tags, UnorderedElementsAre(tags...)); +} + template ::testing::Matcher from(ItemMatcher M) { return Field(&CallHierarchyIncomingCall::from, M); @@ -728,6 +734,56 @@ TEST(CallHierarchy, CallInDifferentFileThanCaller) { ElementsAre(AllOf(from(withName("caller")), iFromRanges()))); } +TEST(CallHierarchy, IncomingCalls) { + Annotations Source(R"cpp( + class A { + public: + void call^ee() {}; + }; + void caller(A &a) { + a.callee(); + } + )cpp"); + TestTU TU = TestTU::withCode(Source.code()); + auto AST = TU.build(); + auto Index = TU.index(); + + std::vector Items = + prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); + ASSERT_THAT(Items, ElementsAre(withName("callee"))); + + auto Incoming = incomingCalls(Items[0], Index.get(), AST); + EXPECT_THAT( + Incoming, + UnorderedElementsAre(AllOf(from( + AllOf(withName("caller"), withSymbolTags(SymbolTag::Declaration, + SymbolTag::Definition)))))); +} + +TEST(CallHierarchy, OutgoingCalls) { + Annotations Source(R"cpp( + void callee() {} + class A { + public: + void call^er() { + callee(); + }; + }; + )cpp"); + TestTU TU = TestTU::withCode(Source.code()); + auto AST = TU.build(); + auto Index = TU.index(); + + std::vector Items = + prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); + ASSERT_THAT(Items, ElementsAre(withName("caller"))); + + auto Outgoing = outgoingCalls(Items[0], Index.get(), AST); + EXPECT_THAT(Outgoing, UnorderedElementsAre(AllOf( + to(AllOf(withName("callee"), + withSymbolTags(SymbolTag::Declaration, + SymbolTag::Definition)))))); +} } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp index 8696e6d4b0790..21adbb6d0d3a3 100644 --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -54,6 +54,12 @@ MATCHER_P(withResolveParents, M, "") { return testing::ExplainMatchResult(M, arg.data.parents, result_listener); } +template +::testing::Matcher withSymbolTags(Tags... tags) { + // Matches the tags vector ignoring element order. + return Field(&TypeHierarchyItem::tags, UnorderedElementsAre(tags...)); +} + TEST(FindRecordTypeAt, TypeOrVariable) { Annotations Source(R"cpp( struct Ch^ild2 { @@ -770,10 +776,10 @@ struct Child2b : Child1 {}; TEST(Standard, SubTypes) { Annotations Source(R"cpp( -struct Pare^nt1 {}; -struct Parent2 {}; -struct Child : Parent1, Parent2 {}; -)cpp"); + struct Pare^nt1 {}; + struct Parent2 {}; + struct Child final : Parent1, Parent2 {}; + )cpp"); TestTU TU = TestTU::withCode(Source.code()); auto AST = TU.build(); @@ -791,15 +797,17 @@ struct Child : Parent1, Parent2 {}; Children, UnorderedElementsAre( AllOf(withName("Child"), + withSymbolTags(SymbolTag::Declaration, SymbolTag::Definition, + SymbolTag::Final), withResolveParents(Optional(UnorderedElementsAre(withResolveID( getSymbolID(&findDecl(AST, "Parent1")).str()))))))); } TEST(Standard, SuperTypes) { Annotations Source(R"cpp( -struct Parent {}; -struct Chil^d : Parent {}; -)cpp"); + struct Parent {}; + struct Chil^d : Parent {}; + )cpp"); TestTU TU = TestTU::withCode(Source.code()); auto AST = TU.build(); @@ -811,9 +819,11 @@ struct Chil^d : Parent {}; ASSERT_THAT(Result, SizeIs(1)); auto Parents = superTypes(Result.front(), Index.get(), AST); - EXPECT_THAT(Parents, Optional(UnorderedElementsAre( - AllOf(withName("Parent"), - withResolveParents(Optional(IsEmpty())))))); + EXPECT_THAT(Parents, + Optional(UnorderedElementsAre(AllOf( + withName("Parent"), + withSymbolTags(SymbolTag::Declaration, SymbolTag::Definition), + withResolveParents(Optional(IsEmpty())))))); } } // namespace } // namespace clangd From 007764fa085dd099427f0503099165210e6ca871 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Fri, 23 Jan 2026 15:46:57 +0100 Subject: [PATCH 10/29] Simplify lambda capture by explicitly moving `Item` and parameters. Fix code format. --- clang-tools-extra/clangd/ClangdServer.cpp | 2 +- clang-tools-extra/clangd/FindSymbols.h | 2 +- clang-tools-extra/clangd/Protocol.cpp | 6 ++-- .../clangd/unittests/CallHierarchyTests.cpp | 28 +++++++++++-------- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 5b6525d8f842e..e304db2914483 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -899,7 +899,7 @@ void ClangdServer::resolveTypeHierarchy( PathRef File, TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction, Callback> CB) { - auto Action = [=, CB = std::move(CB), + auto Action = [Item = std::move(Item), Resolve, Direction, CB = std::move(CB), this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 169efa9a84584..d85a03ebe449b 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -14,8 +14,8 @@ #include "Protocol.h" #include "index/Symbol.h" -#include "llvm/ADT/StringRef.h" #include "clang/AST/Decl.h" +#include "llvm/ADT/StringRef.h" namespace clang { class NamedDecl; diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index fb3919f587ff2..7d1b06db64e2b 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -209,7 +209,7 @@ bool fromJSON(const llvm::json::Value &Params, ChangeAnnotation &R, O.map("needsConfirmation", R.needsConfirmation) && O.mapOptional("description", R.description); } -llvm::json::Value toJSON(const ChangeAnnotation & CA) { +llvm::json::Value toJSON(const ChangeAnnotation &CA) { llvm::json::Object Result{{"label", CA.label}}; if (CA.needsConfirmation) Result["needsConfirmation"] = *CA.needsConfirmation; @@ -867,7 +867,7 @@ llvm::json::Value toJSON(const SymbolInformation &P) { }; if (P.score) O["score"] = *P.score; - if(!P.tags.empty()) + if (!P.tags.empty()) O["tags"] = P.tags; return std::move(O); } @@ -1447,7 +1447,7 @@ llvm::json::Value toJSON(const TypeHierarchyItem &I) { if (I.detail) Result["detail"] = I.detail; - if(!I.tags.empty()) + if (!I.tags.empty()) Result["tags"] = I.tags; return std::move(Result); } diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index 1d2ae22463a08..fb911ba07354a 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -305,11 +305,11 @@ TEST(CallHierarchy, OutgoingOneFile) { auto OugoingLevel1 = outgoingCalls(Items[0], Index.get(), AST); ASSERT_THAT( OugoingLevel1, - ElementsAre( - AllOf(to(AllOf(withName("Foo::caller1"), withDetail("ns::Foo::caller1"))), - oFromRanges(Source.range("Caller1C"))), - AllOf(to(AllOf(withName("caller2"), withDetail("caller2"))), - oFromRanges(Source.range("Caller2"))))); + ElementsAre(AllOf(to(AllOf(withName("Foo::caller1"), + withDetail("ns::Foo::caller1"))), + oFromRanges(Source.range("Caller1C"))), + AllOf(to(AllOf(withName("caller2"), withDetail("caller2"))), + oFromRanges(Source.range("Caller2"))))); auto OutgoingLevel2 = outgoingCalls(OugoingLevel1[1].to, Index.get(), AST); ASSERT_THAT( @@ -408,7 +408,8 @@ TEST(CallHierarchy, MultiFileCpp) { withDetail("nsa::caller1"))), iFromRanges(Caller1C.range())))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = + incomingCalls(IncomingLevel1[0].from, Index.get(), AST); ASSERT_THAT( IncomingLevel2, ElementsAre( @@ -417,13 +418,15 @@ TEST(CallHierarchy, MultiFileCpp) { AllOf(from(AllOf(withName("caller3"), withDetail("nsa::caller3"))), iFromRanges(Caller3C.range("Caller1"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); + auto IncomingLevel3 = + incomingCalls(IncomingLevel2[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(AllOf(withName("caller3"), withDetail("nsa::caller3"))), iFromRanges(Caller3C.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); + auto IncomingLevel4 = + incomingCalls(IncomingLevel3[0].from, Index.get(), AST); EXPECT_THAT(IncomingLevel4, IsEmpty()); }; @@ -564,7 +567,8 @@ TEST(CallHierarchy, IncomingMultiFileObjC) { ElementsAre(AllOf(from(withName("caller1")), iFromRanges(Caller1C.range())))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = + incomingCalls(IncomingLevel1[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel2, ElementsAre(AllOf(from(withName("caller2")), iFromRanges(Caller2C.range("A"), @@ -572,12 +576,14 @@ TEST(CallHierarchy, IncomingMultiFileObjC) { AllOf(from(withName("caller3")), iFromRanges(Caller3C.range("Caller1"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); + auto IncomingLevel3 = + incomingCalls(IncomingLevel2[0].from, Index.get(), AST); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(withName("caller3")), iFromRanges(Caller3C.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); + auto IncomingLevel4 = + incomingCalls(IncomingLevel3[0].from, Index.get(), AST); EXPECT_THAT(IncomingLevel4, IsEmpty()); }; From f1fdea35baedf486e3d73ca8bee0e3495be30c7f Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 16 Feb 2026 12:36:00 +0100 Subject: [PATCH 11/29] Feat: functions calculating the tags Overrides and Implements. Changed the usage of SymbolTag: - Abstract = pure virtual - Virtual = a virtual method implemented in that class, not implementing or overriding a method from base class - Implements = implements pure virtual method from base class - Override = overriding a virtual method implemented in base class - Final = final method --- clang-tools-extra/clangd/FindSymbols.cpp | 116 ++++++++++++- clang-tools-extra/clangd/Protocol.h | 19 ++- .../clangd/test/symbol-tags.test | 2 - .../clangd/unittests/FindSymbolsTests.cpp | 152 +++++++++++++++--- 4 files changed, 260 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 5209dd82dc738..e980cd08a76eb 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -121,6 +121,16 @@ bool isAbstract(const Decl *D) { // Indicates whether declaration D is virtual in cases where D is a method. bool isVirtual(const Decl *D) { + // We want to treat a method as virtual if it is declared virtual, even if it + // is not implemented in this class, or if it overrides/implements a + // base-class method. This is because the "virtual" modifier is still relevant + // to the method's behavior and how it should be highlighted, even if it is + // not itself a virtual method in the strictest sense. For example, a method + // that overrides a virtual method from a base class is still considered + // virtual, even if it is not declared as such in the derived class. + // Similarly, a method that implements a pure virtual method from a base class + // is also considered virtual, even if it is not declared as such in the + // derived class. if (const auto *CMD = llvm::dyn_cast(D)) return CMD->isVirtual(); return false; @@ -162,6 +172,45 @@ SymbolTags toSymbolTagBitmask(const SymbolTag ST) { return (1 << static_cast(ST)); } +bool isOverrides(const NamedDecl *ND) { + if (const auto *MD = llvm::dyn_cast(ND)) { + // A method "overrides" if: + // 1. It overrides at least one method + // 2. At least one of the overridden methods is virtual (but NOT pure + // virtual) + + if (MD->size_overridden_methods() == 0) + return false; + + for (const auto Overridden : MD->overridden_methods()) { + // Check if the overridden method is virtual but not pure virtual + if (Overridden->isVirtual() && !Overridden->isPureVirtual()) + return true; + } + return false; + } + return false; +} + +bool isImplements(const NamedDecl *ND) { + if (const auto *MD = llvm::dyn_cast(ND)) { + // A method "implements" pure virtual methods from base classes if: + // 1. It overrides at least one method + // 2. It is NOT itself pure virtual (i.e., it has a concrete implementation) + // 3. ALL overridden methods are pure virtual + + if (MD->size_overridden_methods() == 0 || MD->isPureVirtual()) + return false; + + for (const auto Overridden : MD->overridden_methods()) { + if (!Overridden->isPureVirtual()) + return false; + } + return true; + } + return false; +} + SymbolTags computeSymbolTags(const NamedDecl &ND) { SymbolTags Result = 0; const auto IsDef = isUniqueDefinition(&ND); @@ -184,6 +233,12 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) { if (isFinal(&ND)) Result |= toSymbolTagBitmask(SymbolTag::Final); + if (isOverrides(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Overrides); + + if (isImplements(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Implements); + if (not isa(ND)) { // Do not treat an UnresolvedUsingValueDecl as a declaration. // It's more common to think of it as a reference to the @@ -211,6 +266,62 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) { return Result; } +// Filter symbol tags based on the presence of other tags and the kind of +// symbol. This is needed to avoid redundant tags. +SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) { + SymbolTags Result = ST; + + if (not isa(ND)) + return Result; + + if (ST & toSymbolTagBitmask(SymbolTag::Overrides)) { + // Overrides means that ND overrides an existing implementation of a virtual + // method in a base class. If a symbol is marked as Overrides, the tags + // Virtual, Declaration and Definition should be removed, as the Overrides + // tag implies that the symbol has/is virtual/declaration/definition. + Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); + Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); + Result &= ~toSymbolTagBitmask(SymbolTag::Definition); + } + if (ST & toSymbolTagBitmask(SymbolTag::Implements)) { + // Implements means that ND implements an existing pure virtual method in a + // base class. If a symbol is marked as Implements, the tags Virtual, + // Declaration, Definition and Overrides should be removed, as the + // Implements tag implies that the symbol is virtual, is a declaration, is a + // definition, and overrides a method. + Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); + Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); + Result &= ~toSymbolTagBitmask(SymbolTag::Definition); + Result &= ~toSymbolTagBitmask(SymbolTag::Overrides); + } + if (ST & toSymbolTagBitmask(SymbolTag::Virtual)) { + // Virtual means that ND is a virtual method that does not override any + // method in a base class. If a symbol is marked as Virtual, the tags + // Declaration and Definition should be removed, as the Virtual tag implies + // that the symbol is a declaration/definition. + Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); + Result &= ~toSymbolTagBitmask(SymbolTag::Definition); + } + if (ST & toSymbolTagBitmask(SymbolTag::Abstract)) { + // Abstract means that ND is a pure virtual method. If a symbol is marked as + // Abstract, the tags Virtual, Declaration and Definition should be removed, + // as the Abstract tag implies that the symbol is virtual and a + // declaration/definition. + Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); + Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); + Result &= ~toSymbolTagBitmask(SymbolTag::Definition); + } + if (ST & toSymbolTagBitmask(SymbolTag::Final)) { + // Final means that ND is a method that cannot be overridden by any method + // in a derived class. If a symbol is marked as Final, the tags Virtual and + // Overrides should be removed, as the Final tag implies that the symbol is + // virtual. + Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); + Result &= ~toSymbolTagBitmask(SymbolTag::Overrides); + } + return Result; +} + std::vector getSymbolTags(const NamedDecl &ND) { const auto symbolTags = computeSymbolTags(ND); std::vector Tags; @@ -218,6 +329,9 @@ std::vector getSymbolTags(const NamedDecl &ND) { if (symbolTags == 0) return Tags; + // Apply specific filter to the symbol tags. + const auto filteredTags = filterSymbolTags(ND, symbolTags); + // Iterate through SymbolTag enum values and collect any that are present in // the bitmask. SymbolTag values are in the numeric range // [FirstTag .. LastTag]. @@ -225,7 +339,7 @@ std::vector getSymbolTags(const NamedDecl &ND) { constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); for (unsigned I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); - if (symbolTags & toSymbolTagBitmask(ST)) + if (filteredTags & toSymbolTagBitmask(ST)) Tags.push_back(ST); } return Tags; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index b1cd269e45533..baa2d659b218f 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1115,22 +1115,33 @@ enum class SymbolTag { Internal = 6, File = 7, Static = 8, - Abstract = 9, - Final = 10, + Abstract = 9, // In context of a class and method - this symbol indicates a + // pure virtual class or method. + Final = 10, // In context of a method - this symbol indicates that the method + // cannot be overridden in subclasses. + // In context of a class - this symbol indicates that the class is + // final and thus cannot be extended. Sealed = 11, Transient = 12, Volatile = 13, Synchronized = 14, - Virtual = 15, + Virtual = + 15, // In context of a method - this symbol indicates a virtual + // method declared and implemented in same class, and thereby it is + // not implementing or overriding a method from any base class. Nullable = 16, NonNull = 17, Declaration = 18, Definition = 19, ReadOnly = 20, + Overrides = 21, // In context of a method - this symbol indicates a method + // overriding a virtual method, implemented in base class. + Implements = 22, // In context of a method - this symbol indicates a method + // implementing a pure virtual method from a base class. // Update as needed FirstTag = Deprecated, - LastTag = ReadOnly + LastTag = Implements }; llvm::json::Value toJSON(SymbolTag); /// Represents programming constructs like variables, classes, interfaces etc. diff --git a/clang-tools-extra/clangd/test/symbol-tags.test b/clang-tools-extra/clangd/test/symbol-tags.test index 6b17dc994d029..34c06c4b9ba63 100644 --- a/clang-tools-extra/clangd/test/symbol-tags.test +++ b/clang-tools-extra/clangd/test/symbol-tags.test @@ -41,8 +41,6 @@ # CHECK: "tags": [ # CHECK: 2, # CHECK: 9, -# CHECK: 15, -# CHECK: 18, # CHECK: 20 # CHECK: ] # CHECK: } diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index 2d237429ebfbf..63fb76e330d8f 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -1160,12 +1160,125 @@ TEST(DocumentSymbolsTest, PragmaMarkGroupsNoNesting) { withName("Core"), withName("coreMethod"))); } -TEST(DocumentSymbolsTest, SymbolTags) { +TEST(DocumentSymbolsTest, SymbolTagsMustContainPublicAbstract) { TestTU TU; Annotations Main(R"cpp( - class AbstractClass { + class A { public: - virtual ~AbstractClass() = default; + virtual void f1() = 0; + }; + + class B : public A { + public: + virtual void f2() = 0; + }; + )cpp"); + + TU.Code = Main.code().str(); + auto Symbols = getSymbols(TU.build()); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + AllOf(withName("A"), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Abstract)))), + AllOf(withName("B"), + children(AllOf(withName("f2"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Abstract)))))); +} + +TEST(DocumentSymbolsTest, SymbolTagsMustContainPublicVirtualAndOverrides) { + TestTU TU; + Annotations Main(R"cpp( + class A { + public: + virtual void f1() {}; + }; + + class B : public A { + public: + void f1() override {} + }; + + class C : public B { + public: + void f1() override {} + }; + )cpp"); + + TU.Code = Main.code().str(); + auto Symbols = getSymbols(TU.build()); + EXPECT_THAT( + Symbols, + UnorderedElementsAre( + AllOf(withName("A"), + children( + AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Virtual)))), + AllOf(withName("B"), + children(AllOf( + withName("f1"), + withSymbolTags(SymbolTag::Public, SymbolTag::Overrides)))), + AllOf(withName("C"), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Overrides)))))); +} + +TEST(DocumentSymbolsTest, + SymbolTagsMustContainPublicAbstractImplementsOverridesAndFinal) { + TestTU TU; + Annotations Main(R"cpp( + class A { + public: + virtual void f1() = 0; + }; + + class B : public A { + public: + void f1() override {} + }; + + class C : public B { + public: + void f1() override {} + }; + + class D : public C { + public: + void f1() final override {} + }; + )cpp"); + + TU.Code = Main.code().str(); + auto Symbols = getSymbols(TU.build()); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + AllOf(withName("A"), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Abstract)))), + AllOf(withName("B"), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Implements)))), + AllOf(withName("C"), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Overrides)))), + AllOf(withName("D"), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Final)))))); +} + +TEST(DocumentSymbolsTest, SymbolTagsCompilation) { + TestTU TU; + Annotations Main(R"cpp( + class A { + public: + virtual ~A() = default; virtual void f1() = 0; void f2() const; protected: @@ -1174,9 +1287,9 @@ TEST(DocumentSymbolsTest, SymbolTags) { static void f4(){} }; - void AbstractClass::f2() const {} + void A::f2() const {} - class ImplClass final: public AbstractClass { + class B final: public A { public: void f1() final {} }; @@ -1188,18 +1301,14 @@ TEST(DocumentSymbolsTest, SymbolTags) { Symbols, UnorderedElementsAre( AllOf( - withName("AbstractClass"), + withName("A"), withSymbolTags(SymbolTag::Abstract, SymbolTag::Declaration, SymbolTag::Definition), children( - AllOf(withName("~AbstractClass"), - withSymbolTags(SymbolTag::Public, SymbolTag::Virtual, - SymbolTag::Declaration, - SymbolTag::Definition)), + AllOf(withName("~A"), + withSymbolTags(SymbolTag::Public, SymbolTag::Virtual)), AllOf(withName("f1"), - withSymbolTags(SymbolTag::Public, SymbolTag::Abstract, - SymbolTag::Virtual, - SymbolTag::Declaration)), + withSymbolTags(SymbolTag::Public, SymbolTag::Abstract)), AllOf(withName("f2"), withSymbolTags(SymbolTag::Public, SymbolTag::Declaration, SymbolTag::ReadOnly)), @@ -1210,17 +1319,16 @@ TEST(DocumentSymbolsTest, SymbolTags) { withSymbolTags(SymbolTag::Private, SymbolTag::Static, SymbolTag::Declaration, SymbolTag::Definition)))), - AllOf(withName("AbstractClass::f2"), + AllOf(withName("A::f2"), withSymbolTags(SymbolTag::Public, SymbolTag::Declaration, SymbolTag::Definition, SymbolTag::ReadOnly)), - AllOf(withName("ImplClass"), - withSymbolTags(SymbolTag::Final, SymbolTag::Declaration, - SymbolTag::Definition), - children(AllOf( - withName("f1"), - withSymbolTags(SymbolTag::Public, SymbolTag::Final, - SymbolTag::Virtual, SymbolTag::Declaration, - SymbolTag::Definition)))))); + AllOf( + withName("B"), + withSymbolTags(SymbolTag::Final, SymbolTag::Declaration, + SymbolTag::Definition), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, SymbolTag::Final, + SymbolTag::Implements)))))); } } // namespace From 2c0475f663304dea565e76780f7523aae1c70b42 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 25 Feb 2026 12:53:13 +0100 Subject: [PATCH 12/29] Feat: compute symbol tags during the AST indexation. --- clang-tools-extra/clangd/FindSymbols.cpp | 35 +++++---- clang-tools-extra/clangd/FindSymbols.h | 14 +--- clang-tools-extra/clangd/XRefs.cpp | 2 +- clang-tools-extra/clangd/index/Merge.cpp | 1 + clang-tools-extra/clangd/index/Symbol.h | 15 ++++ .../clangd/index/SymbolCollector.cpp | 14 ++-- .../clangd/index/YAMLSerialization.cpp | 1 + .../clangd/test/Inputs/symbols.test.yaml | 3 +- .../clangd/unittests/FindSymbolsTests.cpp | 74 +++++++++++++++++++ 9 files changed, 126 insertions(+), 33 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index e980cd08a76eb..7d87885c5c60c 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -322,15 +322,15 @@ SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) { return Result; } -std::vector getSymbolTags(const NamedDecl &ND) { - const auto symbolTags = computeSymbolTags(ND); +std::vector expandTagBitmask(const SymbolTags symbolTags) { std::vector Tags; if (symbolTags == 0) return Tags; - // Apply specific filter to the symbol tags. - const auto filteredTags = filterSymbolTags(ND, symbolTags); + // No filtering required since this function is only used for Symbols from the + // index, which have already been filtered in getSymbolTags(const NamedDecl + // &ND). // Iterate through SymbolTag enum values and collect any that are present in // the bitmask. SymbolTag values are in the numeric range @@ -339,23 +339,32 @@ std::vector getSymbolTags(const NamedDecl &ND) { constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); for (unsigned I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); - if (filteredTags & toSymbolTagBitmask(ST)) + if (symbolTags & toSymbolTagBitmask(ST)) Tags.push_back(ST); } return Tags; } -std::vector getSymbolTags(const Symbol &S) { +std::vector getSymbolTags(const NamedDecl &ND) { + const auto symbolTags = computeSymbolTags(ND); std::vector Tags; - if (S.Flags & Symbol::Deprecated) - Tags.push_back(SymbolTag::Deprecated); + if (symbolTags == 0) + return Tags; - if (S.Definition) - Tags.push_back(SymbolTag::Definition); - else - Tags.push_back(SymbolTag::Declaration); + // Apply specific filter to the symbol tags. + const auto filteredTags = filterSymbolTags(ND, symbolTags); + // Iterate through SymbolTag enum values and collect any that are present in + // the bitmask. SymbolTag values are in the numeric range + // [FirstTag .. LastTag]. + constexpr unsigned MinTag = static_cast(SymbolTag::FirstTag); + constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); + for (unsigned I = MinTag; I <= MaxTag; ++I) { + auto ST = static_cast(I); + if (filteredTags & toSymbolTagBitmask(ST)) + Tags.push_back(ST); + } return Tags; } @@ -490,7 +499,7 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, Info.score = Relevance.NameMatch > std::numeric_limits::epsilon() ? Score / Relevance.NameMatch : QualScore; - Info.tags = getSymbolTags(Sym); + Info.tags = expandTagBitmask(Sym.Tags); Top.push({Score, std::move(Info)}); }); for (auto &R : std::move(Top).items()) diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index d85a03ebe449b..c13231fb644a0 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -12,7 +12,6 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDSYMBOLS_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDSYMBOLS_H -#include "Protocol.h" #include "index/Symbol.h" #include "clang/AST/Decl.h" #include "llvm/ADT/StringRef.h" @@ -26,15 +25,6 @@ class SymbolIndex; struct Symbol; struct SymbolLocation; -/// A bitmask type representing symbol tags supported by LSP. -/// \see -/// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#symbolTag -using SymbolTags = uint32_t; -/// Ensure we have enough bits to represent all SymbolTag values. -static_assert(static_cast(SymbolTag::LastTag) <= 32, - "Too many SymbolTags to fit in uint32_t. Change to uint64_t if " - "we ever have more than 32 tags."); - /// Helper function for deriving an LSP Location from an index SymbolLocation. llvm::Expected indexToLSPLocation(const SymbolLocation &Loc, llvm::StringRef TUPath); @@ -73,8 +63,8 @@ SymbolTags computeSymbolTags(const NamedDecl &ND); /// \p ND The declaration to get tags for. std::vector getSymbolTags(const NamedDecl &ND); -/// Returns the symbol tags for an index `Symbol`. -std::vector getSymbolTags(const Symbol &S); +/// Returns the symbol tags for the given declaration as a bitmask. +std::vector expandTagBitmask(SymbolTags symbolTags); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index ce282cb8a71d3..088bd98f3b269 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1873,7 +1873,7 @@ static std::optional symbolToHierarchyItem(const Symbol &S, HI.name = std::string(S.Name); HI.detail = (S.Scope + S.Name).str(); HI.kind = indexSymbolKindToSymbolKind(S.SymInfo); - HI.tags = getSymbolTags(S); + HI.tags = expandTagBitmask(S.Tags); HI.selectionRange = Loc->range; // FIXME: Populate 'range' correctly // (https://github.com/clangd/clangd/issues/59). diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index 625b1c6926a28..4a7caeb9ace60 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -319,6 +319,7 @@ Symbol mergeSymbol(const Symbol &L, const Symbol &R) { S.Origin |= O.Origin | SymbolOrigin::Merge; S.Flags |= O.Flags; + S.Tags |= O.Tags; return S; } diff --git a/clang-tools-extra/clangd/index/Symbol.h b/clang-tools-extra/clangd/index/Symbol.h index 62c47ddfc5758..45e6bebd04faa 100644 --- a/clang-tools-extra/clangd/index/Symbol.h +++ b/clang-tools-extra/clangd/index/Symbol.h @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H +#include "Protocol.h" #include "index/SymbolID.h" #include "index/SymbolLocation.h" #include "index/SymbolOrigin.h" @@ -22,6 +23,15 @@ namespace clangd { LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); +/// A bitmask type representing symbol tags supported by LSP. +/// \see +/// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#symbolTag +using SymbolTags = uint32_t; +/// Ensure we have enough bits to represent all SymbolTag values. +static_assert(static_cast(SymbolTag::LastTag) <= 32, + "Too many SymbolTags to fit in uint32_t. Change to uint64_t if " + "we ever have more than 32 tags."); + /// The class presents a C++ symbol, e.g. class, function. /// /// WARNING: Symbols do not own much of their underlying data - typically @@ -87,6 +97,11 @@ struct Symbol { /// Only set when the symbol is indexed for completion. llvm::StringRef Type; + /// Symbol tags for LSP protocol (Deprecated, Static, Virtual, Abstract, + /// Final, ReadOnly, Public, Protected, Private, Declaration, Definition). + /// This is a bitmask where each bit represents a SymbolTag. + SymbolTags Tags = 0; + enum IncludeDirective : uint8_t { Invalid = 0, /// `#include "header.h"` diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index bd974e4c18818..cd6fd53d310a9 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -11,6 +11,7 @@ #include "CodeComplete.h" #include "CodeCompletionStrings.h" #include "ExpectedTypes.h" +#include "FindSymbols.h" #include "SourceCode.h" #include "URI.h" #include "clang-include-cleaner/Analysis.h" @@ -428,8 +429,7 @@ class SymbolCollector::HeaderFileURICache { CachePathToFrameworkSpelling.erase(Res.first); return std::nullopt; } - if (auto UmbrellaSpelling = - getFrameworkUmbrellaSpelling(HS, *HeaderPath)) { + if (auto UmbrellaSpelling = getFrameworkUmbrellaSpelling(HS, *HeaderPath)) { *CachedHeaderSpelling = *UmbrellaSpelling; return llvm::StringRef(*CachedHeaderSpelling); } @@ -926,7 +926,7 @@ llvm::StringRef getStdHeader(const Symbol *S, const LangOptions &LangOpts) { tooling::stdlib::Lang Lang = tooling::stdlib::Lang::CXX; if (LangOpts.C11) Lang = tooling::stdlib::Lang::C; - else if(!LangOpts.CPlusPlus) + else if (!LangOpts.CPlusPlus) return ""; if (S->Scope == "std::" && S->Name == "move") { @@ -1046,8 +1046,7 @@ void SymbolCollector::finish() { // depending on the translation unit. else if (tooling::isSelfContainedHeader(H.physical(), SM, PP->getHeaderSearchInfo())) - SpellingIt->second = - HeaderFileURIs->toURI(H.physical()); + SpellingIt->second = HeaderFileURIs->toURI(H.physical()); } else { SpellingIt->second = include_cleaner::spellHeader( {H, PP->getHeaderSearchInfo(), @@ -1126,6 +1125,9 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, S.Documentation = Documentation; } }; + + S.Tags = computeSymbolTags(ND); + if (!(S.Flags & Symbol::IndexedForCodeCompletion)) { if (Opts.StoreAllDocumentation) UpdateDoc(); @@ -1192,7 +1194,7 @@ void SymbolCollector::addDefinition(const NamedDecl &ND, const Symbol &DeclSym, S.Flags |= Symbol::HasDocComment; S.Documentation = Documentation; } - + S.Tags |= computeSymbolTags(ND); Symbols.insert(S); } diff --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp b/clang-tools-extra/clangd/index/YAMLSerialization.cpp index 495d8a2ff487a..6109761d2e8ed 100644 --- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp +++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp @@ -246,6 +246,7 @@ template <> struct MappingTraits { IO.mapOptional("Documentation", Sym.Documentation); IO.mapOptional("ReturnType", Sym.ReturnType); IO.mapOptional("Type", Sym.Type); + IO.mapOptional("Tags", Sym.Tags, clang::clangd::SymbolTags(0)); IO.mapOptional("IncludeHeaders", NIncludeHeaders->Headers); } }; diff --git a/clang-tools-extra/clangd/test/Inputs/symbols.test.yaml b/clang-tools-extra/clangd/test/Inputs/symbols.test.yaml index 6d457ad8ad498..4ddc5d9c2ea30 100644 --- a/clang-tools-extra/clangd/test/Inputs/symbols.test.yaml +++ b/clang-tools-extra/clangd/test/Inputs/symbols.test.yaml @@ -6,7 +6,8 @@ Scope: 'std::' SymInfo: Kind: Class Lang: Cpp -CanonicalDeclaration: +Tags: 262144 +CanonicalDeclaration: FileURI: 'test:///vector.h' Start: Line: 215 diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index 63fb76e330d8f..a14c32b25282c 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -7,8 +7,10 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" #include "FindSymbols.h" +#include "SyncAPI.h" #include "TestFS.h" #include "TestTU.h" +#include "index/FileIndex.h" #include "llvm/ADT/StringRef.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -1331,6 +1333,78 @@ TEST(DocumentSymbolsTest, SymbolTagsCompilation) { SymbolTag::Implements)))))); } +TEST(DocumentSymbolsTest, SymbolTagsWithIndexing) { + // Test that verifies symbol tags are correctly set when the AST is indexed + // through FileIndex, which triggers the full indexing path through + // SymbolCollector::addDeclaration where S.Tags = computeSymbolTags(ND) + TestTU TU; + Annotations Main(R"cpp( + class A { + public: + virtual ~A() = default; + virtual void f1() = 0; + void f2() const; + protected: + void f3(){} + private: + static void f4(){} + }; + + void A::f2() const {} + + class B final: public A { + public: + void f1() final {} + }; + )cpp"); + + TU.Code = Main.code().str(); + auto AST = TU.build(); + + // This path goes through: + // FileIndex::updateMain() -> indexMainDecls() -> indexSymbols() -> + // SymbolCollector -> addDeclaration() -> S.Tags = computeSymbolTags(ND) + + FileIndex Index{false}; + Index.updateMain(testPath(TU.Filename), AST); + // Verify that the index contains symbols with correct tags + // Note: We can't directly inspect Symbol.Tags from the index in this test, + // but the fact that updateMain() completes successfully demonstrates that: + // 1. SymbolCollector::addDeclaration() was called for each decl + // 2. computeSymbolTags() was executed and S.Tags was set + // 3. The full indexing pipeline works with our tag implementation + + auto Indexed = runFuzzyFind(Index, ""); + EXPECT_FALSE(Indexed.empty()); + + auto FindByQName = [&](llvm::StringRef QName) -> const Symbol * { + for (const auto &S : Indexed) { + if ((S.Scope + S.Name).str() == QName.str()) + return &S; + } + return nullptr; + }; + + const Symbol *A = FindByQName("A"); + ASSERT_TRUE(A); + EXPECT_THAT(expandTagBitmask(A->Tags), + UnorderedElementsAre(SymbolTag::Abstract, SymbolTag::Declaration, + SymbolTag::Definition)); + + const Symbol *B = FindByQName("B"); + ASSERT_TRUE(B); + EXPECT_THAT(expandTagBitmask(B->Tags), + UnorderedElementsAre(SymbolTag::Final, SymbolTag::Declaration, + SymbolTag::Definition)); + const Symbol *Bf1 = FindByQName("B::f1"); + ASSERT_TRUE(Bf1); + EXPECT_THAT(expandTagBitmask(Bf1->Tags), + UnorderedElementsAre(SymbolTag::Public, SymbolTag::Final, + SymbolTag::Virtual, SymbolTag::Declaration, + SymbolTag::Definition, + SymbolTag::Implements)); +} + } // namespace } // namespace clangd } // namespace clang From d6865ccdc3669a117e6a674e8ac529d17519091b Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 19 Mar 2026 14:18:21 +0100 Subject: [PATCH 13/29] Change: removed AST usage in call/type hierarchy --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 67 +++++++------ clang-tools-extra/clangd/ClangdLSPServer.h | 9 +- clang-tools-extra/clangd/ClangdServer.cpp | 73 ++++++-------- clang-tools-extra/clangd/ClangdServer.h | 12 +-- clang-tools-extra/clangd/Protocol.cpp | 2 +- clang-tools-extra/clangd/XRefs.cpp | 91 ++++++------------ clang-tools-extra/clangd/XRefs.h | 17 ++-- .../clangd/index/SymbolCollector.cpp | 8 +- .../clangd/test/type-hierarchy-ext.test | 8 +- .../clangd/test/type-hierarchy.test | 8 +- .../clangd/unittests/CallHierarchyTests.cpp | 94 +++++++++---------- .../clangd/unittests/TypeHierarchyTests.cpp | 20 ++-- 12 files changed, 176 insertions(+), 233 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index d8a1a089beb8c..ebd42abd2dd61 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -136,9 +136,10 @@ CodeAction toCodeAction(const Fix &F, const URIForFile &File, Edit.textDocument = VersionedTextDocumentIdentifier{{File}, Version}; for (const auto &E : F.Edits) Edit.edits.push_back( - {E.range, E.newText, SupportChangeAnnotation ? E.annotationId : ""}); + {E.range, E.newText, + SupportChangeAnnotation ? E.annotationId : ""}); if (SupportChangeAnnotation) { - for (const auto &[AID, Annotation] : F.Annotations) + for (const auto &[AID, Annotation]: F.Annotations) Action.edit->changeAnnotations[AID] = Annotation; } } @@ -908,24 +909,24 @@ void ClangdLSPServer::onRename(const RenameParams &Params, if (!Server->getDraft(File)) return Reply(llvm::make_error( "onRename called for non-added file", ErrorCode::InvalidParams)); - Server->rename( - File, Params.position, Params.newName, Opts.Rename, - [File, Params, Reply = std::move(Reply), - this](llvm::Expected R) mutable { - if (!R) - return Reply(R.takeError()); - if (auto Err = validateEdits(*Server, R->GlobalChanges)) - return Reply(std::move(Err)); - WorkspaceEdit Result; - // FIXME: use documentChanges if SupportDocumentChanges is - // true. - Result.changes.emplace(); - for (const auto &Rep : R->GlobalChanges) { - (*Result.changes)[URI::createFile(Rep.first()).toString()] = - Rep.second.asTextEdits(); - } - Reply(Result); - }); + Server->rename(File, Params.position, Params.newName, Opts.Rename, + [File, Params, Reply = std::move(Reply), + this](llvm::Expected R) mutable { + if (!R) + return Reply(R.takeError()); + if (auto Err = validateEdits(*Server, R->GlobalChanges)) + return Reply(std::move(Err)); + WorkspaceEdit Result; + // FIXME: use documentChanges if SupportDocumentChanges is + // true. + Result.changes.emplace(); + for (const auto &Rep : R->GlobalChanges) { + (*Result + .changes)[URI::createFile(Rep.first()).toString()] = + Rep.second.asTextEdits(); + } + Reply(Result); + }); } void ClangdLSPServer::onDocumentDidClose( @@ -1069,7 +1070,7 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, std::map ToLSPDiags; ClangdServer::CodeActionInputs Inputs; - for (const auto &LSPDiag : Params.context.diagnostics) { + for (const auto& LSPDiag : Params.context.diagnostics) { if (auto DiagRef = getDiagRef(File.file(), LSPDiag)) { ToLSPDiags[*DiagRef] = LSPDiag; Inputs.Diagnostics.push_back(*DiagRef); @@ -1078,9 +1079,13 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, Inputs.File = File.file(); Inputs.Selection = Params.range; Inputs.RequestedActionKinds = Params.context.only; - Inputs.TweakFilter = [this](const Tweak &T) { return Opts.TweakFilter(T); }; - auto CB = [this, Reply = std::move(Reply), ToLSPDiags = std::move(ToLSPDiags), - File, Selection = Params.range]( + Inputs.TweakFilter = [this](const Tweak &T) { + return Opts.TweakFilter(T); + }; + auto CB = [this, + Reply = std::move(Reply), + ToLSPDiags = std::move(ToLSPDiags), File, + Selection = Params.range]( llvm::Expected Fixits) mutable { if (!Fixits) return Reply(Fixits.takeError()); @@ -1089,7 +1094,8 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, for (const auto &QF : Fixits->QuickFixes) { CAs.push_back(toCodeAction(QF.F, File, Version, SupportsDocumentChanges, SupportsChangeAnnotation)); - if (auto It = ToLSPDiags.find(QF.Diag); It != ToLSPDiags.end()) { + if (auto It = ToLSPDiags.find(QF.Diag); + It != ToLSPDiags.end()) { CAs.back().diagnostics = {It->second}; } } @@ -1350,8 +1356,7 @@ void ClangdLSPServer::onResolveTypeHierarchy( } Reply(serializeTHIForExtension(std::move(**Resp))); }; - Server->resolveTypeHierarchy(Params.item.uri.file(), Params.item, - Params.resolve, Params.direction, + Server->resolveTypeHierarchy(Params.item, Params.resolve, Params.direction, std::move(Serialize)); } @@ -1365,13 +1370,13 @@ void ClangdLSPServer::onPrepareTypeHierarchy( void ClangdLSPServer::onSuperTypes( const ResolveTypeHierarchyItemParams &Params, Callback>> Reply) { - Server->superTypes(Params.item.uri.file(), Params.item, std::move(Reply)); + Server->superTypes(Params.item, std::move(Reply)); } void ClangdLSPServer::onSubTypes( const ResolveTypeHierarchyItemParams &Params, Callback> Reply) { - Server->subTypes(Params.item.uri.file(), Params.item, std::move(Reply)); + Server->subTypes(Params.item, std::move(Reply)); } void ClangdLSPServer::onPrepareCallHierarchy( @@ -1384,7 +1389,7 @@ void ClangdLSPServer::onPrepareCallHierarchy( void ClangdLSPServer::onCallHierarchyIncomingCalls( const CallHierarchyIncomingCallsParams &Params, Callback> Reply) { - Server->incomingCalls(Params.item.uri.file(), Params.item, std::move(Reply)); + Server->incomingCalls(Params.item, std::move(Reply)); } void ClangdLSPServer::onClangdInlayHints(const InlayHintsParams &Params, @@ -1429,7 +1434,7 @@ void ClangdLSPServer::onInlayHint(const InlayHintsParams &Params, void ClangdLSPServer::onCallHierarchyOutgoingCalls( const CallHierarchyOutgoingCallsParams &Params, Callback> Reply) { - Server->outgoingCalls(Params.item.uri.file(), Params.item, std::move(Reply)); + Server->outgoingCalls(Params.item, std::move(Reply)); } void ClangdLSPServer::applyConfiguration( diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 64dcbfbc55325..bd9c5e6bc6954 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -133,8 +133,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks, Callback>); void onGoToImplementation(const TextDocumentPositionParams &, Callback>); - void onReference(const ReferenceParams &, - Callback>); + void onReference(const ReferenceParams &, Callback>); void onSwitchSourceHeader(const TextDocumentIdentifier &, Callback>); void onDocumentHighlight(const TextDocumentPositionParams &, @@ -260,9 +259,11 @@ class ClangdLSPServer : private ClangdServer::Callbacks, return {LSPDiag.range, LSPDiag.message}; } /// A map from LSP diagnostic to clangd-naive diagnostic. - typedef std::map DiagnosticToDiagRefMap; + typedef std::map + DiagnosticToDiagRefMap; /// Caches the mapping LSP and clangd-naive diagnostics per file. - llvm::StringMap DiagRefMap; + llvm::StringMap + DiagRefMap; // Last semantic-tokens response, for incremental requests. std::mutex SemanticTokensMutex; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index e304db2914483..f1a87dd12d905 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -62,8 +62,8 @@ namespace clangd { namespace { // Tracks number of times a tweak has been offered. -static constexpr trace::Metric - TweakAvailable("tweak_available", trace::Metric::Counter, "tweak_id"); +static constexpr trace::Metric TweakAvailable( + "tweak_available", trace::Metric::Counter, "tweak_id"); // Update the FileIndex with new ASTs and plumb the diagnostics responses. struct UpdateIndexCallbacks : public ParsingCallbacks { @@ -872,42 +872,29 @@ void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve, } void ClangdServer::superTypes( - PathRef File, const TypeHierarchyItem &Item, + const TypeHierarchyItem &Item, Callback>> CB) { - auto Action = [File = File.str(), Item, CB = std::move(CB), - this](llvm::Expected InpAST) mutable { - if (!InpAST) - return CB(InpAST.takeError()); - CB(clangd::superTypes(Item, Index, InpAST->AST)); - }; - WorkScheduler->runWithAST("superTypes Calls", File, std::move(Action)); + WorkScheduler->run("typeHierarchy/superTypes", /*Path=*/"", + [=, CB = std::move(CB)]() mutable { + CB(clangd::superTypes(Item, Index)); + }); } -void ClangdServer::subTypes(PathRef File, const TypeHierarchyItem &Item, +void ClangdServer::subTypes(const TypeHierarchyItem &Item, Callback> CB) { - - auto Action = [File = File.str(), Item, CB = std::move(CB), - this](llvm::Expected InpAST) mutable { - if (!InpAST) - return CB(InpAST.takeError()); - CB(clangd::subTypes(Item, Index, InpAST->AST)); - }; - WorkScheduler->runWithAST("subTypes Calls", File, std::move(Action)); + WorkScheduler->run( + "typeHierarchy/subTypes", /*Path=*/"", + [=, CB = std::move(CB)]() mutable { CB(clangd::subTypes(Item, Index)); }); } void ClangdServer::resolveTypeHierarchy( - PathRef File, TypeHierarchyItem Item, int Resolve, - TypeHierarchyDirection Direction, + TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction, Callback> CB) { - auto Action = [Item = std::move(Item), Resolve, Direction, CB = std::move(CB), - this](llvm::Expected InpAST) mutable { - if (!InpAST) - return CB(InpAST.takeError()); - clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index, InpAST->AST); - CB(Item); - }; - WorkScheduler->runWithAST("resolveTypeHierarchy Calls", File, - std::move(Action)); + WorkScheduler->run( + "Resolve Type Hierarchy", "", [=, CB = std::move(CB)]() mutable { + clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index); + CB(Item); + }); } void ClangdServer::prepareCallHierarchy( @@ -922,15 +909,12 @@ void ClangdServer::prepareCallHierarchy( } void ClangdServer::incomingCalls( - PathRef File, const CallHierarchyItem &Item, + const CallHierarchyItem &Item, Callback> CB) { - auto Action = [Item, CB = std::move(CB), - this](llvm::Expected InpAST) mutable { - if (!InpAST) - return CB(InpAST.takeError()); - CB(clangd::incomingCalls(Item, Index, InpAST->AST)); - }; - WorkScheduler->runWithAST("Incoming Calls", File, std::move(Action)); + WorkScheduler->run("Incoming Calls", "", + [CB = std::move(CB), Item, this]() mutable { + CB(clangd::incomingCalls(Item, Index)); + }); } void ClangdServer::inlayHints(PathRef File, std::optional RestrictRange, @@ -945,15 +929,12 @@ void ClangdServer::inlayHints(PathRef File, std::optional RestrictRange, } void ClangdServer::outgoingCalls( - PathRef File, const CallHierarchyItem &Item, + const CallHierarchyItem &Item, Callback> CB) { - auto Action = [Item, CB = std::move(CB), - this](llvm::Expected InpAST) mutable { - if (!InpAST) - return CB(InpAST.takeError()); - CB(clangd::outgoingCalls(Item, Index, InpAST->AST)); - }; - WorkScheduler->runWithAST("Outgoing Calls", File, std::move(Action)); + WorkScheduler->run("Outgoing Calls", "", + [CB = std::move(CB), Item, this]() mutable { + CB(clangd::outgoingCalls(Item, Index)); + }); } void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index c582a96691deb..3ffaf67553dce 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -283,14 +283,14 @@ class ClangdServer { TypeHierarchyDirection Direction, Callback> CB); /// Get direct parents of a type hierarchy item. - void superTypes(PathRef File, const TypeHierarchyItem &Item, + void superTypes(const TypeHierarchyItem &Item, Callback>> CB); /// Get direct children of a type hierarchy item. - void subTypes(PathRef File, const TypeHierarchyItem &Item, + void subTypes(const TypeHierarchyItem &Item, Callback> CB); /// Resolve type hierarchy item in the given direction. - void resolveTypeHierarchy(PathRef File, TypeHierarchyItem Item, int Resolve, + void resolveTypeHierarchy(TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction, Callback> CB); @@ -299,11 +299,11 @@ class ClangdServer { Callback> CB); /// Resolve incoming calls for a given call hierarchy item. - void incomingCalls(PathRef File, const CallHierarchyItem &Item, - Callback> CB); + void incomingCalls(const CallHierarchyItem &Item, + Callback>); /// Resolve outgoing calls for a given call hierarchy item. - void outgoingCalls(PathRef File, const CallHierarchyItem &Item, + void outgoingCalls(const CallHierarchyItem &Item, Callback>); /// Resolve inlay hints for a given document. diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 7d1b06db64e2b..778ac70584094 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -209,7 +209,7 @@ bool fromJSON(const llvm::json::Value &Params, ChangeAnnotation &R, O.map("needsConfirmation", R.needsConfirmation) && O.mapOptional("description", R.description); } -llvm::json::Value toJSON(const ChangeAnnotation &CA) { +llvm::json::Value toJSON(const ChangeAnnotation & CA) { llvm::json::Object Result{{"label", CA.label}}; if (CA.needsConfirmation) Result["needsConfirmation"] = *CA.needsConfirmation; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 95ab9d891837d..134b6a957aacc 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1934,25 +1934,16 @@ const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, static void fillSubTypes(const SymbolID &ID, std::vector &SubTypes, - const SymbolIndex *Index, int Levels, PathRef TUPath, - const ParsedAST &AST) { + const SymbolIndex *Index, int Levels, PathRef TUPath) { RelationsRequest Req; Req.Subjects.insert(ID); Req.Predicate = RelationKind::BaseOf; - Index->relations(Req, [&Levels, &Index, &SubTypes, &TUPath, - &AST](const SymbolID &Subject, const Symbol &Object) { - std::optional ChildSym; - - if (auto *ND = getNamedDeclFromSymbol(Object, AST)) { - ChildSym = declToTypeHierarchyItem(*ND, AST.tuPath()); - } else { - ChildSym = symbolToTypeHierarchyItem(Object, TUPath); - } - if (ChildSym) { + Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) { + if (std::optional ChildSym = + symbolToTypeHierarchyItem(Object, TUPath)) { if (Levels > 1) { ChildSym->children.emplace(); - fillSubTypes(Object.ID, *ChildSym->children, Index, Levels - 1, TUPath, - AST); + fillSubTypes(Object.ID, *ChildSym->children, Index, Levels - 1, TUPath); } SubTypes.emplace_back(std::move(*ChildSym)); } @@ -2175,10 +2166,10 @@ static QualType typeForNode(const ASTContext &Ctx, const HeuristicResolver *H, return QualType(); } -// Given a type targeted by the cursor, return one or more types that are more -// interesting to target. -static void unwrapFindType(QualType T, const HeuristicResolver *H, - llvm::SmallVector &Out) { +// Given a type targeted by the cursor, return one or more types that are more interesting +// to target. +static void unwrapFindType( + QualType T, const HeuristicResolver* H, llvm::SmallVector& Out) { if (T.isNull()) return; @@ -2217,8 +2208,8 @@ static void unwrapFindType(QualType T, const HeuristicResolver *H, } // Convenience overload, to allow calling this without the out-parameter -static llvm::SmallVector unwrapFindType(QualType T, - const HeuristicResolver *H) { +static llvm::SmallVector unwrapFindType( + QualType T, const HeuristicResolver* H) { llvm::SmallVector Result; unwrapFindType(T, H, Result); return Result; @@ -2240,9 +2231,9 @@ std::vector findType(ParsedAST &AST, Position Pos, std::vector LocatedSymbols; // NOTE: unwrapFindType might return duplicates for something like - // unique_ptr>. Let's *not* remove them, because it gives you - // some information about the type you may have not known before (since - // unique_ptr> != unique_ptr). + // unique_ptr>. Let's *not* remove them, because it gives you some + // information about the type you may have not known before + // (since unique_ptr> != unique_ptr). for (const QualType &Type : unwrapFindType( typeForNode(AST.getASTContext(), AST.getHeuristicResolver(), N), AST.getHeuristicResolver())) @@ -2335,8 +2326,7 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, if (Index) { if (auto ID = getSymbolID(CXXRD)) - fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath, - AST); + fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath); } } Results.emplace_back(std::move(*Result)); @@ -2346,9 +2336,10 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, } std::optional> -superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index, - const ParsedAST &AST) { +superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) { std::vector Results; + if (!Index) + return Results; if (!Item.data.parents) return std::nullopt; if (Item.data.parents->empty()) @@ -2359,14 +2350,8 @@ superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index, Req.IDs.insert(Parent.symbolID); IDToData[Parent.symbolID] = &Parent; } - Index->lookup(Req, [&Item, &Results, &IDToData, &AST](const Symbol &S) { - std::optional THI; - if (auto *ND = getNamedDeclFromSymbol(S, AST)) { - THI = declToTypeHierarchyItem(*ND, AST.tuPath()); - } else { - THI = symbolToTypeHierarchyItem(S, Item.uri.file()); - } - if (THI) { + Index->lookup(Req, [&Item, &Results, &IDToData](const Symbol &S) { + if (auto THI = symbolToTypeHierarchyItem(S, Item.uri.file())) { THI->data = *IDToData.lookup(S.ID); Results.emplace_back(std::move(*THI)); } @@ -2375,10 +2360,9 @@ superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index, } std::vector subTypes(const TypeHierarchyItem &Item, - const SymbolIndex *Index, - const ParsedAST &AST) { + const SymbolIndex *Index) { std::vector Results; - fillSubTypes(Item.data.symbolID, Results, Index, 1, Item.uri.file(), AST); + fillSubTypes(Item.data.symbolID, Results, Index, 1, Item.uri.file()); for (auto &ChildSym : Results) ChildSym.data.parents = {Item.data}; return Results; @@ -2386,7 +2370,7 @@ std::vector subTypes(const TypeHierarchyItem &Item, void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, TypeHierarchyDirection Direction, - const SymbolIndex *Index, const ParsedAST &AST) { + const SymbolIndex *Index) { // We only support typeHierarchy/resolve for children, because for parents // we ignore ResolveLevels and return all levels of parents eagerly. if (!Index || Direction == TypeHierarchyDirection::Parents || @@ -2395,7 +2379,7 @@ void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, Item.children.emplace(); fillSubTypes(Item.data.symbolID, *Item.children, Index, ResolveLevels, - Item.uri.file(), AST); + Item.uri.file()); } std::vector @@ -2425,10 +2409,8 @@ prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath) { } std::vector -incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, - const ParsedAST &AST) { +incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { std::vector Results; - if (!Index || Item.data.empty()) return Results; auto ID = SymbolID::fromStr(Item.data); @@ -2472,14 +2454,7 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, Index->lookup(ContainerLookup, [&](const Symbol &Caller) { auto It = CallsIn.find(Caller.ID); assert(It != CallsIn.end()); - - std::optional CHI; - if (auto *ND = getNamedDeclFromSymbol(Caller, AST)) { - CHI = declToCallHierarchyItem(*ND, AST.tuPath()); - } else { - CHI = symbolToCallHierarchyItem(Caller, Item.uri.file()); - } - if (CHI) { + if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) { std::vector FromRanges; for (const Location &L : It->second) { if (L.uri != CHI->uri) { @@ -2516,8 +2491,7 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, } std::vector -outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, - const ParsedAST &AST) { +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { std::vector Results; if (!Index || Item.data.empty()) return Results; @@ -2563,16 +2537,7 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, auto It = CallsOut.find(Callee.ID); assert(It != CallsOut.end()); - - std::optional CHI; - - if (auto *ND = getNamedDeclFromSymbol(Callee, AST)) { - CHI = declToCallHierarchyItem(*ND, AST.tuPath()); - } else { - CHI = symbolToCallHierarchyItem(Callee, Item.uri.file()); - } - - if (CHI) { + if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file())) { std::vector FromRanges; for (const Location &L : It->second) { if (L.uri != Item.uri) { diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index f2652b5f274db..247e52314c3f9 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -132,31 +132,26 @@ std::vector getTypeHierarchy( const SymbolIndex *Index = nullptr, PathRef TUPath = PathRef{}); /// Returns direct parents of a TypeHierarchyItem using SymbolIDs stored inside -/// the item or using the AST. +/// the item. std::optional> -superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index, - const ParsedAST &AST); - +superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index); /// Returns direct children of a TypeHierarchyItem. std::vector subTypes(const TypeHierarchyItem &Item, - const SymbolIndex *Index, - const ParsedAST &AST); + const SymbolIndex *Index); void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels, TypeHierarchyDirection Direction, - const SymbolIndex *Index, const ParsedAST &AST); + const SymbolIndex *Index); /// Get call hierarchy information at \p Pos. std::vector prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath); std::vector -incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, - const ParsedAST &AST); +incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); std::vector -outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, - const ParsedAST &AST); +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); /// Returns all decls that are referenced in the \p FD except local symbols. llvm::DenseSet getNonLocalDeclRefs(ParsedAST &AST, diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index cd6fd53d310a9..87561f3268f0a 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -429,7 +429,8 @@ class SymbolCollector::HeaderFileURICache { CachePathToFrameworkSpelling.erase(Res.first); return std::nullopt; } - if (auto UmbrellaSpelling = getFrameworkUmbrellaSpelling(HS, *HeaderPath)) { + if (auto UmbrellaSpelling = + getFrameworkUmbrellaSpelling(HS, *HeaderPath)) { *CachedHeaderSpelling = *UmbrellaSpelling; return llvm::StringRef(*CachedHeaderSpelling); } @@ -926,7 +927,7 @@ llvm::StringRef getStdHeader(const Symbol *S, const LangOptions &LangOpts) { tooling::stdlib::Lang Lang = tooling::stdlib::Lang::CXX; if (LangOpts.C11) Lang = tooling::stdlib::Lang::C; - else if (!LangOpts.CPlusPlus) + else if(!LangOpts.CPlusPlus) return ""; if (S->Scope == "std::" && S->Name == "move") { @@ -1046,7 +1047,8 @@ void SymbolCollector::finish() { // depending on the translation unit. else if (tooling::isSelfContainedHeader(H.physical(), SM, PP->getHeaderSearchInfo())) - SpellingIt->second = HeaderFileURIs->toURI(H.physical()); + SpellingIt->second = + HeaderFileURIs->toURI(H.physical()); } else { SpellingIt->second = include_cleaner::spellHeader( {H, PP->getHeaderSearchInfo(), diff --git a/clang-tools-extra/clangd/test/type-hierarchy-ext.test b/clang-tools-extra/clangd/test/type-hierarchy-ext.test index d635b199d002c..983c7538088bf 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy-ext.test +++ b/clang-tools-extra/clangd/test/type-hierarchy-ext.test @@ -17,11 +17,11 @@ # CHECK-NEXT: "name": "Child3", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 25, +# CHECK-NEXT: "character": 13, # CHECK-NEXT: "line": 3 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, +# CHECK-NEXT: "character": 7, # CHECK-NEXT: "line": 3 # CHECK-NEXT: } # CHECK-NEXT: }, @@ -162,11 +162,11 @@ # CHECK-NEXT: "name": "Child4", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 25, +# CHECK-NEXT: "character": 13, # CHECK-NEXT: "line": 4 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, +# CHECK-NEXT: "character": 7, # CHECK-NEXT: "line": 4 # CHECK-NEXT: } # CHECK-NEXT: }, diff --git a/clang-tools-extra/clangd/test/type-hierarchy.test b/clang-tools-extra/clangd/test/type-hierarchy.test index c7b8a16ae51e2..c6079bc88662a 100644 --- a/clang-tools-extra/clangd/test/type-hierarchy.test +++ b/clang-tools-extra/clangd/test/type-hierarchy.test @@ -72,11 +72,11 @@ # CHECK-NEXT: "name": "Child1", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 25, +# CHECK-NEXT: "character": 13, # CHECK-NEXT: "line": 1 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, +# CHECK-NEXT: "character": 7, # CHECK-NEXT: "line": 1 # CHECK-NEXT: } # CHECK-NEXT: }, @@ -127,11 +127,11 @@ # CHECK-NEXT: "name": "Child3", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 25, +# CHECK-NEXT: "character": 13, # CHECK-NEXT: "line": 3 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, +# CHECK-NEXT: "character": 7, # CHECK-NEXT: "line": 3 # CHECK-NEXT: } # CHECK-NEXT: }, diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index fb911ba07354a..b646ac4ce8515 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -95,12 +95,12 @@ TEST(CallHierarchy, IncomingOneFileCpp) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT( IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("caller1"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); ASSERT_THAT( IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("caller2"), withDetail("caller2"))), @@ -109,13 +109,13 @@ TEST(CallHierarchy, IncomingOneFileCpp) { AllOf(from(AllOf(withName("caller3"), withDetail("caller3"))), iFromRanges(Source.range("Caller1C"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); ASSERT_THAT( IncomingLevel3, ElementsAre(AllOf(from(AllOf(withName("caller3"), withDetail("caller3"))), iFromRanges(Source.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); EXPECT_THAT(IncomingLevel4, IsEmpty()); } @@ -143,12 +143,12 @@ TEST(CallHierarchy, IncomingOneFileObjC) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("MyClass::caller1"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); ASSERT_THAT(IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("caller2"), withDetail("MyClass::caller2"))), @@ -158,13 +158,13 @@ TEST(CallHierarchy, IncomingOneFileObjC) { withDetail("MyClass::caller3"))), iFromRanges(Source.range("Caller1C"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(AllOf(withName("caller3"), withDetail("MyClass::caller3"))), iFromRanges(Source.range("Caller2"))))); - auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get(), AST); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); EXPECT_THAT(IncomingLevel4, IsEmpty()); } @@ -190,18 +190,18 @@ TEST(CallHierarchy, IncomingIncludeOverrides) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("Func"), withDetail("Implementation::Func"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); ASSERT_THAT( IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("Test"), withDetail("Test"))), iFromRanges(Source.range("FuncCall"))))); - auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get(), AST); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); EXPECT_THAT(IncomingLevel3, IsEmpty()); } @@ -227,13 +227,13 @@ TEST(CallHierarchy, MainFileOnlyRef) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT( IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("caller1"))), iFromRanges(Source.range("Callee"))))); - auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); EXPECT_THAT( IncomingLevel2, ElementsAre(AllOf(from(AllOf(withName("caller2"), withDetail("caller2"))), @@ -262,7 +262,7 @@ TEST(CallHierarchy, IncomingQualified) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("Waldo::find"))); - auto Incoming = incomingCalls(Items[0], Index.get(), AST); + auto Incoming = incomingCalls(Items[0], Index.get()); EXPECT_THAT( Incoming, ElementsAre( @@ -302,29 +302,29 @@ TEST(CallHierarchy, OutgoingOneFile) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("caller3"))); - auto OugoingLevel1 = outgoingCalls(Items[0], Index.get(), AST); + auto OugoingLevel1 = outgoingCalls(Items[0], Index.get()); ASSERT_THAT( OugoingLevel1, - ElementsAre(AllOf(to(AllOf(withName("Foo::caller1"), - withDetail("ns::Foo::caller1"))), - oFromRanges(Source.range("Caller1C"))), - AllOf(to(AllOf(withName("caller2"), withDetail("caller2"))), - oFromRanges(Source.range("Caller2"))))); + ElementsAre( + AllOf(to(AllOf(withName("caller1"), withDetail("ns::Foo::caller1"))), + oFromRanges(Source.range("Caller1C"))), + AllOf(to(AllOf(withName("caller2"), withDetail("caller2"))), + oFromRanges(Source.range("Caller2"))))); - auto OutgoingLevel2 = outgoingCalls(OugoingLevel1[1].to, Index.get(), AST); + auto OutgoingLevel2 = outgoingCalls(OugoingLevel1[1].to, Index.get()); ASSERT_THAT( OutgoingLevel2, ElementsAre(AllOf( - to(AllOf(withName("Foo::caller1"), withDetail("ns::Foo::caller1"))), + to(AllOf(withName("caller1"), withDetail("ns::Foo::caller1"))), oFromRanges(Source.range("Caller1A"), Source.range("Caller1B"))))); - auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get(), AST); + auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get()); ASSERT_THAT( OutgoingLevel3, ElementsAre(AllOf(to(AllOf(withName("callee"), withDetail("callee"))), oFromRanges(Source.range("Callee"))))); - auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get(), AST); + auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get()); EXPECT_THAT(OutgoingLevel4, IsEmpty()); } @@ -402,14 +402,13 @@ TEST(CallHierarchy, MultiFileCpp) { std::vector Items = prepareCallHierarchy(AST, Pos, TUPath); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(AllOf(withName("caller1"), withDetail("nsa::caller1"))), iFromRanges(Caller1C.range())))); - auto IncomingLevel2 = - incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); ASSERT_THAT( IncomingLevel2, ElementsAre( @@ -418,15 +417,13 @@ TEST(CallHierarchy, MultiFileCpp) { AllOf(from(AllOf(withName("caller3"), withDetail("nsa::caller3"))), iFromRanges(Caller3C.range("Caller1"))))); - auto IncomingLevel3 = - incomingCalls(IncomingLevel2[0].from, Index.get(), AST); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(AllOf(withName("caller3"), withDetail("nsa::caller3"))), iFromRanges(Caller3C.range("Caller2"))))); - auto IncomingLevel4 = - incomingCalls(IncomingLevel3[0].from, Index.get(), AST); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); EXPECT_THAT(IncomingLevel4, IsEmpty()); }; @@ -439,7 +436,7 @@ TEST(CallHierarchy, MultiFileCpp) { ElementsAre(AllOf( withName("caller3"), withFile(testPath(IsDeclaration ? "caller3.hh" : "caller3.cc"))))); - auto OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), AST); + auto OutgoingLevel1 = outgoingCalls(Items[0], Index.get()); ASSERT_THAT( OutgoingLevel1, // fromRanges are interpreted in the context of Items[0]'s file. @@ -453,19 +450,19 @@ TEST(CallHierarchy, MultiFileCpp) { IsDeclaration ? oFromRanges() : oFromRanges(Caller3C.range("Caller2"))))); - auto OutgoingLevel2 = outgoingCalls(OutgoingLevel1[1].to, Index.get(), AST); + auto OutgoingLevel2 = outgoingCalls(OutgoingLevel1[1].to, Index.get()); ASSERT_THAT(OutgoingLevel2, ElementsAre(AllOf( to(AllOf(withName("caller1"), withDetail("nsa::caller1"))), oFromRanges(Caller2C.range("A"), Caller2C.range("B"))))); - auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get(), AST); + auto OutgoingLevel3 = outgoingCalls(OutgoingLevel2[0].to, Index.get()); ASSERT_THAT( OutgoingLevel3, ElementsAre(AllOf(to(AllOf(withName("callee"), withDetail("callee"))), oFromRanges(Caller1C.range())))); - auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get(), AST); + auto OutgoingLevel4 = outgoingCalls(OutgoingLevel3[0].to, Index.get()); EXPECT_THAT(OutgoingLevel4, IsEmpty()); }; @@ -562,13 +559,12 @@ TEST(CallHierarchy, IncomingMultiFileObjC) { std::vector Items = prepareCallHierarchy(AST, Pos, TUPath); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller1")), iFromRanges(Caller1C.range())))); - auto IncomingLevel2 = - incomingCalls(IncomingLevel1[0].from, Index.get(), AST); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); ASSERT_THAT(IncomingLevel2, ElementsAre(AllOf(from(withName("caller2")), iFromRanges(Caller2C.range("A"), @@ -576,14 +572,12 @@ TEST(CallHierarchy, IncomingMultiFileObjC) { AllOf(from(withName("caller3")), iFromRanges(Caller3C.range("Caller1"))))); - auto IncomingLevel3 = - incomingCalls(IncomingLevel2[0].from, Index.get(), AST); + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); ASSERT_THAT(IncomingLevel3, ElementsAre(AllOf(from(withName("caller3")), iFromRanges(Caller3C.range("Caller2"))))); - auto IncomingLevel4 = - incomingCalls(IncomingLevel3[0].from, Index.get(), AST); + auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get()); EXPECT_THAT(IncomingLevel4, IsEmpty()); }; @@ -628,7 +622,7 @@ TEST(CallHierarchy, CallInLocalVarDecl) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto Incoming = incomingCalls(Items[0], Index.get(), AST); + auto Incoming = incomingCalls(Items[0], Index.get()); ASSERT_THAT(Incoming, ElementsAre(AllOf(from(withName("caller1")), iFromRanges(Source.range("call1"))), AllOf(from(withName("caller2")), @@ -655,7 +649,7 @@ TEST(CallHierarchy, HierarchyOnField) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("var1"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("Callee"))))); @@ -676,7 +670,7 @@ TEST(CallHierarchy, HierarchyOnVar) { std::vector Items = prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("var"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("Callee"))))); @@ -698,14 +692,14 @@ TEST(CallHierarchy, HierarchyOnEnumConstant) { std::vector Items = prepareCallHierarchy(AST, Source.point("Heads"), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("heads"))); - auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("CallerH"))))); Items = prepareCallHierarchy(AST, Source.point("Tails"), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("tails"))); - IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + IncomingLevel1 = incomingCalls(Items[0], Index.get()); ASSERT_THAT(IncomingLevel1, ElementsAre(AllOf(from(withName("caller")), iFromRanges(Source.range("CallerT"))))); @@ -730,7 +724,7 @@ TEST(CallHierarchy, CallInDifferentFileThanCaller) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto Incoming = incomingCalls(Items[0], Index.get(), AST); + auto Incoming = incomingCalls(Items[0], Index.get()); // The only call site is in the source file, which is a different file from // the declaration of the function containing the call, which is in the @@ -758,7 +752,7 @@ TEST(CallHierarchy, IncomingCalls) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); - auto Incoming = incomingCalls(Items[0], Index.get(), AST); + auto Incoming = incomingCalls(Items[0], Index.get()); EXPECT_THAT( Incoming, UnorderedElementsAre(AllOf(from( @@ -784,7 +778,7 @@ TEST(CallHierarchy, OutgoingCalls) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("caller"))); - auto Outgoing = outgoingCalls(Items[0], Index.get(), AST); + auto Outgoing = outgoingCalls(Items[0], Index.get()); EXPECT_THAT(Outgoing, UnorderedElementsAre(AllOf( to(AllOf(withName("callee"), withSymbolTags(SymbolTag::Declaration, diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp index 21adbb6d0d3a3..754063ede2724 100644 --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -762,7 +762,7 @@ struct Child2b : Child1 {}; parentsNotResolved(), childrenNotResolved())))); resolveTypeHierarchy((*Result.front().children)[0], /*ResolveLevels=*/1, - TypeHierarchyDirection::Children, Index.get(), AST); + TypeHierarchyDirection::Children, Index.get()); EXPECT_THAT( (*Result.front().children)[0], @@ -776,10 +776,10 @@ struct Child2b : Child1 {}; TEST(Standard, SubTypes) { Annotations Source(R"cpp( - struct Pare^nt1 {}; - struct Parent2 {}; - struct Child final : Parent1, Parent2 {}; - )cpp"); +struct Pare^nt1 {}; +struct Parent2 {}; +struct Child final: Parent1, Parent2 {}; +)cpp"); TestTU TU = TestTU::withCode(Source.code()); auto AST = TU.build(); @@ -789,7 +789,7 @@ TEST(Standard, SubTypes) { TypeHierarchyDirection::Children, Index.get(), testPath(TU.Filename)); ASSERT_THAT(Result, SizeIs(1)); - auto Children = subTypes(Result.front(), Index.get(), AST); + auto Children = subTypes(Result.front(), Index.get()); // Make sure parents are populated when getting children. // FIXME: This is partial. @@ -805,9 +805,9 @@ TEST(Standard, SubTypes) { TEST(Standard, SuperTypes) { Annotations Source(R"cpp( - struct Parent {}; - struct Chil^d : Parent {}; - )cpp"); +struct Parent {}; +struct Chil^d : Parent {}; +)cpp"); TestTU TU = TestTU::withCode(Source.code()); auto AST = TU.build(); @@ -817,7 +817,7 @@ TEST(Standard, SuperTypes) { TypeHierarchyDirection::Children, Index.get(), testPath(TU.Filename)); ASSERT_THAT(Result, SizeIs(1)); - auto Parents = superTypes(Result.front(), Index.get(), AST); + auto Parents = superTypes(Result.front(), Index.get()); EXPECT_THAT(Parents, Optional(UnorderedElementsAre(AllOf( From 66529e55481f7c6e039f3ffa2f36895c50e3559a Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Fri, 20 Mar 2026 13:00:33 +0100 Subject: [PATCH 14/29] Feat: serialize/de-serialize symbol tags. - Bump index version to 21 - Updated sample index - Added tests --- .../clangd/index/Serialization.cpp | 4 +- .../index-serialization/Inputs/sample.idx | Bin 470 -> 488 bytes .../clangd/unittests/SerializationTests.cpp | 109 ++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/index/Serialization.cpp b/clang-tools-extra/clangd/index/Serialization.cpp index f03839599612c..764ec7903aecb 100644 --- a/clang-tools-extra/clangd/index/Serialization.cpp +++ b/clang-tools-extra/clangd/index/Serialization.cpp @@ -328,6 +328,7 @@ void writeSymbol(const Symbol &Sym, const StringTableOut &Strings, writeVar(Strings.index(Sym.Documentation), OS); writeVar(Strings.index(Sym.ReturnType), OS); writeVar(Strings.index(Sym.Type), OS); + writeVar(Sym.Tags, OS); auto WriteInclude = [&](const Symbol::IncludeHeaderWithReferences &Include) { writeVar(Strings.index(Include.IncludeHeader), OS); @@ -357,6 +358,7 @@ Symbol readSymbol(Reader &Data, llvm::ArrayRef Strings, Sym.Documentation = Data.consumeString(Strings); Sym.ReturnType = Data.consumeString(Strings); Sym.Type = Data.consumeString(Strings); + Sym.Tags = Data.consumeVar(); if (!Data.consumeSize(Sym.IncludeHeaders)) return Sym; for (auto &I : Sym.IncludeHeaders) { @@ -457,7 +459,7 @@ readCompileCommand(Reader CmdReader, llvm::ArrayRef Strings) { // The current versioning scheme is simple - non-current versions are rejected. // If you make a breaking change, bump this version number to invalidate stored // data. Later we may want to support some backward compatibility. -constexpr static uint32_t Version = 20; +constexpr static uint32_t Version = 21; llvm::Expected readRIFF(llvm::StringRef Data, SymbolOrigin Origin) { diff --git a/clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx b/clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx index 6368e7145b1e4d628708f40d684bc8db1ef7f94d..6a2ce7a87925fd3e5d22a5366bfdb19ee26275e6 100644 GIT binary patch delta 333 zcmcb{{DQeY$kWa30V4y0bBbq0ZfZ#)3j+g#C=eHy6lG2T(sDq#iaAGn5Aq%|;9)TK z68HG~elmN|af8lxl?^3LoQ?l!PtMIoEsk076etobZG-$@r-Oo%fHZ<>j z9h84`koekKH-dK#g(~9-9U#`RxAGeIVV(KkcpLv zft`(=jT=k=Wk6s-!zMchUwj90kwmo?% zI}=^bvzu~MpFHvawlA<>nW*y(YvVP^a?b$Nxu-}eEes#mP-LH;rHSA*5Ey=PLT3B5udXL~XME?nCamISSJ}7jl-Z?+vh`}slPyI~mh`yo zRhZ6S71$VgtIST~+A))**ZxIB9~Ng(iNC>6T$!6x2Xs_rwc^j8b3*k6CLR=xV<=q6 zywGrsO$-|o(5>t&>?~Yh0;moQoVOgk^0qyBDH{_r&^yfR%v@lCQIdgyje&uYgVnT9 eV$sZA$$tV&tSk&1Ec`6OV1j`Iq(Sym1.Flags), 129); EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion); EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated); + // Tags: Deprecated (1<<1=2) | Static (1<<8=256) = 258 + EXPECT_EQ(Sym1.Tags, 258u); + EXPECT_TRUE(Sym1.Tags & (1u << static_cast(SymbolTag::Deprecated))); + EXPECT_TRUE(Sym1.Tags & (1u << static_cast(SymbolTag::Static))); EXPECT_THAT( Sym1.IncludeHeaders, UnorderedElementsAre( @@ -176,6 +181,7 @@ TEST(SerializationTest, YAMLConversions) { "file:///path/bar.h"); EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion); EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated); + EXPECT_EQ(Sym2.Tags, 0u); // no Tags in YAML → default zero ASSERT_TRUE(bool(ParsedYAML->Refs)); EXPECT_THAT( @@ -292,6 +298,109 @@ TEST(SerializationTest, SrcsTest) { } } +// Verify that Symbol::Tags survive a full YAML -> RIFF -> read roundtrip. +// This exercises both writeSymbol (new Tags field) and readSymbol. +TEST(SerializationTest, TagsRoundTrip) { + // YAML fixture with a single symbol carrying non-zero Tags. + // Deprecated (SymbolTag=1) -> bit 1 -> 1<<1 = 2 + // Static (SymbolTag=8) -> bit 8 -> 1<<8 = 256 + // Combined: 258 + const char *TaggedYAML = R"( +--- +!Symbol +ID: AABBCCDDEEFF0011 +Name: 'Tagged' +Scope: 'ns::' +SymInfo: + Kind: Function + Lang: Cpp +CanonicalDeclaration: + FileURI: file:///tmp/tagged.h + Start: + Line: 0 + Column: 0 + End: + Line: 0 + Column: 1 +Flags: 1 +Tags: 258 +... +)"; + const SymbolID TaggedID = cantFail(SymbolID::fromStr("AABBCCDDEEFF0011")); + constexpr SymbolTags ExpectedTags = + (1u << static_cast(SymbolTag::Deprecated)) | // bit 1 = 2 + (1u << static_cast(SymbolTag::Static)); // bit 8 = 256 + static_assert(ExpectedTags == 258u, "bitmask sanity check"); + + // ── Step 1: YAML deserialization ──────────────────────────────────────── + auto FromYAML = readIndexFile(TaggedYAML); + ASSERT_TRUE(bool(FromYAML)) << FromYAML.takeError(); + ASSERT_TRUE(bool(FromYAML->Symbols)); + { + auto It = FromYAML->Symbols->find(TaggedID); + ASSERT_NE(It, FromYAML->Symbols->end()) + << "symbol not found after YAML parse"; + EXPECT_EQ(It->Tags, ExpectedTags) + << "Tags lost during YAML deserialization"; + } + + // ── Step 2: RIFF serialization + deserialization ──────────────────────── + IndexFileOut Out(*FromYAML); + Out.Format = IndexFileFormat::RIFF; + std::string Serialized = llvm::to_string(Out); + + auto FromRIFF = readIndexFile(Serialized); + ASSERT_TRUE(bool(FromRIFF)) << FromRIFF.takeError(); + ASSERT_TRUE(bool(FromRIFF->Symbols)); + { + auto It = FromRIFF->Symbols->find(TaggedID); + ASSERT_NE(It, FromRIFF->Symbols->end()) + << "symbol not found after RIFF roundtrip"; + EXPECT_EQ(It->Tags, ExpectedTags) << "Tags lost during RIFF serialization"; + // Spot-check individual bits + EXPECT_TRUE(It->Tags & + (1u << static_cast(SymbolTag::Deprecated))); + EXPECT_TRUE(It->Tags & (1u << static_cast(SymbolTag::Static))); + EXPECT_FALSE(It->Tags & (1u << static_cast(SymbolTag::Abstract))); + } + + // ── Step 3: Symbol with Tags=0 must survive too ───────────────────────── + const char *UntaggedYAML = R"( +--- +!Symbol +ID: 0000000000000001 +Name: 'Untagged' +Scope: '' +SymInfo: + Kind: Variable + Lang: Cpp +CanonicalDeclaration: + FileURI: file:///tmp/untagged.h + Start: + Line: 0 + Column: 0 + End: + Line: 0 + Column: 1 +Flags: 1 +... +)"; + auto FromUntaggedYAML = readIndexFile(UntaggedYAML); + ASSERT_TRUE(bool(FromUntaggedYAML)) << FromUntaggedYAML.takeError(); + ASSERT_TRUE(bool(FromUntaggedYAML->Symbols)); + IndexFileOut Out2(*FromUntaggedYAML); + Out2.Format = IndexFileFormat::RIFF; + auto FromUntaggedRIFF = readIndexFile(llvm::to_string(Out2)); + ASSERT_TRUE(bool(FromUntaggedRIFF)) << FromUntaggedRIFF.takeError(); + ASSERT_TRUE(bool(FromUntaggedRIFF->Symbols)); + { + const SymbolID UntaggedID = cantFail(SymbolID::fromStr("0000000000000001")); + auto It = FromUntaggedRIFF->Symbols->find(UntaggedID); + ASSERT_NE(It, FromUntaggedRIFF->Symbols->end()); + EXPECT_EQ(It->Tags, 0u) << "Tags must be zero for untagged symbol"; + } +} + TEST(SerializationTest, CmdlTest) { auto In = readIndexFile(YAML); EXPECT_TRUE(bool(In)) << In.takeError(); From fad6461f1c83cac88b195c4642478a670403e55b Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 23 Mar 2026 14:05:00 +0100 Subject: [PATCH 15/29] Review: code review adjustments. - avoid returning empty vector - return either a filled vector or nullpt meaning there are no parents - removed obsolete function - texts rewritten correctly - tightening the check to < 32 --- clang-tools-extra/clangd/FindSymbols.cpp | 206 +++++++++--------- clang-tools-extra/clangd/FindSymbols.h | 4 +- clang-tools-extra/clangd/XRefs.cpp | 41 +--- clang-tools-extra/clangd/XRefs.h | 3 +- clang-tools-extra/clangd/index/Symbol.h | 2 +- .../clangd/index/YAMLSerialization.cpp | 2 +- .../clangd/unittests/FindSymbolsTests.cpp | 74 ------- .../clangd/unittests/SymbolCollectorTests.cpp | 43 ++++ 8 files changed, 155 insertions(+), 220 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 7d87885c5c60c..c909e077a1d67 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -120,17 +120,17 @@ bool isAbstract(const Decl *D) { } // Indicates whether declaration D is virtual in cases where D is a method. +// We want to treat a method as virtual if it is declared virtual, even if it +// is not implemented in this class, or if it overrides/implements a +// base-class method. This is because the "virtual" modifier is still relevant +// to the method's behavior and how it should be highlighted, even if it is +// not itself a virtual method in the strictest sense. For example, a method +// that overrides a virtual method from a base class is still considered +// virtual, even if it is not declared as such in the derived class. +// Similarly, a method that implements a pure virtual method from a base class +// is also considered virtual, even if it is not declared as such in the +// derived class. bool isVirtual(const Decl *D) { - // We want to treat a method as virtual if it is declared virtual, even if it - // is not implemented in this class, or if it overrides/implements a - // base-class method. This is because the "virtual" modifier is still relevant - // to the method's behavior and how it should be highlighted, even if it is - // not itself a virtual method in the strictest sense. For example, a method - // that overrides a virtual method from a base class is still considered - // virtual, even if it is not declared as such in the derived class. - // Similarly, a method that implements a pure virtual method from a base class - // is also considered virtual, even if it is not declared as such in the - // derived class. if (const auto *CMD = llvm::dyn_cast(D)) return CMD->isVirtual(); return false; @@ -148,61 +148,34 @@ bool isFinal(const Decl *D) { return false; } -// Indicates whether declaration D is a unique definition (as opposed to a -// declaration). -bool isUniqueDefinition(const NamedDecl *Decl) { - if (auto *Func = dyn_cast(Decl)) - return Func->isThisDeclarationADefinition(); - if (auto *Klass = dyn_cast(Decl)) - return Klass->isThisDeclarationADefinition(); - if (auto *Iface = dyn_cast(Decl)) - return Iface->isThisDeclarationADefinition(); - if (auto *Proto = dyn_cast(Decl)) - return Proto->isThisDeclarationADefinition(); - if (auto *Var = dyn_cast(Decl)) - return Var->isThisDeclarationADefinition(); - return isa(Decl) || - isa(Decl) || - isa(Decl) || isa(Decl) || - isa(Decl); -} -} // namespace - -SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return (1 << static_cast(ST)); -} - +// A method "overrides" if: +// 1. It overrides at least one method +// 2. At least one of the overridden methods is virtual (but NOT pure +// virtual) bool isOverrides(const NamedDecl *ND) { if (const auto *MD = llvm::dyn_cast(ND)) { - // A method "overrides" if: - // 1. It overrides at least one method - // 2. At least one of the overridden methods is virtual (but NOT pure - // virtual) - if (MD->size_overridden_methods() == 0) return false; - for (const auto Overridden : MD->overridden_methods()) { + for (const auto *Overridden : MD->overridden_methods()) { // Check if the overridden method is virtual but not pure virtual if (Overridden->isVirtual() && !Overridden->isPureVirtual()) return true; } - return false; } return false; } +// A method "implements" pure virtual methods from base classes if: +// 1. It overrides at least one method +// 2. It is NOT itself pure virtual (i.e., it has a concrete implementation) +// 3. ALL overridden methods are pure virtual bool isImplements(const NamedDecl *ND) { if (const auto *MD = llvm::dyn_cast(ND)) { - // A method "implements" pure virtual methods from base classes if: - // 1. It overrides at least one method - // 2. It is NOT itself pure virtual (i.e., it has a concrete implementation) - // 3. ALL overridden methods are pure virtual - if (MD->size_overridden_methods() == 0 || MD->isPureVirtual()) return false; - for (const auto Overridden : MD->overridden_methods()) { + for (const auto *Overridden : MD->overridden_methods()) { if (!Overridden->isPureVirtual()) return false; } @@ -211,63 +184,28 @@ bool isImplements(const NamedDecl *ND) { return false; } -SymbolTags computeSymbolTags(const NamedDecl &ND) { - SymbolTags Result = 0; - const auto IsDef = isUniqueDefinition(&ND); - - if (ND.isDeprecated()) - Result |= toSymbolTagBitmask(SymbolTag::Deprecated); - - if (isConst(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::ReadOnly); - - if (isStatic(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Static); - - if (isVirtual(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Virtual); - - if (isAbstract(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Abstract); - - if (isFinal(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Final); - - if (isOverrides(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Overrides); - - if (isImplements(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Implements); - - if (not isa(ND)) { - // Do not treat an UnresolvedUsingValueDecl as a declaration. - // It's more common to think of it as a reference to the - // underlying declaration. - Result |= toSymbolTagBitmask(SymbolTag::Declaration); - - if (IsDef) - Result |= toSymbolTagBitmask(SymbolTag::Definition); - } - - switch (ND.getAccess()) { - case AS_public: - Result |= toSymbolTagBitmask(SymbolTag::Public); - break; - case AS_protected: - Result |= toSymbolTagBitmask(SymbolTag::Protected); - break; - case AS_private: - Result |= toSymbolTagBitmask(SymbolTag::Private); - break; - default: - break; - } - - return Result; +// Indicates whether declaration D is a unique definition (as opposed to a +// declaration). +bool isUniqueDefinition(const NamedDecl *Decl) { + if (auto *Func = dyn_cast(Decl)) + return Func->isThisDeclarationADefinition(); + if (auto *Klass = dyn_cast(Decl)) + return Klass->isThisDeclarationADefinition(); + if (auto *Iface = dyn_cast(Decl)) + return Iface->isThisDeclarationADefinition(); + if (auto *Proto = dyn_cast(Decl)) + return Proto->isThisDeclarationADefinition(); + if (auto *Var = dyn_cast(Decl)) + return Var->isThisDeclarationADefinition(); + return isa(Decl) || + isa(Decl) || + isa(Decl) || isa(Decl) || + isa(Decl); } // Filter symbol tags based on the presence of other tags and the kind of -// symbol. This is needed to avoid redundant tags. +// symbol. This is needed to avoid redundant tags, e.g. final implies override, +// override implies virtual, etc. SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) { SymbolTags Result = ST; @@ -321,11 +259,71 @@ SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) { } return Result; } +} // namespace + +SymbolTags toSymbolTagBitmask(const SymbolTag ST) { + return (1 << static_cast(ST)); +} + +SymbolTags computeSymbolTags(const NamedDecl &ND) { + SymbolTags Result = 0; + const auto IsDef = isUniqueDefinition(&ND); + + if (ND.isDeprecated()) + Result |= toSymbolTagBitmask(SymbolTag::Deprecated); -std::vector expandTagBitmask(const SymbolTags symbolTags) { + if (isConst(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::ReadOnly); + + if (isStatic(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Static); + + if (isVirtual(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Virtual); + + if (isAbstract(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Abstract); + + if (isOverrides(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Overrides); + + if (isFinal(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Final); + + if (isImplements(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Implements); + + if (not isa(ND)) { + // Do not treat an UnresolvedUsingValueDecl as a declaration. + // It's more common to think of it as a reference to the + // underlying declaration. + Result |= toSymbolTagBitmask(SymbolTag::Declaration); + + if (IsDef) + Result |= toSymbolTagBitmask(SymbolTag::Definition); + } + + switch (ND.getAccess()) { + case AS_public: + Result |= toSymbolTagBitmask(SymbolTag::Public); + break; + case AS_protected: + Result |= toSymbolTagBitmask(SymbolTag::Protected); + break; + case AS_private: + Result |= toSymbolTagBitmask(SymbolTag::Private); + break; + default: + break; + } + + return Result; +} + +std::vector expandTagBitmask(const SymbolTags STGS) { std::vector Tags; - if (symbolTags == 0) + if (STGS == 0) return Tags; // No filtering required since this function is only used for Symbols from the @@ -339,7 +337,7 @@ std::vector expandTagBitmask(const SymbolTags symbolTags) { constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); for (unsigned I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); - if (symbolTags & toSymbolTagBitmask(ST)) + if (STGS & toSymbolTagBitmask(ST)) Tags.push_back(ST); } return Tags; diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index c13231fb644a0..1e34731ad336f 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -63,8 +63,8 @@ SymbolTags computeSymbolTags(const NamedDecl &ND); /// \p ND The declaration to get tags for. std::vector getSymbolTags(const NamedDecl &ND); -/// Returns the symbol tags for the given declaration as a bitmask. -std::vector expandTagBitmask(SymbolTags symbolTags); +/// Expands a SymbolTags bitmask into individual SymbolTag values. +std::vector expandTagBitmask(SymbolTags STGS); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 134b6a957aacc..72226e420480c 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1902,36 +1902,6 @@ symbolToCallHierarchyItem(const Symbol &S, PathRef TUPath) { return Result; } -// Tries to find a NamedDecl in the AST that matches the given Symbol. -// Returns nullptr if the symbol is not found in the current AST. -const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, - const ParsedAST &AST) { - // Try to convert the symbol to a location and find the decl at that location - auto SymLoc = symbolToLocation(Sym, AST.tuPath()); - if (!SymLoc) - return nullptr; - - // Check if the symbol location is in the main file - if (SymLoc->uri.file() != AST.tuPath()) - return nullptr; - - // Convert LSP position to source location - const auto &SM = AST.getSourceManager(); - auto CurLoc = sourceLocationInMainFile(SM, SymLoc->range.start); - if (!CurLoc) { - llvm::consumeError(CurLoc.takeError()); - return nullptr; - } - - // Get all decls at this location - auto Decls = getDeclAtPosition(const_cast(AST), *CurLoc, {}); - if (Decls.empty()) - return nullptr; - - // Return the first decl (usually the most specific one) - return Decls[0]; -} - static void fillSubTypes(const SymbolID &ID, std::vector &SubTypes, const SymbolIndex *Index, int Levels, PathRef TUPath) { @@ -2337,26 +2307,23 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, std::optional> superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) { - std::vector Results; - if (!Index) - return Results; - if (!Item.data.parents) + if (!Index || !Item.data.parents) return std::nullopt; - if (Item.data.parents->empty()) - return Results; LookupRequest Req; llvm::DenseMap IDToData; for (const auto &Parent : *Item.data.parents) { Req.IDs.insert(Parent.symbolID); IDToData[Parent.symbolID] = &Parent; } + std::vector Results; Index->lookup(Req, [&Item, &Results, &IDToData](const Symbol &S) { if (auto THI = symbolToTypeHierarchyItem(S, Item.uri.file())) { THI->data = *IDToData.lookup(S.ID); Results.emplace_back(std::move(*THI)); } }); - return Results; + return Results.empty() ? std::nullopt + : std::make_optional(std::move(Results)); } std::vector subTypes(const TypeHierarchyItem &Item, diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 247e52314c3f9..3cc35e5dfbd2c 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -132,7 +132,8 @@ std::vector getTypeHierarchy( const SymbolIndex *Index = nullptr, PathRef TUPath = PathRef{}); /// Returns direct parents of a TypeHierarchyItem using SymbolIDs stored inside -/// the item. +/// the item. Returns nullopt if the item does not have parents or if +/// the index is not provided. std::optional> superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index); /// Returns direct children of a TypeHierarchyItem. diff --git a/clang-tools-extra/clangd/index/Symbol.h b/clang-tools-extra/clangd/index/Symbol.h index 45e6bebd04faa..9bce1a1ece7e5 100644 --- a/clang-tools-extra/clangd/index/Symbol.h +++ b/clang-tools-extra/clangd/index/Symbol.h @@ -28,7 +28,7 @@ LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); /// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#symbolTag using SymbolTags = uint32_t; /// Ensure we have enough bits to represent all SymbolTag values. -static_assert(static_cast(SymbolTag::LastTag) <= 32, +static_assert(static_cast(SymbolTag::LastTag) < 32, "Too many SymbolTags to fit in uint32_t. Change to uint64_t if " "we ever have more than 32 tags."); diff --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp b/clang-tools-extra/clangd/index/YAMLSerialization.cpp index 6109761d2e8ed..4d6b5822ece72 100644 --- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp +++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp @@ -246,7 +246,7 @@ template <> struct MappingTraits { IO.mapOptional("Documentation", Sym.Documentation); IO.mapOptional("ReturnType", Sym.ReturnType); IO.mapOptional("Type", Sym.Type); - IO.mapOptional("Tags", Sym.Tags, clang::clangd::SymbolTags(0)); + IO.mapOptional("Tags", Sym.Tags); IO.mapOptional("IncludeHeaders", NIncludeHeaders->Headers); } }; diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index a14c32b25282c..63fb76e330d8f 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -7,10 +7,8 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" #include "FindSymbols.h" -#include "SyncAPI.h" #include "TestFS.h" #include "TestTU.h" -#include "index/FileIndex.h" #include "llvm/ADT/StringRef.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -1333,78 +1331,6 @@ TEST(DocumentSymbolsTest, SymbolTagsCompilation) { SymbolTag::Implements)))))); } -TEST(DocumentSymbolsTest, SymbolTagsWithIndexing) { - // Test that verifies symbol tags are correctly set when the AST is indexed - // through FileIndex, which triggers the full indexing path through - // SymbolCollector::addDeclaration where S.Tags = computeSymbolTags(ND) - TestTU TU; - Annotations Main(R"cpp( - class A { - public: - virtual ~A() = default; - virtual void f1() = 0; - void f2() const; - protected: - void f3(){} - private: - static void f4(){} - }; - - void A::f2() const {} - - class B final: public A { - public: - void f1() final {} - }; - )cpp"); - - TU.Code = Main.code().str(); - auto AST = TU.build(); - - // This path goes through: - // FileIndex::updateMain() -> indexMainDecls() -> indexSymbols() -> - // SymbolCollector -> addDeclaration() -> S.Tags = computeSymbolTags(ND) - - FileIndex Index{false}; - Index.updateMain(testPath(TU.Filename), AST); - // Verify that the index contains symbols with correct tags - // Note: We can't directly inspect Symbol.Tags from the index in this test, - // but the fact that updateMain() completes successfully demonstrates that: - // 1. SymbolCollector::addDeclaration() was called for each decl - // 2. computeSymbolTags() was executed and S.Tags was set - // 3. The full indexing pipeline works with our tag implementation - - auto Indexed = runFuzzyFind(Index, ""); - EXPECT_FALSE(Indexed.empty()); - - auto FindByQName = [&](llvm::StringRef QName) -> const Symbol * { - for (const auto &S : Indexed) { - if ((S.Scope + S.Name).str() == QName.str()) - return &S; - } - return nullptr; - }; - - const Symbol *A = FindByQName("A"); - ASSERT_TRUE(A); - EXPECT_THAT(expandTagBitmask(A->Tags), - UnorderedElementsAre(SymbolTag::Abstract, SymbolTag::Declaration, - SymbolTag::Definition)); - - const Symbol *B = FindByQName("B"); - ASSERT_TRUE(B); - EXPECT_THAT(expandTagBitmask(B->Tags), - UnorderedElementsAre(SymbolTag::Final, SymbolTag::Declaration, - SymbolTag::Definition)); - const Symbol *Bf1 = FindByQName("B::f1"); - ASSERT_TRUE(Bf1); - EXPECT_THAT(expandTagBitmask(Bf1->Tags), - UnorderedElementsAre(SymbolTag::Public, SymbolTag::Final, - SymbolTag::Virtual, SymbolTag::Declaration, - SymbolTag::Definition, - SymbolTag::Implements)); -} - } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp index 94116fca3cbb2..10059247aa7d7 100644 --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" +#include "FindSymbols.h" #include "TestFS.h" #include "TestTU.h" #include "URI.h" @@ -1335,6 +1336,48 @@ TEST_F(SymbolCollectorTest, OverrideRelationsMultipleInheritance) { OverriddenBy(CBar, DBar), OverriddenBy(CBaz, DBaz))); } +TEST_F(SymbolCollectorTest, SymbolTagsWithIndexing) { + // Test that verifies symbol tags are correctly set when the AST is indexed + // through FileIndex, which triggers the full indexing path through + // SymbolCollector::addDeclaration where S.Tags = computeSymbolTags(ND) + std::string Header = R"cpp( + class A { + public: + virtual ~A() = default; + virtual void f1() = 0; + void f2() const; + protected: + void f3(){} + private: + static void f4(){} + }; + + void A::f2() const {} + + class B final: public A { + public: + void f1() final {} + }; + )cpp"; + + runSymbolCollector(Header, /*Main=*/""); + const Symbol &A = findSymbol(Symbols, "A"); + EXPECT_THAT(expandTagBitmask(A.Tags), + UnorderedElementsAre(SymbolTag::Abstract, SymbolTag::Declaration, + SymbolTag::Definition)); + + const Symbol &B = findSymbol(Symbols, "B"); + EXPECT_THAT(expandTagBitmask(B.Tags), + UnorderedElementsAre(SymbolTag::Final, SymbolTag::Declaration, + SymbolTag::Definition)); + const Symbol &Bf1 = findSymbol(Symbols, "B::f1"); + EXPECT_THAT(expandTagBitmask(Bf1.Tags), + UnorderedElementsAre(SymbolTag::Public, SymbolTag::Final, + SymbolTag::Virtual, SymbolTag::Declaration, + SymbolTag::Definition, + SymbolTag::Implements)); +} + TEST_F(SymbolCollectorTest, ObjCOverrideRelationsSimpleInheritance) { std::string Header = R"cpp( @interface A From f99012b507cf99aa5daa71bc0ecbd1be96eb687e Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 24 Mar 2026 14:58:05 +0100 Subject: [PATCH 16/29] Change: relocated Tags to decrease padding. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 64-bit The problematic areas are: SymInfo (6 bytes) → followed by a 2-byte gap Origin (2 bytes) → followed by a 2-byte gap before StringRef Tags (4 bytes) → followed by a 4-byte gap before SmallVector Flags at the end → 7 bytes end padding 32-bit SymInfo → 2-byte gap Origin → 2-byte gap Flags at the end → 3 bytes end padding This patch reduces paddings: 64-bit: 240 -> 232 (-8 Bytes, ~3.3%) 32-bit: 144 -> 140 (-4 Bytes, ~2.8%) --- clang-tools-extra/clangd/index/Symbol.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clangd/index/Symbol.h b/clang-tools-extra/clangd/index/Symbol.h index 9bce1a1ece7e5..5f83953639a55 100644 --- a/clang-tools-extra/clangd/index/Symbol.h +++ b/clang-tools-extra/clangd/index/Symbol.h @@ -51,6 +51,8 @@ struct Symbol { SymbolID ID; /// The symbol information, like symbol kind. index::SymbolInfo SymInfo = index::SymbolInfo(); + /// Where this symbol came from. Usually an index provides a constant value. + SymbolOrigin Origin = SymbolOrigin::Unknown; /// The unqualified name of the symbol, e.g. "bar" (for ns::bar). llvm::StringRef Name; /// The containing namespace. e.g. "" (global), "ns::" (top-level namespace). @@ -70,8 +72,10 @@ struct Symbol { /// The number of translation units that reference this symbol from their main /// file. This number is only meaningful if aggregated in an index. unsigned References = 0; - /// Where this symbol came from. Usually an index provides a constant value. - SymbolOrigin Origin = SymbolOrigin::Unknown; + /// Symbol tags for LSP protocol (Deprecated, Static, Virtual, Abstract, + /// Final, ReadOnly, Public, Protected, Private, Declaration, Definition). + /// This is a bitmask where each bit represents a SymbolTag. + SymbolTags Tags = 0; /// A brief description of the symbol that can be appended in the completion /// candidate list. For example, "(X x, Y y) const" is a function signature. /// Only set when the symbol is indexed for completion. @@ -97,11 +101,6 @@ struct Symbol { /// Only set when the symbol is indexed for completion. llvm::StringRef Type; - /// Symbol tags for LSP protocol (Deprecated, Static, Virtual, Abstract, - /// Final, ReadOnly, Public, Protected, Private, Declaration, Definition). - /// This is a bitmask where each bit represents a SymbolTag. - SymbolTags Tags = 0; - enum IncludeDirective : uint8_t { Invalid = 0, /// `#include "header.h"` From 317abd3552d402c595f197f0cdecdec0881030ce Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 16 Apr 2026 12:18:42 +0200 Subject: [PATCH 17/29] Review: code review adjustments. Apply consistent SymbolTag filtering for indexed C++ methods Introduce `getSymbolTags(const Symbol&)` and use it for index-backed symbol paths so tag filtering is applied consistently outside AST-based code paths. - add `getSymbolTags(const Symbol&)` in `FindSymbols` and document behavior - apply method-specific tag filtering in: - workspace symbols (`FindSymbols.cpp`) - hierarchy items (`XRefs.cpp`) - simplify `filterSymbolTags` to operate on `SymbolTags` only - keep `Declaration`/`Definition` tags for virtual/override/implements cases (only remove tags that are semantically implied, e.g. `Virtual` by `Overrides`/`Implements`/`Abstract`) - update unit tests in `FindSymbolsTests.cpp` and `SymbolCollectorTests.cpp` to assert the new expected tag sets - use `getSymbolTags(...)` in tests instead of directly expanding raw bitmasks - minor cleanup in `SymbolCollector::addDeclaration` tag assignment ordering --- clang-tools-extra/clangd/FindSymbols.cpp | 75 +++++++++---------- clang-tools-extra/clangd/FindSymbols.h | 16 +++- clang-tools-extra/clangd/XRefs.cpp | 2 +- .../clangd/index/SymbolCollector.cpp | 4 +- .../clangd/unittests/FindSymbolsTests.cpp | 53 ++++++++----- .../clangd/unittests/SymbolCollectorTests.cpp | 13 ++-- 6 files changed, 93 insertions(+), 70 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index c909e077a1d67..78839bba00387 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -120,16 +120,6 @@ bool isAbstract(const Decl *D) { } // Indicates whether declaration D is virtual in cases where D is a method. -// We want to treat a method as virtual if it is declared virtual, even if it -// is not implemented in this class, or if it overrides/implements a -// base-class method. This is because the "virtual" modifier is still relevant -// to the method's behavior and how it should be highlighted, even if it is -// not itself a virtual method in the strictest sense. For example, a method -// that overrides a virtual method from a base class is still considered -// virtual, even if it is not declared as such in the derived class. -// Similarly, a method that implements a pure virtual method from a base class -// is also considered virtual, even if it is not declared as such in the -// derived class. bool isVirtual(const Decl *D) { if (const auto *CMD = llvm::dyn_cast(D)) return CMD->isVirtual(); @@ -206,20 +196,15 @@ bool isUniqueDefinition(const NamedDecl *Decl) { // Filter symbol tags based on the presence of other tags and the kind of // symbol. This is needed to avoid redundant tags, e.g. final implies override, // override implies virtual, etc. -SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) { +SymbolTags filterSymbolTags(const SymbolTags ST) { SymbolTags Result = ST; - if (not isa(ND)) - return Result; - if (ST & toSymbolTagBitmask(SymbolTag::Overrides)) { // Overrides means that ND overrides an existing implementation of a virtual // method in a base class. If a symbol is marked as Overrides, the tags // Virtual, Declaration and Definition should be removed, as the Overrides // tag implies that the symbol has/is virtual/declaration/definition. Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); - Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); - Result &= ~toSymbolTagBitmask(SymbolTag::Definition); } if (ST & toSymbolTagBitmask(SymbolTag::Implements)) { // Implements means that ND implements an existing pure virtual method in a @@ -228,26 +213,14 @@ SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) { // Implements tag implies that the symbol is virtual, is a declaration, is a // definition, and overrides a method. Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); - Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); - Result &= ~toSymbolTagBitmask(SymbolTag::Definition); Result &= ~toSymbolTagBitmask(SymbolTag::Overrides); } - if (ST & toSymbolTagBitmask(SymbolTag::Virtual)) { - // Virtual means that ND is a virtual method that does not override any - // method in a base class. If a symbol is marked as Virtual, the tags - // Declaration and Definition should be removed, as the Virtual tag implies - // that the symbol is a declaration/definition. - Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); - Result &= ~toSymbolTagBitmask(SymbolTag::Definition); - } if (ST & toSymbolTagBitmask(SymbolTag::Abstract)) { // Abstract means that ND is a pure virtual method. If a symbol is marked as // Abstract, the tags Virtual, Declaration and Definition should be removed, // as the Abstract tag implies that the symbol is virtual and a // declaration/definition. Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); - Result &= ~toSymbolTagBitmask(SymbolTag::Declaration); - Result &= ~toSymbolTagBitmask(SymbolTag::Definition); } if (ST & toSymbolTagBitmask(SymbolTag::Final)) { // Final means that ND is a method that cannot be overridden by any method @@ -259,6 +232,24 @@ SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) { } return Result; } + +bool isCXXClassMethod(const clang::clangd::Symbol &S) { + using clang::index::SymbolKind; + using clang::index::SymbolLanguage; + + if (S.SymInfo.Lang != SymbolLanguage::CXX) + return false; + + return llvm::is_contained({SymbolKind::InstanceMethod, + SymbolKind::StaticMethod, SymbolKind::Constructor, + SymbolKind::Destructor, + SymbolKind::ConversionFunction}, + S.SymInfo.Kind); +} + +template constexpr E enumIncrement(E Value) { + return static_cast(static_cast>(Value) + 1); +} } // namespace SymbolTags toSymbolTagBitmask(const SymbolTag ST) { @@ -343,25 +334,31 @@ std::vector expandTagBitmask(const SymbolTags STGS) { return Tags; } +std::vector getSymbolTags(const Symbol &S) { + const SymbolTags Tags = + isCXXClassMethod(S) ? filterSymbolTags(S.Tags) : S.Tags; + return expandTagBitmask(Tags); +} + std::vector getSymbolTags(const NamedDecl &ND) { - const auto symbolTags = computeSymbolTags(ND); + const auto STGS = computeSymbolTags(ND); + SymbolTags FilteredTags = STGS; std::vector Tags; - if (symbolTags == 0) + if (STGS == 0) return Tags; - // Apply specific filter to the symbol tags. - const auto filteredTags = filterSymbolTags(ND, symbolTags); + // Apply specific filter to the symbol tags only on CXX class methods. + if (isa(ND)) + FilteredTags = filterSymbolTags(STGS); // Iterate through SymbolTag enum values and collect any that are present in // the bitmask. SymbolTag values are in the numeric range // [FirstTag .. LastTag]. - constexpr unsigned MinTag = static_cast(SymbolTag::FirstTag); - constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); - for (unsigned I = MinTag; I <= MaxTag; ++I) { - auto ST = static_cast(I); - if (filteredTags & toSymbolTagBitmask(ST)) - Tags.push_back(ST); + for (SymbolTag Tag = SymbolTag::FirstTag; Tag <= SymbolTag::LastTag; + Tag = enumIncrement(Tag)) { + if (FilteredTags & toSymbolTagBitmask(Tag)) + Tags.push_back(Tag); } return Tags; } @@ -497,7 +494,7 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, Info.score = Relevance.NameMatch > std::numeric_limits::epsilon() ? Score / Relevance.NameMatch : QualScore; - Info.tags = expandTagBitmask(Sym.Tags); + Info.tags = getSymbolTags(Sym); Top.push({Score, std::move(Info)}); }); for (auto &R : std::move(Top).items()) diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 1e34731ad336f..9604940f5030e 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -63,9 +63,19 @@ SymbolTags computeSymbolTags(const NamedDecl &ND); /// \p ND The declaration to get tags for. std::vector getSymbolTags(const NamedDecl &ND); -/// Expands a SymbolTags bitmask into individual SymbolTag values. -std::vector expandTagBitmask(SymbolTags STGS); - +/// Returns the \c SymbolTag values for the given indexed \p S. +/// +/// Converts the bitmask stored in \c Symbol::Tags into a flat vector of +/// \c SymbolTag enum values. For C++ class methods (instance methods, static +/// methods, constructors, destructors, and conversion functions), a semantic +/// filter is applied first to remove tags that are implied by higher-priority +/// tags (e.g. \c Overrides implies \c Virtual, so \c Virtual is suppressed). +/// For all other symbol kinds the bitmask is expanded as-is. +/// +/// \param S The indexed symbol whose tags should be returned. +/// \return A vector of \c SymbolTag values present in \c S.Tags, after +/// applying any applicable filters. +std::vector getSymbolTags(const Symbol &S); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 72226e420480c..d35ac77a8494a 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1873,7 +1873,7 @@ static std::optional symbolToHierarchyItem(const Symbol &S, HI.name = std::string(S.Name); HI.detail = (S.Scope + S.Name).str(); HI.kind = indexSymbolKindToSymbolKind(S.SymInfo); - HI.tags = expandTagBitmask(S.Tags); + HI.tags = getSymbolTags(S); HI.selectionRange = Loc->range; // FIXME: Populate 'range' correctly // (https://github.com/clangd/clangd/issues/59). diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 87561f3268f0a..ba0b7f12d7aa0 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -1127,9 +1127,6 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, S.Documentation = Documentation; } }; - - S.Tags = computeSymbolTags(ND); - if (!(S.Flags & Symbol::IndexedForCodeCompletion)) { if (Opts.StoreAllDocumentation) UpdateDoc(); @@ -1142,6 +1139,7 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, SymbolCompletion.CursorKind); S.Signature = Signature; + S.Tags = computeSymbolTags(ND); S.CompletionSnippetSuffix = SnippetSuffix; std::string ReturnType = getReturnType(*CCS); S.ReturnType = ReturnType; diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index 63fb76e330d8f..2636e583b5b09 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -1181,10 +1181,12 @@ TEST(DocumentSymbolsTest, SymbolTagsMustContainPublicAbstract) { AllOf(withName("A"), children(AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, SymbolTag::Abstract)))), AllOf(withName("B"), children(AllOf(withName("f2"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, SymbolTag::Abstract)))))); } @@ -1212,17 +1214,23 @@ TEST(DocumentSymbolsTest, SymbolTagsMustContainPublicVirtualAndOverrides) { EXPECT_THAT( Symbols, UnorderedElementsAre( - AllOf(withName("A"), - children( - AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, - SymbolTag::Virtual)))), + AllOf( + withName("A"), + children(AllOf( + withName("f1"), + withSymbolTags(SymbolTag::Public, SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::Virtual)))), AllOf(withName("B"), - children(AllOf( - withName("f1"), - withSymbolTags(SymbolTag::Public, SymbolTag::Overrides)))), + children(AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, + SymbolTag::Definition, + SymbolTag::Overrides)))), AllOf(withName("C"), children(AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::Overrides)))))); } @@ -1258,18 +1266,25 @@ TEST(DocumentSymbolsTest, AllOf(withName("A"), children(AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, SymbolTag::Abstract)))), AllOf(withName("B"), children(AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::Implements)))), AllOf(withName("C"), children(AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::Overrides)))), AllOf(withName("D"), children(AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::Final)))))); } @@ -1306,9 +1321,12 @@ TEST(DocumentSymbolsTest, SymbolTagsCompilation) { SymbolTag::Definition), children( AllOf(withName("~A"), - withSymbolTags(SymbolTag::Public, SymbolTag::Virtual)), - AllOf(withName("f1"), - withSymbolTags(SymbolTag::Public, SymbolTag::Abstract)), + withSymbolTags(SymbolTag::Public, SymbolTag::Virtual, + SymbolTag::Declaration, + SymbolTag::Definition)), + AllOf(withName("f1"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, + SymbolTag::Abstract)), AllOf(withName("f2"), withSymbolTags(SymbolTag::Public, SymbolTag::Declaration, SymbolTag::ReadOnly)), @@ -1322,13 +1340,14 @@ TEST(DocumentSymbolsTest, SymbolTagsCompilation) { AllOf(withName("A::f2"), withSymbolTags(SymbolTag::Public, SymbolTag::Declaration, SymbolTag::Definition, SymbolTag::ReadOnly)), - AllOf( - withName("B"), - withSymbolTags(SymbolTag::Final, SymbolTag::Declaration, - SymbolTag::Definition), - children(AllOf(withName("f1"), - withSymbolTags(SymbolTag::Public, SymbolTag::Final, - SymbolTag::Implements)))))); + AllOf(withName("B"), + withSymbolTags(SymbolTag::Final, SymbolTag::Declaration, + SymbolTag::Definition), + children(AllOf( + withName("f1"), + withSymbolTags(SymbolTag::Public, SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::Final, + SymbolTag::Implements)))))); } } // namespace diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp index 10059247aa7d7..f0884e2a545a1 100644 --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -1362,20 +1362,19 @@ TEST_F(SymbolCollectorTest, SymbolTagsWithIndexing) { runSymbolCollector(Header, /*Main=*/""); const Symbol &A = findSymbol(Symbols, "A"); - EXPECT_THAT(expandTagBitmask(A.Tags), + EXPECT_THAT(getSymbolTags(A), UnorderedElementsAre(SymbolTag::Abstract, SymbolTag::Declaration, SymbolTag::Definition)); const Symbol &B = findSymbol(Symbols, "B"); - EXPECT_THAT(expandTagBitmask(B.Tags), + EXPECT_THAT(getSymbolTags(B), UnorderedElementsAre(SymbolTag::Final, SymbolTag::Declaration, SymbolTag::Definition)); const Symbol &Bf1 = findSymbol(Symbols, "B::f1"); - EXPECT_THAT(expandTagBitmask(Bf1.Tags), - UnorderedElementsAre(SymbolTag::Public, SymbolTag::Final, - SymbolTag::Virtual, SymbolTag::Declaration, - SymbolTag::Definition, - SymbolTag::Implements)); + EXPECT_THAT(getSymbolTags(Bf1), + UnorderedElementsAre( + SymbolTag::Public, SymbolTag::Final, SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::Implements)); } TEST_F(SymbolCollectorTest, ObjCOverrideRelationsSimpleInheritance) { From 477dd757692ff5d7c20ee32b376d3174434e2d4d Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 21 Apr 2026 14:59:24 +0200 Subject: [PATCH 18/29] Review: code review adjustments. Refine filterSymbolTags semantics for method-related tags Improve SymbolTag normalization in `filterSymbolTags` by making tag precedence explicit and handling redundant combinations consistently. Changes: - Introduce local bitmask constants (`Virtual`, `Overrides`, `Implements`, `Abstract`, `Final`) for readability and maintainability. - Keep precedence logic explicit: - `Implements` removes `Overrides` and `Virtual` - `Overrides` removes `Virtual` - `Abstract` removes `Virtual` - `Final` removes redundant `Virtual`/`Overrides` - Align inline comments with actual filtering behavior. This makes tag filtering easier to reason about and avoids inconsistent symbol-tag output for C++ methods. --- clang-tools-extra/clangd/FindSymbols.cpp | 71 ++++++++----------- .../clangd/index/SymbolCollector.cpp | 3 +- 2 files changed, 32 insertions(+), 42 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 78839bba00387..7bae579f4b1df 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -194,43 +194,32 @@ bool isUniqueDefinition(const NamedDecl *Decl) { } // Filter symbol tags based on the presence of other tags and the kind of -// symbol. This is needed to avoid redundant tags, e.g. final implies override, -// override implies virtual, etc. -SymbolTags filterSymbolTags(const SymbolTags ST) { - SymbolTags Result = ST; - - if (ST & toSymbolTagBitmask(SymbolTag::Overrides)) { - // Overrides means that ND overrides an existing implementation of a virtual - // method in a base class. If a symbol is marked as Overrides, the tags - // Virtual, Declaration and Definition should be removed, as the Overrides - // tag implies that the symbol has/is virtual/declaration/definition. - Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); - } - if (ST & toSymbolTagBitmask(SymbolTag::Implements)) { - // Implements means that ND implements an existing pure virtual method in a - // base class. If a symbol is marked as Implements, the tags Virtual, - // Declaration, Definition and Overrides should be removed, as the - // Implements tag implies that the symbol is virtual, is a declaration, is a - // definition, and overrides a method. - Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); - Result &= ~toSymbolTagBitmask(SymbolTag::Overrides); - } - if (ST & toSymbolTagBitmask(SymbolTag::Abstract)) { - // Abstract means that ND is a pure virtual method. If a symbol is marked as - // Abstract, the tags Virtual, Declaration and Definition should be removed, - // as the Abstract tag implies that the symbol is virtual and a - // declaration/definition. - Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); - } - if (ST & toSymbolTagBitmask(SymbolTag::Final)) { - // Final means that ND is a method that cannot be overridden by any method - // in a derived class. If a symbol is marked as Final, the tags Virtual and - // Overrides should be removed, as the Final tag implies that the symbol is - // virtual. - Result &= ~toSymbolTagBitmask(SymbolTag::Virtual); - Result &= ~toSymbolTagBitmask(SymbolTag::Overrides); - } - return Result; +// symbol. This is needed to avoid redundant tags, e.g. Overrides implies +// Virtual and Implements implies Overrides/Virtual. +SymbolTags filterSymbolTags(SymbolTags ST) { + const SymbolTags VirtualMask = toSymbolTagBitmask(SymbolTag::Virtual); + const SymbolTags OverridesMask = toSymbolTagBitmask(SymbolTag::Overrides); + const SymbolTags ImplementsMask = toSymbolTagBitmask(SymbolTag::Implements); + const SymbolTags AbstractMask = toSymbolTagBitmask(SymbolTag::Abstract); + const SymbolTags FinalMask = toSymbolTagBitmask(SymbolTag::Final); + + // Implements implies Overrides + Virtual. + if (ST & ImplementsMask) + ST &= ~(OverridesMask | VirtualMask); + + // Overrides implies Virtual. + if (ST & OverridesMask) + ST &= ~VirtualMask; + + // Abstract implies Virtual. + if (ST & AbstractMask) + ST &= ~VirtualMask; + + // Final implies Virtual; Overrides is also redundant as Final overrides are still overrides. + if (ST & FinalMask) + ST &= ~(VirtualMask | OverridesMask); + + return ST; } bool isCXXClassMethod(const clang::clangd::Symbol &S) { @@ -253,7 +242,7 @@ template constexpr E enumIncrement(E Value) { } // namespace SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return (1 << static_cast(ST)); + return (1 << static_cast(ST)); } SymbolTags computeSymbolTags(const NamedDecl &ND) { @@ -324,9 +313,9 @@ std::vector expandTagBitmask(const SymbolTags STGS) { // Iterate through SymbolTag enum values and collect any that are present in // the bitmask. SymbolTag values are in the numeric range // [FirstTag .. LastTag]. - constexpr unsigned MinTag = static_cast(SymbolTag::FirstTag); - constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); - for (unsigned I = MinTag; I <= MaxTag; ++I) { + constexpr uint32_t MinTag = static_cast(SymbolTag::FirstTag); + constexpr uint32_t MaxTag = static_cast(SymbolTag::LastTag); + for (uint32_t I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); if (STGS & toSymbolTagBitmask(ST)) Tags.push_back(ST); diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index ba0b7f12d7aa0..a7c5f35f3aea9 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -1103,6 +1103,8 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, if (ND.getAvailability() == AR_Deprecated) S.Flags |= Symbol::Deprecated; + S.Tags = computeSymbolTags(ND); + // Add completion info. // FIXME: we may want to choose a different redecl, or combine from several. assert(ASTCtx && PP && "ASTContext and Preprocessor must be set."); @@ -1139,7 +1141,6 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, SymbolCompletion.CursorKind); S.Signature = Signature; - S.Tags = computeSymbolTags(ND); S.CompletionSnippetSuffix = SnippetSuffix; std::string ReturnType = getReturnType(*CCS); S.ReturnType = ReturnType; From 765ef3e7e5be85b8feb60a615da6d6c62805aad9 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 22 Apr 2026 09:55:56 +0200 Subject: [PATCH 19/29] Refactor: Make SymbolTags type-safe and adapt tag handling Replace raw integer-based SymbolTags with a dedicated strong type that is tied to SymbolTag and sanitizes invalid bits. This removes a fragile coupling between SymbolTag and SymbolTags and prevents representing non-existing tags. Update clangd tag logic and serialization paths accordingly: - switch tag construction/checks to SymbolTags::fromTag()/typed operations - keep existing on-disk bit positions compatible - adapt RIFF/YAML serialization to read/write raw SymbolTags storage safely - update serialization tests to use the typed SymbolTags API This improves robustness and maintainability while preserving behavior and backward compatibility of serialized tag values. --- clang-tools-extra/clangd/FindSymbols.cpp | 8 +- .../clangd/index/Serialization.cpp | 4 +- clang-tools-extra/clangd/index/Symbol.h | 106 ++++++++++++++++-- .../clangd/index/YAMLSerialization.cpp | 17 +++ .../clangd/unittests/SerializationTests.cpp | 30 +++-- 5 files changed, 134 insertions(+), 31 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 7bae579f4b1df..68f07a0870cc7 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -242,11 +242,11 @@ template constexpr E enumIncrement(E Value) { } // namespace SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return (1 << static_cast(ST)); + return SymbolTags::fromTag(ST); } SymbolTags computeSymbolTags(const NamedDecl &ND) { - SymbolTags Result = 0; + SymbolTags Result; const auto IsDef = isUniqueDefinition(&ND); if (ND.isDeprecated()) @@ -303,7 +303,7 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) { std::vector expandTagBitmask(const SymbolTags STGS) { std::vector Tags; - if (STGS == 0) + if (STGS.empty()) return Tags; // No filtering required since this function is only used for Symbols from the @@ -334,7 +334,7 @@ std::vector getSymbolTags(const NamedDecl &ND) { SymbolTags FilteredTags = STGS; std::vector Tags; - if (STGS == 0) + if (STGS.empty()) return Tags; // Apply specific filter to the symbol tags only on CXX class methods. diff --git a/clang-tools-extra/clangd/index/Serialization.cpp b/clang-tools-extra/clangd/index/Serialization.cpp index 764ec7903aecb..e36171e0a1a75 100644 --- a/clang-tools-extra/clangd/index/Serialization.cpp +++ b/clang-tools-extra/clangd/index/Serialization.cpp @@ -328,7 +328,7 @@ void writeSymbol(const Symbol &Sym, const StringTableOut &Strings, writeVar(Strings.index(Sym.Documentation), OS); writeVar(Strings.index(Sym.ReturnType), OS); writeVar(Strings.index(Sym.Type), OS); - writeVar(Sym.Tags, OS); + writeVar(Sym.Tags.raw(), OS); auto WriteInclude = [&](const Symbol::IncludeHeaderWithReferences &Include) { writeVar(Strings.index(Include.IncludeHeader), OS); @@ -358,7 +358,7 @@ Symbol readSymbol(Reader &Data, llvm::ArrayRef Strings, Sym.Documentation = Data.consumeString(Strings); Sym.ReturnType = Data.consumeString(Strings); Sym.Type = Data.consumeString(Strings); - Sym.Tags = Data.consumeVar(); + Sym.Tags = SymbolTags::fromRaw(Data.consumeVar()); if (!Data.consumeSize(Sym.IncludeHeaders)) return Sym; for (auto &I : Sym.IncludeHeaders) { diff --git a/clang-tools-extra/clangd/index/Symbol.h b/clang-tools-extra/clangd/index/Symbol.h index 5f83953639a55..780e7d1682c95 100644 --- a/clang-tools-extra/clangd/index/Symbol.h +++ b/clang-tools-extra/clangd/index/Symbol.h @@ -17,20 +17,108 @@ #include "llvm/ADT/BitmaskEnum.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/StringSaver.h" +#include +#include namespace clang { namespace clangd { LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); -/// A bitmask type representing symbol tags supported by LSP. -/// \see -/// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#symbolTag -using SymbolTags = uint32_t; -/// Ensure we have enough bits to represent all SymbolTag values. -static_assert(static_cast(SymbolTag::LastTag) < 32, - "Too many SymbolTags to fit in uint32_t. Change to uint64_t if " - "we ever have more than 32 tags."); +/// Type-safe bitset for SymbolTag values. +/// +/// - The storage type adapts to SymbolTag's underlying type. +/// - Bits outside the supported [FirstTag..LastTag] range are dropped. +class SymbolTags { +public: + using Storage = std::make_unsigned_t>; + + constexpr SymbolTags() = default; + + static constexpr SymbolTags fromRaw(Storage Raw) { + SymbolTags Result; + Result.Bits = Raw & validMask(); + return Result; + } + + static constexpr SymbolTags fromTag(SymbolTag Tag) { + SymbolTags Result; + Result.set(Tag); + return Result; + } + + constexpr Storage raw() const { return Bits; } + constexpr bool empty() const { return Bits == 0; } + constexpr explicit operator bool() const { return !empty(); } + + constexpr bool contains(SymbolTag Tag) const { + return (Bits & bitMask(Tag)) != 0; + } + + constexpr void set(SymbolTag Tag) { Bits |= bitMask(Tag); } + constexpr void reset(SymbolTag Tag) { Bits &= ~bitMask(Tag); } + + constexpr SymbolTags &operator|=(SymbolTags Other) { + Bits = (Bits | Other.Bits) & validMask(); + return *this; + } + + constexpr SymbolTags &operator&=(SymbolTags Other) { + Bits &= Other.Bits; + return *this; + } + + friend constexpr SymbolTags operator|(SymbolTags L, SymbolTags R) { + L |= R; + return L; + } + + friend constexpr SymbolTags operator&(SymbolTags L, SymbolTags R) { + L &= R; + return L; + } + + friend constexpr SymbolTags operator~(SymbolTags V) { + return fromRaw(static_cast(~V.Bits)); + } + + friend constexpr bool operator==(SymbolTags L, SymbolTags R) { + return L.Bits == R.Bits; + } + +private: + static constexpr unsigned firstTag() { + return static_cast(SymbolTag::FirstTag); + } + + static constexpr unsigned lastTag() { + return static_cast(SymbolTag::LastTag); + } + + static constexpr Storage validMask() { + constexpr auto Digits = std::numeric_limits::digits; + const Storage Lower = + (firstTag() == 0) ? Storage{0} : ((Storage{1} << firstTag()) - 1); + const Storage Upper = (lastTag() + 1 == Digits) + ? ~Storage{0} + : ((Storage{1} << (lastTag() + 1)) - 1); + return Upper & ~Lower; + } + + static constexpr Storage bitMask(SymbolTag Tag) { + const auto Ordinal = static_cast(Tag); + return Storage{1} << Ordinal; + } + + Storage Bits = 0; +}; + +static_assert(static_cast(SymbolTag::FirstTag) <= + static_cast(SymbolTag::LastTag), + "SymbolTag::FirstTag must not exceed SymbolTag::LastTag"); +static_assert(static_cast(SymbolTag::LastTag) < + std::numeric_limits::digits, + "Too many SymbolTag values for SymbolTags::Storage"); /// The class presents a C++ symbol, e.g. class, function. /// @@ -75,7 +163,7 @@ struct Symbol { /// Symbol tags for LSP protocol (Deprecated, Static, Virtual, Abstract, /// Final, ReadOnly, Public, Protected, Private, Declaration, Definition). /// This is a bitmask where each bit represents a SymbolTag. - SymbolTags Tags = 0; + SymbolTags Tags; /// A brief description of the symbol that can be appended in the completion /// candidate list. For example, "(X x, Y y) const" is a function signature. /// Only set when the symbol is indexed for completion. diff --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp b/clang-tools-extra/clangd/index/YAMLSerialization.cpp index 4d6b5822ece72..162e5d1287ba4 100644 --- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp +++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp @@ -86,11 +86,28 @@ using clang::clangd::RelationKind; using clang::clangd::Symbol; using clang::clangd::SymbolID; using clang::clangd::SymbolLocation; +using clang::clangd::SymbolTags; using clang::index::SymbolInfo; using clang::index::SymbolKind; using clang::index::SymbolLanguage; using clang::tooling::CompileCommand; +template <> struct ScalarTraits { + static void output(const SymbolTags &Value, void *, raw_ostream &Out) { + Out << Value.raw(); + } + + static StringRef input(StringRef Scalar, void *, SymbolTags &Value) { + SymbolTags::Storage Raw; + if (Scalar.getAsInteger(/*Radix=*/10, Raw)) + return "invalid SymbolTags value"; + Value = SymbolTags::fromRaw(Raw); + return {}; + } + + static QuotingType mustQuote(StringRef) { return QuotingType::None; } +}; + // Helper to (de)serialize the SymbolID. We serialize it as a hex string. struct NormalizedSymbolID { NormalizedSymbolID(IO &) {} diff --git a/clang-tools-extra/clangd/unittests/SerializationTests.cpp b/clang-tools-extra/clangd/unittests/SerializationTests.cpp index 6b95331c8f7c5..14e9ff72565b5 100644 --- a/clang-tools-extra/clangd/unittests/SerializationTests.cpp +++ b/clang-tools-extra/clangd/unittests/SerializationTests.cpp @@ -161,10 +161,12 @@ TEST(SerializationTest, YAMLConversions) { EXPECT_EQ(static_cast(Sym1.Flags), 129); EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion); EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated); - // Tags: Deprecated (1<<1=2) | Static (1<<8=256) = 258 - EXPECT_EQ(Sym1.Tags, 258u); - EXPECT_TRUE(Sym1.Tags & (1u << static_cast(SymbolTag::Deprecated))); - EXPECT_TRUE(Sym1.Tags & (1u << static_cast(SymbolTag::Static))); + // Tags: Deprecated | Static + EXPECT_EQ(Sym1.Tags, + SymbolTags::fromTag(SymbolTag::Deprecated) | + SymbolTags::fromTag(SymbolTag::Static)); + EXPECT_TRUE((Sym1.Tags & SymbolTags::fromTag(SymbolTag::Deprecated))); + EXPECT_TRUE((Sym1.Tags & SymbolTags::fromTag(SymbolTag::Static))); EXPECT_THAT( Sym1.IncludeHeaders, UnorderedElementsAre( @@ -181,7 +183,7 @@ TEST(SerializationTest, YAMLConversions) { "file:///path/bar.h"); EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion); EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated); - EXPECT_EQ(Sym2.Tags, 0u); // no Tags in YAML → default zero + EXPECT_TRUE(Sym2.Tags.empty()); // no Tags in YAML -> default zero ASSERT_TRUE(bool(ParsedYAML->Refs)); EXPECT_THAT( @@ -302,9 +304,7 @@ TEST(SerializationTest, SrcsTest) { // This exercises both writeSymbol (new Tags field) and readSymbol. TEST(SerializationTest, TagsRoundTrip) { // YAML fixture with a single symbol carrying non-zero Tags. - // Deprecated (SymbolTag=1) -> bit 1 -> 1<<1 = 2 - // Static (SymbolTag=8) -> bit 8 -> 1<<8 = 256 - // Combined: 258 + // Combined SymbolTags value: Deprecated | Static. const char *TaggedYAML = R"( --- !Symbol @@ -328,9 +328,8 @@ Tags: 258 )"; const SymbolID TaggedID = cantFail(SymbolID::fromStr("AABBCCDDEEFF0011")); constexpr SymbolTags ExpectedTags = - (1u << static_cast(SymbolTag::Deprecated)) | // bit 1 = 2 - (1u << static_cast(SymbolTag::Static)); // bit 8 = 256 - static_assert(ExpectedTags == 258u, "bitmask sanity check"); + SymbolTags::fromTag(SymbolTag::Deprecated) | + SymbolTags::fromTag(SymbolTag::Static); // ── Step 1: YAML deserialization ──────────────────────────────────────── auto FromYAML = readIndexFile(TaggedYAML); @@ -358,10 +357,9 @@ Tags: 258 << "symbol not found after RIFF roundtrip"; EXPECT_EQ(It->Tags, ExpectedTags) << "Tags lost during RIFF serialization"; // Spot-check individual bits - EXPECT_TRUE(It->Tags & - (1u << static_cast(SymbolTag::Deprecated))); - EXPECT_TRUE(It->Tags & (1u << static_cast(SymbolTag::Static))); - EXPECT_FALSE(It->Tags & (1u << static_cast(SymbolTag::Abstract))); + EXPECT_TRUE(It->Tags & SymbolTags::fromTag(SymbolTag::Deprecated)); + EXPECT_TRUE(It->Tags & SymbolTags::fromTag(SymbolTag::Static)); + EXPECT_FALSE(It->Tags & SymbolTags::fromTag(SymbolTag::Abstract)); } // ── Step 3: Symbol with Tags=0 must survive too ───────────────────────── @@ -397,7 +395,7 @@ Flags: 1 const SymbolID UntaggedID = cantFail(SymbolID::fromStr("0000000000000001")); auto It = FromUntaggedRIFF->Symbols->find(UntaggedID); ASSERT_NE(It, FromUntaggedRIFF->Symbols->end()); - EXPECT_EQ(It->Tags, 0u) << "Tags must be zero for untagged symbol"; + EXPECT_TRUE(It->Tags.empty()) << "Tags must be zero for untagged symbol"; } } From 949f51018fc54f4f32d0833e29cb24c0aa9802d5 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 22 Apr 2026 09:59:28 +0200 Subject: [PATCH 20/29] Fix: code format. --- clang-tools-extra/clangd/unittests/SerializationTests.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/SerializationTests.cpp b/clang-tools-extra/clangd/unittests/SerializationTests.cpp index 14e9ff72565b5..7a62e2007fb31 100644 --- a/clang-tools-extra/clangd/unittests/SerializationTests.cpp +++ b/clang-tools-extra/clangd/unittests/SerializationTests.cpp @@ -162,9 +162,8 @@ TEST(SerializationTest, YAMLConversions) { EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion); EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated); // Tags: Deprecated | Static - EXPECT_EQ(Sym1.Tags, - SymbolTags::fromTag(SymbolTag::Deprecated) | - SymbolTags::fromTag(SymbolTag::Static)); + EXPECT_EQ(Sym1.Tags, SymbolTags::fromTag(SymbolTag::Deprecated) | + SymbolTags::fromTag(SymbolTag::Static)); EXPECT_TRUE((Sym1.Tags & SymbolTags::fromTag(SymbolTag::Deprecated))); EXPECT_TRUE((Sym1.Tags & SymbolTags::fromTag(SymbolTag::Static))); EXPECT_THAT( From 88aa2a110ce08c8332bf8f22793aaeeea08bf009 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 22 Apr 2026 10:22:12 +0200 Subject: [PATCH 21/29] Fix: code format. --- clang-tools-extra/clangd/FindSymbols.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 68f07a0870cc7..4dae25068d6ca 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -215,7 +215,8 @@ SymbolTags filterSymbolTags(SymbolTags ST) { if (ST & AbstractMask) ST &= ~VirtualMask; - // Final implies Virtual; Overrides is also redundant as Final overrides are still overrides. + // Final implies Virtual; Overrides is also redundant as Final overrides are + // still overrides. if (ST & FinalMask) ST &= ~(VirtualMask | OverridesMask); From bdacf738a50a5ebd8c7ec69e14c8846ecfc3fae0 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 27 Apr 2026 09:39:24 +0200 Subject: [PATCH 22/29] Replaced toSymbolTagBitmask with SymbolTags::fromTag. --- clang-tools-extra/clangd/FindSymbols.cpp | 44 +++++++++---------- clang-tools-extra/clangd/FindSymbols.h | 3 -- .../clangd/SemanticHighlighting.cpp | 6 +-- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 4dae25068d6ca..fc34f5076a3b6 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -197,11 +197,11 @@ bool isUniqueDefinition(const NamedDecl *Decl) { // symbol. This is needed to avoid redundant tags, e.g. Overrides implies // Virtual and Implements implies Overrides/Virtual. SymbolTags filterSymbolTags(SymbolTags ST) { - const SymbolTags VirtualMask = toSymbolTagBitmask(SymbolTag::Virtual); - const SymbolTags OverridesMask = toSymbolTagBitmask(SymbolTag::Overrides); - const SymbolTags ImplementsMask = toSymbolTagBitmask(SymbolTag::Implements); - const SymbolTags AbstractMask = toSymbolTagBitmask(SymbolTag::Abstract); - const SymbolTags FinalMask = toSymbolTagBitmask(SymbolTag::Final); + const SymbolTags VirtualMask = SymbolTags::fromTag(SymbolTag::Virtual); + const SymbolTags OverridesMask = SymbolTags::fromTag(SymbolTag::Overrides); + const SymbolTags ImplementsMask = SymbolTags::fromTag(SymbolTag::Implements); + const SymbolTags AbstractMask = SymbolTags::fromTag(SymbolTag::Abstract); + const SymbolTags FinalMask = SymbolTags::fromTag(SymbolTag::Final); // Implements implies Overrides + Virtual. if (ST & ImplementsMask) @@ -242,57 +242,53 @@ template constexpr E enumIncrement(E Value) { } } // namespace -SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return SymbolTags::fromTag(ST); -} - SymbolTags computeSymbolTags(const NamedDecl &ND) { SymbolTags Result; const auto IsDef = isUniqueDefinition(&ND); if (ND.isDeprecated()) - Result |= toSymbolTagBitmask(SymbolTag::Deprecated); + Result |= SymbolTags::fromTag(SymbolTag::Deprecated); if (isConst(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::ReadOnly); + Result |= SymbolTags::fromTag(SymbolTag::ReadOnly); if (isStatic(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Static); + Result |= SymbolTags::fromTag(SymbolTag::Static); if (isVirtual(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Virtual); + Result |= SymbolTags::fromTag(SymbolTag::Virtual); if (isAbstract(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Abstract); + Result |= SymbolTags::fromTag(SymbolTag::Abstract); if (isOverrides(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Overrides); + Result |= SymbolTags::fromTag(SymbolTag::Overrides); if (isFinal(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Final); + Result |= SymbolTags::fromTag(SymbolTag::Final); if (isImplements(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Implements); + Result |= SymbolTags::fromTag(SymbolTag::Implements); if (not isa(ND)) { // Do not treat an UnresolvedUsingValueDecl as a declaration. // It's more common to think of it as a reference to the // underlying declaration. - Result |= toSymbolTagBitmask(SymbolTag::Declaration); + Result |= SymbolTags::fromTag(SymbolTag::Declaration); if (IsDef) - Result |= toSymbolTagBitmask(SymbolTag::Definition); + Result |= SymbolTags::fromTag(SymbolTag::Definition); } switch (ND.getAccess()) { case AS_public: - Result |= toSymbolTagBitmask(SymbolTag::Public); + Result |= SymbolTags::fromTag(SymbolTag::Public); break; case AS_protected: - Result |= toSymbolTagBitmask(SymbolTag::Protected); + Result |= SymbolTags::fromTag(SymbolTag::Protected); break; case AS_private: - Result |= toSymbolTagBitmask(SymbolTag::Private); + Result |= SymbolTags::fromTag(SymbolTag::Private); break; default: break; @@ -318,7 +314,7 @@ std::vector expandTagBitmask(const SymbolTags STGS) { constexpr uint32_t MaxTag = static_cast(SymbolTag::LastTag); for (uint32_t I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); - if (STGS & toSymbolTagBitmask(ST)) + if (STGS & SymbolTags::fromTag(ST)) Tags.push_back(ST); } return Tags; @@ -347,7 +343,7 @@ std::vector getSymbolTags(const NamedDecl &ND) { // [FirstTag .. LastTag]. for (SymbolTag Tag = SymbolTag::FirstTag; Tag <= SymbolTag::LastTag; Tag = enumIncrement(Tag)) { - if (FilteredTags & toSymbolTagBitmask(Tag)) + if (FilteredTags & SymbolTags::fromTag(Tag)) Tags.push_back(Tag); } return Tags; diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 9604940f5030e..ae573ba56fbf9 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -51,9 +51,6 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, /// same order that they appear. llvm::Expected> getDocumentSymbols(ParsedAST &AST); -/// Converts a single SymbolTag to a bitmask. -SymbolTags toSymbolTagBitmask(SymbolTag ST); - /// Computes symbol tags for a given NamedDecl. SymbolTags computeSymbolTags(const NamedDecl &ND); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 79446a166d0d8..6825fee975a4a 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1070,15 +1070,15 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { }; for (const auto &[Tag, Modifier] : TagModifierMap) { - if (SymbolTags & toSymbolTagBitmask(Tag)) + if (SymbolTags & SymbolTags::fromTag(Tag)) Tok.addModifier(Modifier); } if (R.IsDecl && - (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration))) { + (SymbolTags & SymbolTags::fromTag(SymbolTag::Declaration))) { Tok.addModifier(HighlightingModifier::Declaration); - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition)) + if (SymbolTags & SymbolTags::fromTag(SymbolTag::Definition)) Tok.addModifier(HighlightingModifier::Definition); } From 86a41ca30a044130d0cf774f3b3badf881f8c68a Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 27 Apr 2026 11:17:39 +0200 Subject: [PATCH 23/29] Simplified filterSymbolTags: - Reused a combined mask (`RemoveVirtualAndOverrides`) to avoid repeating `~(Overrides | Virtual)`. - Merged the two identical "remove virtual" cases (`Overrides`, `Abstract`) into one condition. --- clang-tools-extra/clangd/FindSymbols.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index fc34f5076a3b6..caa0fa24e305f 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -203,23 +203,20 @@ SymbolTags filterSymbolTags(SymbolTags ST) { const SymbolTags AbstractMask = SymbolTags::fromTag(SymbolTag::Abstract); const SymbolTags FinalMask = SymbolTags::fromTag(SymbolTag::Final); - // Implements implies Overrides + Virtual. + const SymbolTags RemoveVirtualAndOverrides = VirtualMask | OverridesMask; + + // Implements implies both Overrides and Virtual. if (ST & ImplementsMask) - ST &= ~(OverridesMask | VirtualMask); + ST &= ~RemoveVirtualAndOverrides; - // Overrides implies Virtual. - if (ST & OverridesMask) - ST &= ~VirtualMask; + // Final also suppresses both Virtual and Overrides in this model. + if (ST & FinalMask) + ST &= ~RemoveVirtualAndOverrides; - // Abstract implies Virtual. - if (ST & AbstractMask) + // Overrides or Abstract each imply Virtual. + if (ST & (OverridesMask | AbstractMask)) ST &= ~VirtualMask; - // Final implies Virtual; Overrides is also redundant as Final overrides are - // still overrides. - if (ST & FinalMask) - ST &= ~(VirtualMask | OverridesMask); - return ST; } From 2aea84882b2b6b6c34629af65c65c30cfba9cc9e Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 27 Apr 2026 15:01:46 +0200 Subject: [PATCH 24/29] Revert "Replaced toSymbolTagBitmask with SymbolTags::fromTag." This reverts commit bdacf738a50a5ebd8c7ec69e14c8846ecfc3fae0. --- clang-tools-extra/clangd/FindSymbols.cpp | 44 ++++++++++--------- clang-tools-extra/clangd/FindSymbols.h | 3 ++ .../clangd/SemanticHighlighting.cpp | 6 +-- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index caa0fa24e305f..301aad0ead31a 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -197,11 +197,11 @@ bool isUniqueDefinition(const NamedDecl *Decl) { // symbol. This is needed to avoid redundant tags, e.g. Overrides implies // Virtual and Implements implies Overrides/Virtual. SymbolTags filterSymbolTags(SymbolTags ST) { - const SymbolTags VirtualMask = SymbolTags::fromTag(SymbolTag::Virtual); - const SymbolTags OverridesMask = SymbolTags::fromTag(SymbolTag::Overrides); - const SymbolTags ImplementsMask = SymbolTags::fromTag(SymbolTag::Implements); - const SymbolTags AbstractMask = SymbolTags::fromTag(SymbolTag::Abstract); - const SymbolTags FinalMask = SymbolTags::fromTag(SymbolTag::Final); + const SymbolTags VirtualMask = toSymbolTagBitmask(SymbolTag::Virtual); + const SymbolTags OverridesMask = toSymbolTagBitmask(SymbolTag::Overrides); + const SymbolTags ImplementsMask = toSymbolTagBitmask(SymbolTag::Implements); + const SymbolTags AbstractMask = toSymbolTagBitmask(SymbolTag::Abstract); + const SymbolTags FinalMask = toSymbolTagBitmask(SymbolTag::Final); const SymbolTags RemoveVirtualAndOverrides = VirtualMask | OverridesMask; @@ -239,53 +239,57 @@ template constexpr E enumIncrement(E Value) { } } // namespace +SymbolTags toSymbolTagBitmask(const SymbolTag ST) { + return SymbolTags::fromTag(ST); +} + SymbolTags computeSymbolTags(const NamedDecl &ND) { SymbolTags Result; const auto IsDef = isUniqueDefinition(&ND); if (ND.isDeprecated()) - Result |= SymbolTags::fromTag(SymbolTag::Deprecated); + Result |= toSymbolTagBitmask(SymbolTag::Deprecated); if (isConst(&ND)) - Result |= SymbolTags::fromTag(SymbolTag::ReadOnly); + Result |= toSymbolTagBitmask(SymbolTag::ReadOnly); if (isStatic(&ND)) - Result |= SymbolTags::fromTag(SymbolTag::Static); + Result |= toSymbolTagBitmask(SymbolTag::Static); if (isVirtual(&ND)) - Result |= SymbolTags::fromTag(SymbolTag::Virtual); + Result |= toSymbolTagBitmask(SymbolTag::Virtual); if (isAbstract(&ND)) - Result |= SymbolTags::fromTag(SymbolTag::Abstract); + Result |= toSymbolTagBitmask(SymbolTag::Abstract); if (isOverrides(&ND)) - Result |= SymbolTags::fromTag(SymbolTag::Overrides); + Result |= toSymbolTagBitmask(SymbolTag::Overrides); if (isFinal(&ND)) - Result |= SymbolTags::fromTag(SymbolTag::Final); + Result |= toSymbolTagBitmask(SymbolTag::Final); if (isImplements(&ND)) - Result |= SymbolTags::fromTag(SymbolTag::Implements); + Result |= toSymbolTagBitmask(SymbolTag::Implements); if (not isa(ND)) { // Do not treat an UnresolvedUsingValueDecl as a declaration. // It's more common to think of it as a reference to the // underlying declaration. - Result |= SymbolTags::fromTag(SymbolTag::Declaration); + Result |= toSymbolTagBitmask(SymbolTag::Declaration); if (IsDef) - Result |= SymbolTags::fromTag(SymbolTag::Definition); + Result |= toSymbolTagBitmask(SymbolTag::Definition); } switch (ND.getAccess()) { case AS_public: - Result |= SymbolTags::fromTag(SymbolTag::Public); + Result |= toSymbolTagBitmask(SymbolTag::Public); break; case AS_protected: - Result |= SymbolTags::fromTag(SymbolTag::Protected); + Result |= toSymbolTagBitmask(SymbolTag::Protected); break; case AS_private: - Result |= SymbolTags::fromTag(SymbolTag::Private); + Result |= toSymbolTagBitmask(SymbolTag::Private); break; default: break; @@ -311,7 +315,7 @@ std::vector expandTagBitmask(const SymbolTags STGS) { constexpr uint32_t MaxTag = static_cast(SymbolTag::LastTag); for (uint32_t I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); - if (STGS & SymbolTags::fromTag(ST)) + if (STGS & toSymbolTagBitmask(ST)) Tags.push_back(ST); } return Tags; @@ -340,7 +344,7 @@ std::vector getSymbolTags(const NamedDecl &ND) { // [FirstTag .. LastTag]. for (SymbolTag Tag = SymbolTag::FirstTag; Tag <= SymbolTag::LastTag; Tag = enumIncrement(Tag)) { - if (FilteredTags & SymbolTags::fromTag(Tag)) + if (FilteredTags & toSymbolTagBitmask(Tag)) Tags.push_back(Tag); } return Tags; diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index ae573ba56fbf9..9604940f5030e 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -51,6 +51,9 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, /// same order that they appear. llvm::Expected> getDocumentSymbols(ParsedAST &AST); +/// Converts a single SymbolTag to a bitmask. +SymbolTags toSymbolTagBitmask(SymbolTag ST); + /// Computes symbol tags for a given NamedDecl. SymbolTags computeSymbolTags(const NamedDecl &ND); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 6825fee975a4a..79446a166d0d8 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1070,15 +1070,15 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { }; for (const auto &[Tag, Modifier] : TagModifierMap) { - if (SymbolTags & SymbolTags::fromTag(Tag)) + if (SymbolTags & toSymbolTagBitmask(Tag)) Tok.addModifier(Modifier); } if (R.IsDecl && - (SymbolTags & SymbolTags::fromTag(SymbolTag::Declaration))) { + (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration))) { Tok.addModifier(HighlightingModifier::Declaration); - if (SymbolTags & SymbolTags::fromTag(SymbolTag::Definition)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition)) Tok.addModifier(HighlightingModifier::Definition); } From c39d64b9d01a8ac3504d1f8ce403336a2a5696ca Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 27 Apr 2026 15:02:00 +0200 Subject: [PATCH 25/29] Revert "Simplified filterSymbolTags:" This reverts commit 86a41ca30a044130d0cf774f3b3badf881f8c68a. --- clang-tools-extra/clangd/FindSymbols.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 301aad0ead31a..4dae25068d6ca 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -203,20 +203,23 @@ SymbolTags filterSymbolTags(SymbolTags ST) { const SymbolTags AbstractMask = toSymbolTagBitmask(SymbolTag::Abstract); const SymbolTags FinalMask = toSymbolTagBitmask(SymbolTag::Final); - const SymbolTags RemoveVirtualAndOverrides = VirtualMask | OverridesMask; - - // Implements implies both Overrides and Virtual. + // Implements implies Overrides + Virtual. if (ST & ImplementsMask) - ST &= ~RemoveVirtualAndOverrides; + ST &= ~(OverridesMask | VirtualMask); - // Final also suppresses both Virtual and Overrides in this model. - if (ST & FinalMask) - ST &= ~RemoveVirtualAndOverrides; + // Overrides implies Virtual. + if (ST & OverridesMask) + ST &= ~VirtualMask; - // Overrides or Abstract each imply Virtual. - if (ST & (OverridesMask | AbstractMask)) + // Abstract implies Virtual. + if (ST & AbstractMask) ST &= ~VirtualMask; + // Final implies Virtual; Overrides is also redundant as Final overrides are + // still overrides. + if (ST & FinalMask) + ST &= ~(VirtualMask | OverridesMask); + return ST; } From d54d642c1084d1336a3d4ec941d0af423a086fdb Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 29 Apr 2026 12:31:00 +0200 Subject: [PATCH 26/29] Revert "Refactor: Make SymbolTags type-safe and adapt tag handling" This reverts commit 765ef3e7 --- clang-tools-extra/clangd/FindSymbols.cpp | 8 +- .../clangd/index/Serialization.cpp | 4 +- clang-tools-extra/clangd/index/Symbol.h | 106 ++---------------- .../clangd/index/YAMLSerialization.cpp | 17 --- .../clangd/unittests/SerializationTests.cpp | 29 ++--- 5 files changed, 31 insertions(+), 133 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 4dae25068d6ca..9c5fee84ddd8b 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -243,11 +243,11 @@ template constexpr E enumIncrement(E Value) { } // namespace SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return SymbolTags::fromTag(ST); + return (1 << static_cast(ST)); } SymbolTags computeSymbolTags(const NamedDecl &ND) { - SymbolTags Result; + SymbolTags Result = 0; const auto IsDef = isUniqueDefinition(&ND); if (ND.isDeprecated()) @@ -304,7 +304,7 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) { std::vector expandTagBitmask(const SymbolTags STGS) { std::vector Tags; - if (STGS.empty()) + if (STGS == 0) return Tags; // No filtering required since this function is only used for Symbols from the @@ -335,7 +335,7 @@ std::vector getSymbolTags(const NamedDecl &ND) { SymbolTags FilteredTags = STGS; std::vector Tags; - if (STGS.empty()) + if (STGS == 0) return Tags; // Apply specific filter to the symbol tags only on CXX class methods. diff --git a/clang-tools-extra/clangd/index/Serialization.cpp b/clang-tools-extra/clangd/index/Serialization.cpp index e36171e0a1a75..764ec7903aecb 100644 --- a/clang-tools-extra/clangd/index/Serialization.cpp +++ b/clang-tools-extra/clangd/index/Serialization.cpp @@ -328,7 +328,7 @@ void writeSymbol(const Symbol &Sym, const StringTableOut &Strings, writeVar(Strings.index(Sym.Documentation), OS); writeVar(Strings.index(Sym.ReturnType), OS); writeVar(Strings.index(Sym.Type), OS); - writeVar(Sym.Tags.raw(), OS); + writeVar(Sym.Tags, OS); auto WriteInclude = [&](const Symbol::IncludeHeaderWithReferences &Include) { writeVar(Strings.index(Include.IncludeHeader), OS); @@ -358,7 +358,7 @@ Symbol readSymbol(Reader &Data, llvm::ArrayRef Strings, Sym.Documentation = Data.consumeString(Strings); Sym.ReturnType = Data.consumeString(Strings); Sym.Type = Data.consumeString(Strings); - Sym.Tags = SymbolTags::fromRaw(Data.consumeVar()); + Sym.Tags = Data.consumeVar(); if (!Data.consumeSize(Sym.IncludeHeaders)) return Sym; for (auto &I : Sym.IncludeHeaders) { diff --git a/clang-tools-extra/clangd/index/Symbol.h b/clang-tools-extra/clangd/index/Symbol.h index 780e7d1682c95..5f83953639a55 100644 --- a/clang-tools-extra/clangd/index/Symbol.h +++ b/clang-tools-extra/clangd/index/Symbol.h @@ -17,108 +17,20 @@ #include "llvm/ADT/BitmaskEnum.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/StringSaver.h" -#include -#include namespace clang { namespace clangd { LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); -/// Type-safe bitset for SymbolTag values. -/// -/// - The storage type adapts to SymbolTag's underlying type. -/// - Bits outside the supported [FirstTag..LastTag] range are dropped. -class SymbolTags { -public: - using Storage = std::make_unsigned_t>; - - constexpr SymbolTags() = default; - - static constexpr SymbolTags fromRaw(Storage Raw) { - SymbolTags Result; - Result.Bits = Raw & validMask(); - return Result; - } - - static constexpr SymbolTags fromTag(SymbolTag Tag) { - SymbolTags Result; - Result.set(Tag); - return Result; - } - - constexpr Storage raw() const { return Bits; } - constexpr bool empty() const { return Bits == 0; } - constexpr explicit operator bool() const { return !empty(); } - - constexpr bool contains(SymbolTag Tag) const { - return (Bits & bitMask(Tag)) != 0; - } - - constexpr void set(SymbolTag Tag) { Bits |= bitMask(Tag); } - constexpr void reset(SymbolTag Tag) { Bits &= ~bitMask(Tag); } - - constexpr SymbolTags &operator|=(SymbolTags Other) { - Bits = (Bits | Other.Bits) & validMask(); - return *this; - } - - constexpr SymbolTags &operator&=(SymbolTags Other) { - Bits &= Other.Bits; - return *this; - } - - friend constexpr SymbolTags operator|(SymbolTags L, SymbolTags R) { - L |= R; - return L; - } - - friend constexpr SymbolTags operator&(SymbolTags L, SymbolTags R) { - L &= R; - return L; - } - - friend constexpr SymbolTags operator~(SymbolTags V) { - return fromRaw(static_cast(~V.Bits)); - } - - friend constexpr bool operator==(SymbolTags L, SymbolTags R) { - return L.Bits == R.Bits; - } - -private: - static constexpr unsigned firstTag() { - return static_cast(SymbolTag::FirstTag); - } - - static constexpr unsigned lastTag() { - return static_cast(SymbolTag::LastTag); - } - - static constexpr Storage validMask() { - constexpr auto Digits = std::numeric_limits::digits; - const Storage Lower = - (firstTag() == 0) ? Storage{0} : ((Storage{1} << firstTag()) - 1); - const Storage Upper = (lastTag() + 1 == Digits) - ? ~Storage{0} - : ((Storage{1} << (lastTag() + 1)) - 1); - return Upper & ~Lower; - } - - static constexpr Storage bitMask(SymbolTag Tag) { - const auto Ordinal = static_cast(Tag); - return Storage{1} << Ordinal; - } - - Storage Bits = 0; -}; - -static_assert(static_cast(SymbolTag::FirstTag) <= - static_cast(SymbolTag::LastTag), - "SymbolTag::FirstTag must not exceed SymbolTag::LastTag"); -static_assert(static_cast(SymbolTag::LastTag) < - std::numeric_limits::digits, - "Too many SymbolTag values for SymbolTags::Storage"); +/// A bitmask type representing symbol tags supported by LSP. +/// \see +/// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#symbolTag +using SymbolTags = uint32_t; +/// Ensure we have enough bits to represent all SymbolTag values. +static_assert(static_cast(SymbolTag::LastTag) < 32, + "Too many SymbolTags to fit in uint32_t. Change to uint64_t if " + "we ever have more than 32 tags."); /// The class presents a C++ symbol, e.g. class, function. /// @@ -163,7 +75,7 @@ struct Symbol { /// Symbol tags for LSP protocol (Deprecated, Static, Virtual, Abstract, /// Final, ReadOnly, Public, Protected, Private, Declaration, Definition). /// This is a bitmask where each bit represents a SymbolTag. - SymbolTags Tags; + SymbolTags Tags = 0; /// A brief description of the symbol that can be appended in the completion /// candidate list. For example, "(X x, Y y) const" is a function signature. /// Only set when the symbol is indexed for completion. diff --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp b/clang-tools-extra/clangd/index/YAMLSerialization.cpp index 162e5d1287ba4..4d6b5822ece72 100644 --- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp +++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp @@ -86,28 +86,11 @@ using clang::clangd::RelationKind; using clang::clangd::Symbol; using clang::clangd::SymbolID; using clang::clangd::SymbolLocation; -using clang::clangd::SymbolTags; using clang::index::SymbolInfo; using clang::index::SymbolKind; using clang::index::SymbolLanguage; using clang::tooling::CompileCommand; -template <> struct ScalarTraits { - static void output(const SymbolTags &Value, void *, raw_ostream &Out) { - Out << Value.raw(); - } - - static StringRef input(StringRef Scalar, void *, SymbolTags &Value) { - SymbolTags::Storage Raw; - if (Scalar.getAsInteger(/*Radix=*/10, Raw)) - return "invalid SymbolTags value"; - Value = SymbolTags::fromRaw(Raw); - return {}; - } - - static QuotingType mustQuote(StringRef) { return QuotingType::None; } -}; - // Helper to (de)serialize the SymbolID. We serialize it as a hex string. struct NormalizedSymbolID { NormalizedSymbolID(IO &) {} diff --git a/clang-tools-extra/clangd/unittests/SerializationTests.cpp b/clang-tools-extra/clangd/unittests/SerializationTests.cpp index 7a62e2007fb31..6b95331c8f7c5 100644 --- a/clang-tools-extra/clangd/unittests/SerializationTests.cpp +++ b/clang-tools-extra/clangd/unittests/SerializationTests.cpp @@ -161,11 +161,10 @@ TEST(SerializationTest, YAMLConversions) { EXPECT_EQ(static_cast(Sym1.Flags), 129); EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion); EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated); - // Tags: Deprecated | Static - EXPECT_EQ(Sym1.Tags, SymbolTags::fromTag(SymbolTag::Deprecated) | - SymbolTags::fromTag(SymbolTag::Static)); - EXPECT_TRUE((Sym1.Tags & SymbolTags::fromTag(SymbolTag::Deprecated))); - EXPECT_TRUE((Sym1.Tags & SymbolTags::fromTag(SymbolTag::Static))); + // Tags: Deprecated (1<<1=2) | Static (1<<8=256) = 258 + EXPECT_EQ(Sym1.Tags, 258u); + EXPECT_TRUE(Sym1.Tags & (1u << static_cast(SymbolTag::Deprecated))); + EXPECT_TRUE(Sym1.Tags & (1u << static_cast(SymbolTag::Static))); EXPECT_THAT( Sym1.IncludeHeaders, UnorderedElementsAre( @@ -182,7 +181,7 @@ TEST(SerializationTest, YAMLConversions) { "file:///path/bar.h"); EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion); EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated); - EXPECT_TRUE(Sym2.Tags.empty()); // no Tags in YAML -> default zero + EXPECT_EQ(Sym2.Tags, 0u); // no Tags in YAML → default zero ASSERT_TRUE(bool(ParsedYAML->Refs)); EXPECT_THAT( @@ -303,7 +302,9 @@ TEST(SerializationTest, SrcsTest) { // This exercises both writeSymbol (new Tags field) and readSymbol. TEST(SerializationTest, TagsRoundTrip) { // YAML fixture with a single symbol carrying non-zero Tags. - // Combined SymbolTags value: Deprecated | Static. + // Deprecated (SymbolTag=1) -> bit 1 -> 1<<1 = 2 + // Static (SymbolTag=8) -> bit 8 -> 1<<8 = 256 + // Combined: 258 const char *TaggedYAML = R"( --- !Symbol @@ -327,8 +328,9 @@ Tags: 258 )"; const SymbolID TaggedID = cantFail(SymbolID::fromStr("AABBCCDDEEFF0011")); constexpr SymbolTags ExpectedTags = - SymbolTags::fromTag(SymbolTag::Deprecated) | - SymbolTags::fromTag(SymbolTag::Static); + (1u << static_cast(SymbolTag::Deprecated)) | // bit 1 = 2 + (1u << static_cast(SymbolTag::Static)); // bit 8 = 256 + static_assert(ExpectedTags == 258u, "bitmask sanity check"); // ── Step 1: YAML deserialization ──────────────────────────────────────── auto FromYAML = readIndexFile(TaggedYAML); @@ -356,9 +358,10 @@ Tags: 258 << "symbol not found after RIFF roundtrip"; EXPECT_EQ(It->Tags, ExpectedTags) << "Tags lost during RIFF serialization"; // Spot-check individual bits - EXPECT_TRUE(It->Tags & SymbolTags::fromTag(SymbolTag::Deprecated)); - EXPECT_TRUE(It->Tags & SymbolTags::fromTag(SymbolTag::Static)); - EXPECT_FALSE(It->Tags & SymbolTags::fromTag(SymbolTag::Abstract)); + EXPECT_TRUE(It->Tags & + (1u << static_cast(SymbolTag::Deprecated))); + EXPECT_TRUE(It->Tags & (1u << static_cast(SymbolTag::Static))); + EXPECT_FALSE(It->Tags & (1u << static_cast(SymbolTag::Abstract))); } // ── Step 3: Symbol with Tags=0 must survive too ───────────────────────── @@ -394,7 +397,7 @@ Flags: 1 const SymbolID UntaggedID = cantFail(SymbolID::fromStr("0000000000000001")); auto It = FromUntaggedRIFF->Symbols->find(UntaggedID); ASSERT_NE(It, FromUntaggedRIFF->Symbols->end()); - EXPECT_TRUE(It->Tags.empty()) << "Tags must be zero for untagged symbol"; + EXPECT_EQ(It->Tags, 0u) << "Tags must be zero for untagged symbol"; } } From 88b58006f74a607188109bae89d807a664e38350 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 29 Apr 2026 13:52:16 +0200 Subject: [PATCH 27/29] Reapply "Simplified filterSymbolTags:" This reverts commit c39d64b9d01a8ac3504d1f8ce403336a2a5696ca. --- clang-tools-extra/clangd/FindSymbols.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 9c5fee84ddd8b..0186945709e30 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -203,23 +203,20 @@ SymbolTags filterSymbolTags(SymbolTags ST) { const SymbolTags AbstractMask = toSymbolTagBitmask(SymbolTag::Abstract); const SymbolTags FinalMask = toSymbolTagBitmask(SymbolTag::Final); - // Implements implies Overrides + Virtual. + const SymbolTags RemoveVirtualAndOverrides = VirtualMask | OverridesMask; + + // Implements implies both Overrides and Virtual. if (ST & ImplementsMask) - ST &= ~(OverridesMask | VirtualMask); + ST &= ~RemoveVirtualAndOverrides; - // Overrides implies Virtual. - if (ST & OverridesMask) - ST &= ~VirtualMask; + // Final also suppresses both Virtual and Overrides in this model. + if (ST & FinalMask) + ST &= ~RemoveVirtualAndOverrides; - // Abstract implies Virtual. - if (ST & AbstractMask) + // Overrides or Abstract each imply Virtual. + if (ST & (OverridesMask | AbstractMask)) ST &= ~VirtualMask; - // Final implies Virtual; Overrides is also redundant as Final overrides are - // still overrides. - if (ST & FinalMask) - ST &= ~(VirtualMask | OverridesMask); - return ST; } From 0ffcf778a6e6724cf5103e348301b24054811487 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 29 Apr 2026 14:08:06 +0200 Subject: [PATCH 28/29] changed typecast from uint32_t to unsigned. --- clang-tools-extra/clangd/FindSymbols.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 0186945709e30..c21c9e16c1556 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -311,9 +311,9 @@ std::vector expandTagBitmask(const SymbolTags STGS) { // Iterate through SymbolTag enum values and collect any that are present in // the bitmask. SymbolTag values are in the numeric range // [FirstTag .. LastTag]. - constexpr uint32_t MinTag = static_cast(SymbolTag::FirstTag); - constexpr uint32_t MaxTag = static_cast(SymbolTag::LastTag); - for (uint32_t I = MinTag; I <= MaxTag; ++I) { + constexpr unsigned MinTag = static_cast(SymbolTag::FirstTag); + constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); + for (unsigned I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); if (STGS & toSymbolTagBitmask(ST)) Tags.push_back(ST); From c1005bb746a3cad489a7f4ac68410d1aa4dafec1 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 29 Apr 2026 14:56:57 +0200 Subject: [PATCH 29/29] changed typecast from uint32_t to unsigned. --- clang-tools-extra/clangd/FindSymbols.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index c21c9e16c1556..36d2e42307dea 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -240,7 +240,7 @@ template constexpr E enumIncrement(E Value) { } // namespace SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return (1 << static_cast(ST)); + return (1 << static_cast(ST)); } SymbolTags computeSymbolTags(const NamedDecl &ND) {