From f4f37212f33182f0f65a0f9ae0f4db58f94dedf4 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 10 Dec 2025 16:20:56 +0100 Subject: [PATCH 01/13] Determine the access method to a referenced symbol. The access method helps to know in a quick way, what is happening with the referenced symbol somewhere in the code, i.e. is the referenced symbol used as a rvalue object to read a value from or as lvalue object to write some value into. The basic access methods to a referenced symbol can be write and/or read. --- clang-tools-extra/clangd/Protocol.cpp | 9 +- clang-tools-extra/clangd/Protocol.h | 13 +++ clang-tools-extra/clangd/XRefs.cpp | 147 ++++++++++++++++++++++++-- 3 files changed, 158 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 560b8e00ed377..6027e60609fe2 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; @@ -1478,6 +1478,10 @@ llvm::json::Value toJSON(SymbolTag Tag) { return llvm::json::Value(static_cast(Tag)); } +llvm::json::Value toJSON(ReferenceTag Tag) { + return llvm::json::Value(static_cast(Tag)); +} + llvm::json::Value toJSON(const CallHierarchyItem &I) { llvm::json::Object Result{{"name", I.name}, {"kind", static_cast(I.kind)}, @@ -1490,6 +1494,9 @@ llvm::json::Value toJSON(const CallHierarchyItem &I) { Result["detail"] = I.detail; if (!I.data.empty()) Result["data"] = I.data; + if (!I.referenceTags.empty()) + Result["referenceTags"] = I.referenceTags; + return std::move(Result); } diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 2248572060431..edb28ef33d6a2 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -405,6 +405,13 @@ enum class SymbolKind { Operator = 25, TypeParameter = 26 }; + +/// Tags describing the reference kind. +enum class ReferenceTag { + Read = 1, + Write = 2, +}; + bool fromJSON(const llvm::json::Value &, SymbolKind &, llvm::json::Path); constexpr auto SymbolKindMin = static_cast(SymbolKind::File); constexpr auto SymbolKindMax = static_cast(SymbolKind::TypeParameter); @@ -1513,6 +1520,9 @@ struct TypeHierarchyItem { /// The kind of this item. SymbolKind kind; + /// The symbol tags for this item. + std::vector referenceTags; + /// More detail for this item, e.g. the signature of a function. std::optional detail; @@ -1590,6 +1600,9 @@ struct CallHierarchyItem { /// Tags for this item. std::vector tags; + /// The tags describing reference kinds of this item. + std::vector referenceTags; + /// More detaill for this item, e.g. the signature of a function. std::string detail; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index ef45acf501612..fd4dfe95213a5 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -68,6 +68,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" +#include "support/Logger.h" #include #include #include @@ -1749,6 +1750,130 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, return OS; } +// This would require a visitor pattern: +class ParamUsageVisitor : public RecursiveASTVisitor { +public: + bool HasWrite = false; + bool HasRead = false; + const ParmVarDecl *TargetParam; + + ParamUsageVisitor(const ParmVarDecl *P) : TargetParam(P) {} + + bool VisitUnaryOperator(UnaryOperator *UO) { + // Check for increment/decrement on parameter + if (UO->isIncrementDecrementOp()) { + if (auto *DRE = dyn_cast(UO->getSubExpr())) { + if (DRE->getDecl() == TargetParam) { + HasWrite = true; + } + } + } + return true; + } + + bool VisitBinaryOperator(BinaryOperator *BO) { + // Check for assignment to parameter + if (BO->isAssignmentOp()) { + if (auto *DRE = dyn_cast(BO->getLHS())) { + if (DRE->getDecl() == TargetParam) { + HasWrite = true; + } + } + } + // Any other use is at least a read + if (auto *DRE = dyn_cast(BO->getRHS())) { + if (DRE->getDecl() == TargetParam) { + HasRead = true; + } + } + return true; + } +}; + +static std::vector analyseParameterUsage(const FunctionDecl *FD) { + std::vector Result; + // This requires more sophisticated analysis - checking if param is modified + const Stmt *Body = FD->getBody(); + if (!Body) + return Result; // No definition available + + for (unsigned I = 0; I < FD->getNumParams(); ++I) { + const ParmVarDecl *Param = FD->getParamDecl(I); + + // Check const qualifier + // QualType ParamType = Param->getType(); + // bool IsReadOnly = ParamType.isConstQualified(); + + // For deeper analysis, you'd need to: + // 1. Walk the AST of the function body + // 2. Find all references to the parameter + // 3. Check if they appear on the left side of assignments (write) + // or only on the right side (read) + + ParamUsageVisitor Visitor(Param); + Visitor.TraverseStmt(const_cast(Body)); + if (Visitor.HasWrite) + Result.push_back(ReferenceTag::Write); + if (Visitor.HasRead) + Result.push_back(ReferenceTag::Read); + } + return Result; +} + +template +static void determineParameterUsage(const NamedDecl &ND, HierarchyItem &HI) { + // Get parent context and check if it's a function parameter + const DeclContext *DC = ND.getDeclContext(); + elog("determineParameterUsage: called for ND={0}", ND.getNameAsString()); + if (const auto *TD = llvm::dyn_cast(DC)) { + elog("determineParameterUsage: ND is inside a TagDecl: {0}", TD->getNameAsString()); + // No parameter analysis for TagDecl parent contexts. + } else if (const auto *FD = llvm::dyn_cast(DC)) { + elog("determineParameterUsage: ND is inside a FunctionDecl"); + for (unsigned I = 0; I < FD->getNumParams(); ++I) { + if (FD->getParamDecl(I) == &ND) { + elog("determineParameterUsage: ND is the {0}-th parameter of function {1}", I, FD->getNameAsString()); + + const ParmVarDecl *Param = FD->getParamDecl(I); + QualType ParamType = Param->getType(); + + bool IsConst = false; + bool IsConstRef = false; + bool IsConstPtr = false; + + // Check if const (read-only) + IsConst = ParamType.isConstQualified(); + elog("determineParameterUsage: ParamType.isConstQualified() = {0}", IsConst); + + // Check if it's a const reference + if (const auto *RT = ParamType->getAs()) { + IsConstRef = RT->getPointeeType().isConstQualified(); + elog("determineParameterUsage: ParamType is ReferenceType, isConstQualified = {0}", IsConstRef); + } + + // Check if it's a const pointer + if (const auto *PT = ParamType->getAs()) { + IsConstPtr = PT->getPointeeType().isConstQualified(); + elog("determineParameterUsage: ParamType is PointerType, isConstQualified = {0}", IsConstPtr); + } + + if (IsConst && IsConstRef && IsConstPtr) { + elog("determineParameterUsage: All const checks passed, marking as Read"); + HI.referenceTags.push_back(ReferenceTag::Read); + } else { + elog("determineParameterUsage: Performing analyseParameterUsage"); + HI.referenceTags = analyseParameterUsage(FD); + } + + break; + } + elog("determineParameterUsage: ND is not parameter {0} of function {1}", I, FD->getNameAsString()); + } + } else { + elog("determineParameterUsage: ND is not inside a FunctionDecl or TagDecl"); + } +} + template static std::optional declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { @@ -1790,6 +1915,8 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { HI.range = HI.selectionRange; } + determineParameterUsage(ND, HI); + HI.uri = URIForFile::canonicalize(*FilePath, TUPath); return HI; @@ -2097,15 +2224,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. @@ -2139,8 +2266,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; @@ -2162,9 +2289,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())) From 2c2a42c3c95cedcde8bd06ff23c68efc753ea5ab Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Fri, 2 Jan 2026 14:02:50 +0100 Subject: [PATCH 02/13] Added Visitor to traverse AST to detemine access type of expressions. --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 2 +- clang-tools-extra/clangd/ClangdServer.cpp | 13 +- clang-tools-extra/clangd/ClangdServer.h | 2 +- clang-tools-extra/clangd/Protocol.cpp | 2 +- clang-tools-extra/clangd/XRefs.cpp | 174 +++++++++--------- clang-tools-extra/clangd/XRefs.h | 2 +- .../clangd/unittests/CallHierarchyTests.cpp | 137 +++++++++++--- 7 files changed, 212 insertions(+), 120 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 1518f177b06a0..22aa6eeb581bd 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 ac1e9aa5f0ff1..4d8f3f4bd780d 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -908,12 +908,15 @@ void ClangdServer::prepareCallHierarchy( } void ClangdServer::incomingCalls( - const CallHierarchyItem &Item, + 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 d45d13fa034b5..20ca6bcc221d0 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -299,7 +299,7 @@ class ClangdServer { Callback> CB); /// Resolve incoming calls for a given call hierarchy item. - void incomingCalls(const CallHierarchyItem &Item, + void incomingCalls(PathRef File, const CallHierarchyItem &Item, Callback>); /// Resolve outgoing calls for a given call hierarchy item. diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 6027e60609fe2..7786f62b4347d 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -1496,7 +1496,7 @@ llvm::json::Value toJSON(const CallHierarchyItem &I) { Result["data"] = I.data; if (!I.referenceTags.empty()) Result["referenceTags"] = I.referenceTags; - + return std::move(Result); } diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index fd4dfe95213a5..d3dacee945ff7 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1755,10 +1755,18 @@ class ParamUsageVisitor : public RecursiveASTVisitor { public: bool HasWrite = false; bool HasRead = false; - const ParmVarDecl *TargetParam; + const ValueDecl *TargetParam; - ParamUsageVisitor(const ParmVarDecl *P) : TargetParam(P) {} + ParamUsageVisitor(const ValueDecl *P) : TargetParam(P) {} + // Any reference to the target is at least a read, unless we later + // identify it specifically as a write (e.g. assignment/inc/dec on LHS). + bool VisitDeclRefExpr(DeclRefExpr *DRE) { + if (DRE->getDecl() == TargetParam) + HasRead = true; + return true; + } + bool VisitUnaryOperator(UnaryOperator *UO) { // Check for increment/decrement on parameter if (UO->isIncrementDecrementOp()) { @@ -1774,14 +1782,17 @@ class ParamUsageVisitor : public RecursiveASTVisitor { bool VisitBinaryOperator(BinaryOperator *BO) { // Check for assignment to parameter if (BO->isAssignmentOp()) { - if (auto *DRE = dyn_cast(BO->getLHS())) { + if (isa(BO->getLHS())) { + auto *DRE = dyn_cast(BO->getLHS()); if (DRE->getDecl() == TargetParam) { HasWrite = true; } } } + // Any other use is at least a read - if (auto *DRE = dyn_cast(BO->getRHS())) { + if (isa(BO->getRHS())) { + auto *DRE = dyn_cast(BO->getRHS()); if (DRE->getDecl() == TargetParam) { HasRead = true; } @@ -1790,90 +1801,25 @@ class ParamUsageVisitor : public RecursiveASTVisitor { } }; -static std::vector analyseParameterUsage(const FunctionDecl *FD) { +static std::vector analyseParameterUsage(const FunctionDecl *FD, + const ValueDecl *PVD) { std::vector Result; - // This requires more sophisticated analysis - checking if param is modified const Stmt *Body = FD->getBody(); if (!Body) return Result; // No definition available - for (unsigned I = 0; I < FD->getNumParams(); ++I) { - const ParmVarDecl *Param = FD->getParamDecl(I); + // Walk the body and determine read/write usage of the referenced variable + // within this function. + ParamUsageVisitor Visitor(PVD); + Visitor.TraverseStmt(const_cast(Body)); + if (Visitor.HasWrite) + Result.push_back(ReferenceTag::Write); + if (Visitor.HasRead) + Result.push_back(ReferenceTag::Read); - // Check const qualifier - // QualType ParamType = Param->getType(); - // bool IsReadOnly = ParamType.isConstQualified(); - - // For deeper analysis, you'd need to: - // 1. Walk the AST of the function body - // 2. Find all references to the parameter - // 3. Check if they appear on the left side of assignments (write) - // or only on the right side (read) - - ParamUsageVisitor Visitor(Param); - Visitor.TraverseStmt(const_cast(Body)); - if (Visitor.HasWrite) - Result.push_back(ReferenceTag::Write); - if (Visitor.HasRead) - Result.push_back(ReferenceTag::Read); - } return Result; } -template -static void determineParameterUsage(const NamedDecl &ND, HierarchyItem &HI) { - // Get parent context and check if it's a function parameter - const DeclContext *DC = ND.getDeclContext(); - elog("determineParameterUsage: called for ND={0}", ND.getNameAsString()); - if (const auto *TD = llvm::dyn_cast(DC)) { - elog("determineParameterUsage: ND is inside a TagDecl: {0}", TD->getNameAsString()); - // No parameter analysis for TagDecl parent contexts. - } else if (const auto *FD = llvm::dyn_cast(DC)) { - elog("determineParameterUsage: ND is inside a FunctionDecl"); - for (unsigned I = 0; I < FD->getNumParams(); ++I) { - if (FD->getParamDecl(I) == &ND) { - elog("determineParameterUsage: ND is the {0}-th parameter of function {1}", I, FD->getNameAsString()); - - const ParmVarDecl *Param = FD->getParamDecl(I); - QualType ParamType = Param->getType(); - - bool IsConst = false; - bool IsConstRef = false; - bool IsConstPtr = false; - - // Check if const (read-only) - IsConst = ParamType.isConstQualified(); - elog("determineParameterUsage: ParamType.isConstQualified() = {0}", IsConst); - - // Check if it's a const reference - if (const auto *RT = ParamType->getAs()) { - IsConstRef = RT->getPointeeType().isConstQualified(); - elog("determineParameterUsage: ParamType is ReferenceType, isConstQualified = {0}", IsConstRef); - } - - // Check if it's a const pointer - if (const auto *PT = ParamType->getAs()) { - IsConstPtr = PT->getPointeeType().isConstQualified(); - elog("determineParameterUsage: ParamType is PointerType, isConstQualified = {0}", IsConstPtr); - } - - if (IsConst && IsConstRef && IsConstPtr) { - elog("determineParameterUsage: All const checks passed, marking as Read"); - HI.referenceTags.push_back(ReferenceTag::Read); - } else { - elog("determineParameterUsage: Performing analyseParameterUsage"); - HI.referenceTags = analyseParameterUsage(FD); - } - - break; - } - elog("determineParameterUsage: ND is not parameter {0} of function {1}", I, FD->getNameAsString()); - } - } else { - elog("determineParameterUsage: ND is not inside a FunctionDecl or TagDecl"); - } -} - template static std::optional declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { @@ -1904,7 +1850,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())}; @@ -1915,7 +1861,7 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { HI.range = HI.selectionRange; } - determineParameterUsage(ND, HI); + //determineParameterUsage(ND, HI); HI.uri = URIForFile::canonicalize(*FilePath, TUPath); @@ -2464,8 +2410,38 @@ prepareCallHierarchy(ParsedAST &AST, Position Pos, 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]; +} + std::vector -incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { +incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, ParsedAST &AST) { std::vector Results; if (!Index || Item.data.empty()) return Results; @@ -2474,6 +2450,24 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { elog("incomingCalls failed to find symbol: {0}", ID.takeError()); return Results; } + + + LookupRequest LR; + LR.IDs.insert(*ID); + + std::optional PVD; + Index->lookup(LR, [&ID, &AST, &PVD](const Symbol &Sym) { + // This callback is called once per found symbol; here we expect exactly one + if (Sym.ID == *ID) { + PVD = getNamedDeclFromSymbol(Sym, AST); + } + }); + + if (PVD == nullptr || !PVD.has_value()) { + // Not found in index + return Results; + } + // In this function, we find incoming calls based on the index only. // In principle, the AST could have more up-to-date information about // occurrences within the current file. However, going from a SymbolID @@ -2510,7 +2504,21 @@ 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())) { + + std::optional CHI; + if (auto *ND = getNamedDeclFromSymbol(Caller, AST)) { + CHI = declToCallHierarchyItem(*ND, AST.tuPath()); + if (const auto *FD = llvm::dyn_cast(ND)) { + if (isa(PVD.value())) { + const auto *VD = llvm::dyn_cast(PVD.value()); + CHI->referenceTags = analyseParameterUsage(FD, VD); // FD is the caller of var + } + } + } else { + CHI = symbolToCallHierarchyItem(Caller, Item.uri.file()); + } + + if (CHI) { std::vector FromRanges; for (const Location &L : It->second) { if (L.uri != CHI->uri) { diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 247e52314c3f9..911cf72d72f03 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -148,7 +148,7 @@ 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, 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..ad5c7d06a0739 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,12 +664,93 @@ 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"))))); } +TEST(CallHierarchy, HierarchyOnVarWithReadReference) { + // Tests that the call hierarchy works on non-local variables and read/write tags are set. + Annotations Source(R"cpp( + int v^ar = 1; + void caller() { + int x = 0; + x = var; + } + )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("var"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Read); +} + +TEST(CallHierarchy, HierarchyOnVarWithWriteReference) { + // Tests that the call hierarchy works on non-local variables and read/write tags are set. + Annotations Source(R"cpp( + int v^ar = 1; + void caller() { + var = 2; + } + )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("var"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Write); +} + +TEST(CallHierarchy, HierarchyOnVarWithUnaryWriteReference) { + // Tests that the call hierarchy works on non-local variables and read/write tags are set. + Annotations Source(R"cpp( + int v^ar = 1; + void caller() { + var++; + } + )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("var"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Write); +} + +// TEST(CallHierarchy, HierarchyOnFieldWithReadWriteReference) { +// // Tests that the call hierarchy works on non-local variables and read/write tags are set. +// Annotations Source(R"cpp( +// struct Vars { +// int v^ar1 = 1; +// }; +// void caller() { +// Vars values; +// values.var1 = 2; +// } +// )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("var1"))); +// auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); +// EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Read); +// //EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(1) == ReferenceTag::Read); +// } + TEST(CallHierarchy, HierarchyOnEnumConstant) { // Tests that the call hierarchy works on enum constants. Annotations Source(R"cpp( @@ -686,14 +767,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 +799,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 98e1167c36c1b4a3b62b779c3647d9ea6e99606a Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 29 Jan 2026 19:09:15 +0100 Subject: [PATCH 03/13] Fix Decl check. - Fix lit tests - Fix unit tests. --- clang-tools-extra/clangd/XRefs.cpp | 34 +++++---- clang-tools-extra/clangd/XRefs.h | 3 +- .../clangd/test/call-hierarchy.test | 1 + .../clangd/test/type-hierarchy-ext.test | 3 + .../clangd/test/type-hierarchy.test | 1 + .../clangd/unittests/CallHierarchyTests.cpp | 74 ++++++++++--------- 6 files changed, 63 insertions(+), 53 deletions(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index d3dacee945ff7..69947c77238f6 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -68,7 +68,6 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" -#include "support/Logger.h" #include #include #include @@ -1756,9 +1755,9 @@ class ParamUsageVisitor : public RecursiveASTVisitor { bool HasWrite = false; bool HasRead = false; const ValueDecl *TargetParam; - + ParamUsageVisitor(const ValueDecl *P) : TargetParam(P) {} - + // Any reference to the target is at least a read, unless we later // identify it specifically as a write (e.g. assignment/inc/dec on LHS). bool VisitDeclRefExpr(DeclRefExpr *DRE) { @@ -1778,7 +1777,6 @@ class ParamUsageVisitor : public RecursiveASTVisitor { } return true; } - bool VisitBinaryOperator(BinaryOperator *BO) { // Check for assignment to parameter if (BO->isAssignmentOp()) { @@ -1861,8 +1859,6 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { HI.range = HI.selectionRange; } - //determineParameterUsage(ND, HI); - HI.uri = URIForFile::canonicalize(*FilePath, TUPath); return HI; @@ -2441,7 +2437,8 @@ const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, } std::vector -incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, ParsedAST &AST) { +incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + ParsedAST &AST) { std::vector Results; if (!Index || Item.data.empty()) return Results; @@ -2451,20 +2448,22 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, ParsedAST return Results; } - LookupRequest LR; LR.IDs.insert(*ID); - std::optional PVD; - Index->lookup(LR, [&ID, &AST, &PVD](const Symbol &Sym) { + std::optional Decl; + Index->lookup(LR, [&ID, &AST, &Decl](const Symbol &Sym) { // This callback is called once per found symbol; here we expect exactly one if (Sym.ID == *ID) { - PVD = getNamedDeclFromSymbol(Sym, AST); + Decl = getNamedDeclFromSymbol(Sym, AST); } }); - if (PVD == nullptr || !PVD.has_value()) { - // Not found in index + // Note: Decl may be nullptr if the symbol is in a header file (not the main + // file). In that case, we still want to continue and use index-based + // resolution. + if (!Decl.has_value()) { + // Symbol not found in index return Results; } @@ -2509,9 +2508,12 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, ParsedAST if (auto *ND = getNamedDeclFromSymbol(Caller, AST)) { CHI = declToCallHierarchyItem(*ND, AST.tuPath()); if (const auto *FD = llvm::dyn_cast(ND)) { - if (isa(PVD.value())) { - const auto *VD = llvm::dyn_cast(PVD.value()); - CHI->referenceTags = analyseParameterUsage(FD, VD); // FD is the caller of var + if (Decl.has_value() && Decl.value() != nullptr) { + if (const auto *VD = + llvm::dyn_cast(Decl.value())) { + CHI->referenceTags = + analyseParameterUsage(FD, VD); // FD is the caller of var + } } } } else { diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 911cf72d72f03..31e7b0b14440f 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, ParsedAST &AST); +incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + ParsedAST &AST); std::vector outgoingCalls(const CallHierarchyItem &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 6548ea0068a8d..44c89d22d9c85 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 a5f13ab13d0b3..cf52f7af1ec61 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": { diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index ad5c7d06a0739..15387b10aad50 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -67,6 +67,12 @@ ::testing::Matcher oFromRanges(RangeMatchers... M) { UnorderedElementsAre(M...)); } +template +::testing::Matcher withReferenceTags(References... refs) { + return Field(&CallHierarchyItem::referenceTags, + UnorderedElementsAre(refs...)); +} + TEST(CallHierarchy, IncomingOneFileCpp) { Annotations Source(R"cpp( void call^ee(int); @@ -402,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( @@ -411,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()); }; @@ -558,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"), @@ -566,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()); }; @@ -671,7 +683,8 @@ TEST(CallHierarchy, HierarchyOnVar) { } TEST(CallHierarchy, HierarchyOnVarWithReadReference) { - // Tests that the call hierarchy works on non-local variables and read/write tags are set. + // Tests that the call hierarchy works on non-local variables and a read is + // set. Annotations Source(R"cpp( int v^ar = 1; void caller() { @@ -687,11 +700,14 @@ TEST(CallHierarchy, HierarchyOnVarWithReadReference) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("var"))); auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); - EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Read); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT( + IncomingLevel1, + ElementsAre(AllOf(from( + AllOf(withName("caller"), withReferenceTags(ReferenceTag::Read)))))); } TEST(CallHierarchy, HierarchyOnVarWithWriteReference) { - // Tests that the call hierarchy works on non-local variables and read/write tags are set. Annotations Source(R"cpp( int v^ar = 1; void caller() { @@ -706,11 +722,15 @@ TEST(CallHierarchy, HierarchyOnVarWithWriteReference) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("var"))); auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); - EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Write); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT( + IncomingLevel1, + ElementsAre(AllOf(from( + AllOf(withName("caller"), + withReferenceTags(ReferenceTag::Write, ReferenceTag::Read)))))); } -TEST(CallHierarchy, HierarchyOnVarWithUnaryWriteReference) { - // Tests that the call hierarchy works on non-local variables and read/write tags are set. +TEST(CallHierarchy, HierarchyOnVarWithUnaryReadWriteReference) { Annotations Source(R"cpp( int v^ar = 1; void caller() { @@ -725,32 +745,14 @@ TEST(CallHierarchy, HierarchyOnVarWithUnaryWriteReference) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("var"))); auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); - EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Write); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT( + IncomingLevel1, + UnorderedElementsAre(AllOf(from( + AllOf(withName("caller"), + withReferenceTags(ReferenceTag::Write, ReferenceTag::Read)))))); } -// TEST(CallHierarchy, HierarchyOnFieldWithReadWriteReference) { -// // Tests that the call hierarchy works on non-local variables and read/write tags are set. -// Annotations Source(R"cpp( -// struct Vars { -// int v^ar1 = 1; -// }; -// void caller() { -// Vars values; -// values.var1 = 2; -// } -// )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("var1"))); -// auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); -// EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(0) == ReferenceTag::Read); -// //EXPECT_TRUE(IncomingLevel1.front().from.referenceTags.at(1) == ReferenceTag::Read); -// } - TEST(CallHierarchy, HierarchyOnEnumConstant) { // Tests that the call hierarchy works on enum constants. Annotations Source(R"cpp( From 37759683319008b27bc89626f8224aa3d640554f Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 2 Feb 2026 23:22:07 +0100 Subject: [PATCH 04/13] Fix: expected result in write-reference test. --- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index 15387b10aad50..0ec58a7602b11 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -726,8 +726,8 @@ TEST(CallHierarchy, HierarchyOnVarWithWriteReference) { EXPECT_THAT( IncomingLevel1, ElementsAre(AllOf(from( - AllOf(withName("caller"), - withReferenceTags(ReferenceTag::Write, ReferenceTag::Read)))))); + AllOf(withName("caller"), withReferenceTags(ReferenceTag::Write)))))); +} } TEST(CallHierarchy, HierarchyOnVarWithUnaryReadWriteReference) { From e30941813a45fe04f0550dde1139184791ad8208 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 2 Feb 2026 23:24:48 +0100 Subject: [PATCH 05/13] New test: caller within a class. --- .../clangd/unittests/CallHierarchyTests.cpp | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index 0ec58a7602b11..5582997148b33 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -728,6 +728,30 @@ TEST(CallHierarchy, HierarchyOnVarWithWriteReference) { ElementsAre(AllOf(from( AllOf(withName("caller"), withReferenceTags(ReferenceTag::Write)))))); } + +TEST(CallHierarchy, HierarchyOnClassMemberWithWriteReference) { + Annotations Source(R"cpp( + int v^ar = 1; + class MyClass { + public: + void caller() { + var = 2; + } + }; + )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("var"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT( + IncomingLevel1, + ElementsAre(AllOf(from( + AllOf(withName("caller"), withReferenceTags(ReferenceTag::Write)))))); } TEST(CallHierarchy, HierarchyOnVarWithUnaryReadWriteReference) { From b41a24ecc1aebd0d2f63c97a7d022c8bfd775c2f Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 2 Feb 2026 23:27:46 +0100 Subject: [PATCH 06/13] Replaced Visitor methods for unary and binary operations with Traverse. Try to get the function definition before calling analyseParameterUsage. Visit member expressions. --- clang-tools-extra/clangd/XRefs.cpp | 74 ++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 69947c77238f6..668eade7eb084 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1755,45 +1755,64 @@ class ParamUsageVisitor : public RecursiveASTVisitor { bool HasWrite = false; bool HasRead = false; const ValueDecl *TargetParam; + llvm::SmallSet WriteOnlyExprs; ParamUsageVisitor(const ValueDecl *P) : TargetParam(P) {} - // Any reference to the target is at least a read, unless we later - // identify it specifically as a write (e.g. assignment/inc/dec on LHS). - bool VisitDeclRefExpr(DeclRefExpr *DRE) { - if (DRE->getDecl() == TargetParam) - HasRead = true; - return true; - } - - bool VisitUnaryOperator(UnaryOperator *UO) { - // Check for increment/decrement on parameter - if (UO->isIncrementDecrementOp()) { - if (auto *DRE = dyn_cast(UO->getSubExpr())) { + // Identify write-only contexts before visiting children + bool TraverseBinaryOperator(BinaryOperator *BO) { + if (BO->isAssignmentOp()) { + auto *LHS = BO->getLHS()->IgnoreParenImpCasts(); + if (auto *DRE = dyn_cast(LHS)) { if (DRE->getDecl() == TargetParam) { HasWrite = true; + // For pure assignment (=), LHS is write-only + if (BO->getOpcode() == BO_Assign) + WriteOnlyExprs.insert(DRE); + } + } else if (auto *ME = dyn_cast(LHS)) { + if (ME->getMemberDecl() == TargetParam) { + HasWrite = true; + if (BO->getOpcode() == BO_Assign) + WriteOnlyExprs.insert(ME); } } } - return true; + // Continue with default traversal + return RecursiveASTVisitor::TraverseBinaryOperator(BO); } - bool VisitBinaryOperator(BinaryOperator *BO) { - // Check for assignment to parameter - if (BO->isAssignmentOp()) { - if (isa(BO->getLHS())) { - auto *DRE = dyn_cast(BO->getLHS()); + + bool TraverseUnaryOperator(UnaryOperator *UO) { + if (UO->isIncrementDecrementOp()) { + auto *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts(); + if (auto *DRE = dyn_cast(SubExpr)) { if (DRE->getDecl() == TargetParam) { HasWrite = true; + // Inc/Dec is both read and write, don't add to WriteOnlyExprs + } + } else if (auto *ME = dyn_cast(SubExpr)) { + if (ME->getMemberDecl() == TargetParam) { + HasWrite = true; } } } + return RecursiveASTVisitor::TraverseUnaryOperator(UO); + } - // Any other use is at least a read - if (isa(BO->getRHS())) { - auto *DRE = dyn_cast(BO->getRHS()); - if (DRE->getDecl() == TargetParam) { + // Check for reads, excluding write-only contexts + bool VisitDeclRefExpr(DeclRefExpr *DRE) { + if (DRE->getDecl() == TargetParam) { + if (!WriteOnlyExprs.count(DRE)) + HasRead = true; + } + return true; + } + + // Handle member expressions + bool VisitMemberExpr(MemberExpr *ME) { + if (ME->getMemberDecl() == TargetParam) { + if (!WriteOnlyExprs.count(ME)) HasRead = true; - } } return true; } @@ -2511,8 +2530,13 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, if (Decl.has_value() && Decl.value() != nullptr) { if (const auto *VD = llvm::dyn_cast(Decl.value())) { - CHI->referenceTags = - analyseParameterUsage(FD, VD); // FD is the caller of var + // Use the function definition if available, not just a + // declaration + const FunctionDecl *FuncDef = FD->getDefinition(); + if (!FuncDef) + FuncDef = FD; + CHI->referenceTags = analyseParameterUsage( + FuncDef, VD); // FuncDef is the caller of value decl VD } } } From a6e99c5e0ae8ad2f731ee063dea6d2c070db181c Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Sun, 15 Feb 2026 22:26:47 +0100 Subject: [PATCH 07/13] New: traverse ctor declaration and corresponding unit test. --- clang-tools-extra/clangd/XRefs.cpp | 27 +++++++++---- .../clangd/unittests/CallHierarchyTests.cpp | 39 +++++++++++++++---- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 668eade7eb084..5f02eb9324e1e 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1782,6 +1782,16 @@ class ParamUsageVisitor : public RecursiveASTVisitor { return RecursiveASTVisitor::TraverseBinaryOperator(BO); } + bool TraverseConstructorInitializer(CXXCtorInitializer *Init) { + if (Init) { + if (const FieldDecl *FD = Init->getMember()) { + if (FD == TargetParam) + HasWrite = true; + } + } + return RecursiveASTVisitor::TraverseConstructorInitializer(Init); + } + bool TraverseUnaryOperator(UnaryOperator *UO) { if (UO->isIncrementDecrementOp()) { auto *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts(); @@ -1821,14 +1831,17 @@ class ParamUsageVisitor : public RecursiveASTVisitor { static std::vector analyseParameterUsage(const FunctionDecl *FD, const ValueDecl *PVD) { std::vector Result; - const Stmt *Body = FD->getBody(); - if (!Body) - return Result; // No definition available - - // Walk the body and determine read/write usage of the referenced variable - // within this function. ParamUsageVisitor Visitor(PVD); - Visitor.TraverseStmt(const_cast(Body)); + + if (const auto *Ctor = dyn_cast(FD)) { + Visitor.TraverseDecl(const_cast(Ctor)); + } else if (const Stmt *Body = FD->getBody()) { + // Walk the body and determine read/write usage of the referenced variable + // within this function. + Visitor.TraverseStmt(const_cast(Body)); + } else + return Result; + if (Visitor.HasWrite) Result.push_back(ReferenceTag::Write); if (Visitor.HasRead) diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index 5582997148b33..72fb35c5ca027 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -731,14 +731,14 @@ TEST(CallHierarchy, HierarchyOnVarWithWriteReference) { TEST(CallHierarchy, HierarchyOnClassMemberWithWriteReference) { Annotations Source(R"cpp( - int v^ar = 1; - class MyClass { - public: - void caller() { - var = 2; - } - }; - )cpp"); + class MyClass { + public: + void caller() { + var = 2; + } + int v^ar = 1; + }; + )cpp"); TestTU TU = TestTU::withCode(Source.code()); auto AST = TU.build(); auto Index = TU.index(); @@ -754,6 +754,29 @@ TEST(CallHierarchy, HierarchyOnClassMemberWithWriteReference) { AllOf(withName("caller"), withReferenceTags(ReferenceTag::Write)))))); } +TEST(CallHierarchy, HierarchyOnClassMemberWithWriteReferenceInCtorInitList) { + Annotations Source(R"cpp( + class MyClass { + int v^ar; + public: + MyClass() : var(1) {} + }; + )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("var"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT( + IncomingLevel1, + ElementsAre(AllOf(from(AllOf(withName("MyClass"), + withReferenceTags(ReferenceTag::Write)))))); +} + TEST(CallHierarchy, HierarchyOnVarWithUnaryReadWriteReference) { Annotations Source(R"cpp( int v^ar = 1; From c8d916f29e66bb1d4ccda8adf8150fcc6bc273ed Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 7 May 2026 11:03:53 +0200 Subject: [PATCH 08/13] [clangd] Refine incoming call hierarchy reference tags Honor the client's referenceTags capability for callHierarchy/\nincomingCalls, recover target decls for header symbols when AST\nlookup falls back to the index, and treat compound assignments\nas both read and write accesses. --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 4 +- clang-tools-extra/clangd/ClangdLSPServer.h | 2 + clang-tools-extra/clangd/ClangdServer.cpp | 7 +- clang-tools-extra/clangd/ClangdServer.h | 1 + clang-tools-extra/clangd/Protocol.cpp | 9 +- clang-tools-extra/clangd/Protocol.h | 17 +- clang-tools-extra/clangd/XRefs.cpp | 199 +++++++++++++----- clang-tools-extra/clangd/XRefs.h | 2 +- .../clangd/unittests/CallHierarchyTests.cpp | 77 +++++++ .../clangd/unittests/ClangdLSPServerTests.cpp | 69 +++++- 10 files changed, 323 insertions(+), 64 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 0a57573ecc469..25603c9ed1ebe 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -537,6 +537,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, SupportsHierarchicalDocumentSymbol = Params.capabilities.HierarchicalDocumentSymbol; SupportsReferenceContainer = Params.capabilities.ReferenceContainer; + SupportsReferenceTags = Params.capabilities.ReferenceTagsSupport; SupportFileStatus = Params.initializationOptions.FileStatus; SupportsDocumentChanges = Params.capabilities.DocumentChanges; SupportsChangeAnnotation = Params.capabilities.ChangeAnnotation; @@ -1389,7 +1390,8 @@ 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.uri.file(), Params.item, + SupportsReferenceTags, std::move(Reply)); } void ClangdLSPServer::onClangdInlayHints(const InlayHintsParams &Params, diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 6ada3fd9e6e47..860632476cbe4 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -299,6 +299,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks, bool SupportFileStatus = false; /// Whether the client supports attaching a container string to references. bool SupportsReferenceContainer = false; + /// Whether the client supports reference tags on hierarchy/reference items. + bool SupportsReferenceTags = false; /// Which kind of markup should we use in textDocument/hover responses. MarkupKind HoverContentFormat = MarkupKind::PlainText; /// Whether the client supports offsets for parameter info labels. diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index dc81fe3df2794..267e05b71fc8d 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -912,13 +912,14 @@ void ClangdServer::prepareCallHierarchy( } void ClangdServer::incomingCalls( - PathRef File, const CallHierarchyItem &Item, + PathRef File, const CallHierarchyItem &Item, bool ComputeReferenceTags, Callback> CB) { - auto Action = [Item, CB = std::move(CB), + auto Action = [Item, ComputeReferenceTags, CB = std::move(CB), this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - CB(clangd::incomingCalls(Item, Index, InpAST->AST)); + CB(clangd::incomingCalls(Item, Index, InpAST->AST, + ComputeReferenceTags)); }; WorkScheduler->runWithAST("Incoming Calls", File, std::move(Action)); } diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 8fe2c017033e9..1989969331259 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -303,6 +303,7 @@ class ClangdServer { /// Resolve incoming calls for a given call hierarchy item. void incomingCalls(PathRef File, const CallHierarchyItem &Item, + bool ComputeReferenceTags, Callback>); /// Resolve outgoing calls for a given call hierarchy item. diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 6e0336122af23..60dddfcea62d8 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -398,9 +398,16 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R, if (auto RelatedInfo = Diagnostics->getBoolean("relatedInformation")) R.DiagnosticRelatedInformation = *RelatedInfo; } - if (auto *References = TextDocument->getObject("references")) + if (auto *References = TextDocument->getObject("references")) { if (auto ContainerSupport = References->getBoolean("container")) R.ReferenceContainer = *ContainerSupport; + if (auto ItemsSupport = References->getBoolean("referenceItemsSupport")) + R.ReferenceItemsSupport = *ItemsSupport; + } + if (auto *CallHierarchy = TextDocument->getObject("callHierarchy")) { + if (auto TagsSupport = CallHierarchy->getBoolean("referenceTagsSupport")) + R.ReferenceTagsSupport = *TagsSupport; + } if (auto *Completion = TextDocument->getObject("completion")) { if (auto *Item = Completion->getObject("completionItem")) { if (auto SnippetSupport = Item->getBoolean("snippetSupport")) diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index d86ffe360f995..56738e3e93d57 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -575,6 +575,21 @@ struct ClientCapabilities { /// notification. This is a clangd extension. /// textDocument.inactiveRegionsCapabilities.inactiveRegions bool InactiveRegions = false; + + /// Determines whether the client supports reference tags on call hierarchy + /// items. If the value is missing, the server assumes that the client does + /// not support reference tags. + /// textDocument.callHierarchy.referenceTagsSupport + /// @since 3.18.0 + bool ReferenceTagsSupport = false; + + /// Determines whether the client supports and prefers Reference items instead + /// of Location items. If this value is missing, the server assumes that the + /// client accepts Location items as defined in earlier versions of the + /// protocol. + /// textDocument.references.referenceItemsSupport + /// @since 3.18.0 + bool ReferenceItemsSupport = false; }; bool fromJSON(const llvm::json::Value &, ClientCapabilities &, llvm::json::Path); @@ -1555,8 +1570,6 @@ struct TypeHierarchyItem { /// The kind of this item. SymbolKind kind; - /// The symbol tags for this item. - std::vector referenceTags; /// 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 eac2d26593831..2cbcd89286dc8 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1788,43 +1788,79 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, return OS; } -// This would require a visitor pattern: +// Visitor to determine read/write access patterns of a target declaration +// within a function body. class ParamUsageVisitor : public RecursiveASTVisitor { public: bool HasWrite = false; bool HasRead = false; const ValueDecl *TargetParam; + std::optional TargetID; llvm::SmallSet WriteOnlyExprs; - ParamUsageVisitor(const ValueDecl *P) : TargetParam(P) {} + ParamUsageVisitor(const ValueDecl *P) + : TargetParam(P ? llvm::dyn_cast(P->getCanonicalDecl()) + : nullptr), + TargetID(P ? getSymbolID(P) : std::optional{}) {} - // Identify write-only contexts before visiting children - bool TraverseBinaryOperator(BinaryOperator *BO) { - if (BO->isAssignmentOp()) { - auto *LHS = BO->getLHS()->IgnoreParenImpCasts(); - if (auto *DRE = dyn_cast(LHS)) { - if (DRE->getDecl() == TargetParam) { - HasWrite = true; - // For pure assignment (=), LHS is write-only - if (BO->getOpcode() == BO_Assign) - WriteOnlyExprs.insert(DRE); - } - } else if (auto *ME = dyn_cast(LHS)) { - if (ME->getMemberDecl() == TargetParam) { - HasWrite = true; - if (BO->getOpcode() == BO_Assign) - WriteOnlyExprs.insert(ME); - } + bool isTarget(const ValueDecl *VD) const { + if (!VD) + return false; + + if (TargetParam) { + if (const auto *Canonical = + llvm::dyn_cast(VD->getCanonicalDecl())) + if (Canonical == TargetParam) + return true; + } + + if (TargetID) { + if (auto ID = getSymbolID(VD)) + return ID == *TargetID; + } + + return false; + } + + void markAssignmentLHS(const Expr *LHS, BinaryOperatorKind Opcode) { + const Expr *Base = LHS ? LHS->IgnoreParenImpCasts() : nullptr; + if (!Base) + return; + + if (const auto *DRE = dyn_cast(Base)) { + if (isTarget(DRE->getDecl())) { + HasWrite = true; + // For pure assignment (=), LHS is write-only. Compound assignments + // such as += still read the previous value. + if (Opcode == BO_Assign) + WriteOnlyExprs.insert(DRE); + } + } else if (const auto *ME = dyn_cast(Base)) { + if (isTarget(ME->getMemberDecl())) { + HasWrite = true; + if (Opcode == BO_Assign) + WriteOnlyExprs.insert(ME); } } + } + + // Identify write-only contexts before visiting children + bool TraverseBinaryOperator(BinaryOperator *BO) { + if (BO->isAssignmentOp()) + markAssignmentLHS(BO->getLHS(), BO->getOpcode()); // Continue with default traversal return RecursiveASTVisitor::TraverseBinaryOperator(BO); } + bool TraverseCompoundAssignOperator(CompoundAssignOperator *CAO) { + markAssignmentLHS(CAO->getLHS(), CAO->getOpcode()); + return RecursiveASTVisitor::TraverseCompoundAssignOperator(CAO); + } + bool TraverseConstructorInitializer(CXXCtorInitializer *Init) { if (Init) { if (const FieldDecl *FD = Init->getMember()) { - if (FD == TargetParam) + if (isTarget(FD)) HasWrite = true; } } @@ -1835,12 +1871,12 @@ class ParamUsageVisitor : public RecursiveASTVisitor { if (UO->isIncrementDecrementOp()) { auto *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts(); if (auto *DRE = dyn_cast(SubExpr)) { - if (DRE->getDecl() == TargetParam) { + if (isTarget(DRE->getDecl())) { HasWrite = true; // Inc/Dec is both read and write, don't add to WriteOnlyExprs } } else if (auto *ME = dyn_cast(SubExpr)) { - if (ME->getMemberDecl() == TargetParam) { + if (isTarget(ME->getMemberDecl())) { HasWrite = true; } } @@ -1850,7 +1886,7 @@ class ParamUsageVisitor : public RecursiveASTVisitor { // Check for reads, excluding write-only contexts bool VisitDeclRefExpr(DeclRefExpr *DRE) { - if (DRE->getDecl() == TargetParam) { + if (isTarget(DRE->getDecl())) { if (!WriteOnlyExprs.count(DRE)) HasRead = true; } @@ -1859,7 +1895,7 @@ class ParamUsageVisitor : public RecursiveASTVisitor { // Handle member expressions bool VisitMemberExpr(MemberExpr *ME) { - if (ME->getMemberDecl() == TargetParam) { + if (isTarget(ME->getMemberDecl())) { if (!WriteOnlyExprs.count(ME)) HasRead = true; } @@ -2480,39 +2516,66 @@ prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath) { return Result; } +// Walks the entire translation unit and returns the first NamedDecl whose +// computed SymbolID matches the given ID. This allows finding declarations in +// both the main file and included header files. +static const NamedDecl *findDeclBySymbolID(const SymbolID &ID, + const ASTContext &Ctx) { + struct Finder : public RecursiveASTVisitor { + const SymbolID &TargetID; + const NamedDecl *Result = nullptr; + + Finder(const SymbolID &ID) : TargetID(ID) {} + + bool VisitNamedDecl(NamedDecl *ND) { + if (Result) + return false; // Already found – stop early. + if (auto DeclID = getSymbolID(ND)) { + if (DeclID == TargetID) { + Result = ND; + return false; + } + } + return true; + } + }; + + Finder F(ID); + F.TraverseDecl(Ctx.getTranslationUnitDecl()); + return F.Result; +} + // Tries to find a NamedDecl in the AST that matches the given Symbol. +// First attempts a fast path via source location (main file). If that fails +// (e.g. the symbol lives in an included header), falls back to a SymbolID- +// based walk of the full translation unit. // 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 +static const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, + const ParsedAST &AST) { + // Fast path: symbol is in the main file – use source-location lookup. 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; + if (SymLoc && SymLoc->uri.file() == AST.tuPath()) { + const auto &SM = AST.getSourceManager(); + auto CurLoc = sourceLocationInMainFile(SM, SymLoc->range.start); + if (CurLoc) { + auto Decls = + getDeclAtPosition(const_cast(AST), *CurLoc, {}); + if (!Decls.empty()) + return Decls[0]; + } else { + llvm::consumeError(CurLoc.takeError()); + } } - // 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]; + // Slow path: symbol is in an included header – walk the entire TU by + // SymbolID. This is needed so that analyseParameterUsage can still receive + // a valid ValueDecl* for the target symbol. + return findDeclBySymbolID(Sym.ID, AST.getASTContext()); } std::vector incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, - ParsedAST &AST) { + ParsedAST &AST, bool ComputeReferenceTags) { std::vector Results; if (!Index || Item.data.empty()) return Results; @@ -2581,17 +2644,45 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, std::optional CHI; if (auto *ND = getNamedDeclFromSymbol(Caller, AST)) { CHI = declToCallHierarchyItem(*ND, AST.tuPath()); - if (const auto *FD = llvm::dyn_cast(ND)) { - if (Decl.has_value() && Decl.value() != nullptr) { - if (const auto *VD = - llvm::dyn_cast(Decl.value())) { + if (ComputeReferenceTags) { + if (const auto *FD = llvm::dyn_cast(ND)) { + const ValueDecl *VD = nullptr; + if (Decl.has_value() && Decl.value() != nullptr) + VD = llvm::dyn_cast(Decl.value()); + + // Header symbols can come from preamble/external AST, where the + // declaration may not be discoverable through symbol lookup. + // Fall back to resolving the referenced decl at a call site. + if (!VD) { + const auto &SM = AST.getSourceManager(); + for (const Location &L : It->second) { + if (L.uri.file() != AST.tuPath()) + continue; + auto RefLoc = sourceLocationInMainFile(SM, L.range.start); + if (!RefLoc) { + llvm::consumeError(RefLoc.takeError()); + continue; + } + for (const NamedDecl *AtPos : getDeclAtPosition(AST, *RefLoc, {})) { + if (const auto *Resolved = + llvm::dyn_cast(AtPos)) { + VD = Resolved; + break; + } + } + if (VD) + break; + } + } + + if (VD) { // Use the function definition if available, not just a - // declaration + // declaration. const FunctionDecl *FuncDef = FD->getDefinition(); if (!FuncDef) FuncDef = FD; CHI->referenceTags = analyseParameterUsage( - FuncDef, VD); // FuncDef is the caller of value decl VD + FuncDef, VD); // FuncDef is the caller of value decl VD. } } } diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 31e7b0b14440f..4dfc530b4fe9c 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -149,7 +149,7 @@ prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath); std::vector incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, - ParsedAST &AST); + ParsedAST &AST, bool ComputeReferenceTags = true); 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 72fb35c5ca027..ff6be2ca875ad 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -729,6 +729,30 @@ TEST(CallHierarchy, HierarchyOnVarWithWriteReference) { AllOf(withName("caller"), withReferenceTags(ReferenceTag::Write)))))); } +TEST(CallHierarchy, HierarchyOnVarWithoutReferenceTagsSupport) { + Annotations Source(R"cpp( + int v^ar = 1; + void caller() { + var = 2; + } + )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("var"))); + + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST, + /*ComputeReferenceTags=*/false); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT(IncomingLevel1, + ElementsAre(AllOf(from(Field(&CallHierarchyItem::name, "caller")), + from(Field(&CallHierarchyItem::referenceTags, + IsEmpty()))))); +} + TEST(CallHierarchy, HierarchyOnClassMemberWithWriteReference) { Annotations Source(R"cpp( class MyClass { @@ -800,6 +824,59 @@ TEST(CallHierarchy, HierarchyOnVarWithUnaryReadWriteReference) { withReferenceTags(ReferenceTag::Write, ReferenceTag::Read)))))); } +TEST(CallHierarchy, HierarchyOnVarWithCompoundAssignmentReference) { + // Compound assignment (e.g. +=) is both a read and a write. + Annotations Source(R"cpp( + int v^ar = 1; + void caller() { + var += 2; + } + )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("var"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT( + IncomingLevel1, + UnorderedElementsAre(AllOf(from(AllOf( + withName("caller"), + withReferenceTags(ReferenceTag::Write, ReferenceTag::Read)))))); +} + +TEST(CallHierarchy, HierarchyOnHeaderVarWithWriteReference) { + // Verifies that reference tags are computed even when the target symbol lives + // in a header file (not the main translation unit). + Annotations Header(R"cpp( + int var = 1; + )cpp"); + Annotations Source(R"cpp( + #include "HeaderSymbol.h" + void caller() { + v^ar = 2; + } + )cpp"); + TestTU TU = TestTU::withCode(Source.code()); + TU.HeaderFilename = "HeaderSymbol.h"; + TU.HeaderCode = Header.code(); + auto AST = TU.build(); + auto Index = TU.index(); + + std::vector Items = + prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); + ASSERT_THAT(Items, ElementsAre(withName("var"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); + ASSERT_FALSE(IncomingLevel1.empty()); + EXPECT_THAT(IncomingLevel1, + ElementsAre(AllOf(from(AllOf( + withName("caller"), + withReferenceTags(ReferenceTag::Write)))))); +} + TEST(CallHierarchy, HierarchyOnEnumConstant) { // Tests that the call hierarchy works on enum constants. Annotations Source(R"cpp( diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp index 95bf5e54fc792..bd5e07f24dcbc 100644 --- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -65,11 +65,11 @@ class LSPTest : public ::testing::Test { Base.FeatureModules = &FeatureModules; } - LSPClient &start() { + LSPClient &start(llvm::json::Object InitializeParams = {}) { EXPECT_FALSE(Server) << "Already initialized"; Server.emplace(Client.transport(), FS, Opts); ServerThread.emplace([&] { EXPECT_TRUE(Server->run()); }); - Client.call("initialize", llvm::json::Object{}); + Client.call("initialize", std::move(InitializeParams)); return Client; } @@ -307,6 +307,71 @@ TEST_F(LSPTest, IncomingCalls) { EXPECT_EQ(From["name"], "caller1"); } +TEST_F(LSPTest, IncomingCallsWithReferenceTagsSupport) { + Annotations Code(R"cpp( + int va^r = 1; + void caller() { + var = 2; + } + )cpp"); + auto &Client = start(llvm::json::Object{{ + "capabilities", + llvm::json::Object{{ + "textDocument", + llvm::json::Object{{ + "callHierarchy", + llvm::json::Object{{"referenceTagsSupport", true}}, + }}, + }}, + }}); + Client.didOpen("foo.cpp", Code.code()); + auto Items = Client + .call("textDocument/prepareCallHierarchy", + llvm::json::Object{{ + "textDocument", Client.documentID("foo.cpp")}, + {"position", Code.point()}}) + .takeValue(); + auto FirstItem = (*Items.getAsArray())[0]; + auto Calls = Client + .call("callHierarchy/incomingCalls", + llvm::json::Object{{"item", FirstItem}}) + .takeValue(); + auto FirstCall = *(*Calls.getAsArray())[0].getAsObject(); + auto From = *FirstCall["from"].getAsObject(); + + EXPECT_EQ(From["name"], "caller"); + EXPECT_EQ(From["referenceTags"], + llvm::json::Value(llvm::json::Array{static_cast( + ReferenceTag::Write)})); +} + +TEST_F(LSPTest, IncomingCallsWithoutReferenceTagsSupport) { + Annotations Code(R"cpp( + int va^r = 1; + void caller() { + var = 2; + } + )cpp"); + auto &Client = start(); + Client.didOpen("foo.cpp", Code.code()); + auto Items = Client + .call("textDocument/prepareCallHierarchy", + llvm::json::Object{{ + "textDocument", Client.documentID("foo.cpp")}, + {"position", Code.point()}}) + .takeValue(); + auto FirstItem = (*Items.getAsArray())[0]; + auto Calls = Client + .call("callHierarchy/incomingCalls", + llvm::json::Object{{"item", FirstItem}}) + .takeValue(); + auto FirstCall = *(*Calls.getAsArray())[0].getAsObject(); + auto From = *FirstCall["from"].getAsObject(); + + EXPECT_EQ(From["name"], "caller"); + EXPECT_FALSE(From.get("referenceTags")); +} + TEST_F(LSPTest, CDBConfigIntegration) { auto CfgProvider = config::Provider::fromAncestorRelativeYAMLFiles(".clangd", FS); From aa42b7204d65a861c06249941bc85d3e0ba38137 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 13 May 2026 17:52:47 +0200 Subject: [PATCH 09/13] [clangd] Add read/write reference tags to outgoing variable hierarchy items Extend call hierarchy outgoing results to annotate variable/field references with read/write `referenceTags` when supported by the client. - keep function calls untagged (no [clangd] Add outgoing call reference tags with fast-path and stable ordering Compute read/write reference tags for outgoing call hierarchy entries on variables and fields, while keeping function calls untagged. - preserve index-only outgoing call resolution when reference tags are not needed - keep outgoing results deterministically ordered - compute variable/field usage in a single AST traversal - add and update unit/LSP tests for tagged outgoing calls read/write classification) - preserve existing incomingCalls behavior (no functional changes) - update/adjust outgoing-focused tests to cover variable/field tagging --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 3 +- clang-tools-extra/clangd/ClangdServer.cpp | 25 +- clang-tools-extra/clangd/ClangdServer.h | 3 +- clang-tools-extra/clangd/XRefs.cpp | 297 ++++++++++++-- clang-tools-extra/clangd/XRefs.h | 7 +- clang-tools-extra/clangd/index/Ref.h | 4 +- .../clangd/unittests/CallHierarchyTests.cpp | 369 +++++++++++++++++- .../clangd/unittests/ClangdLSPServerTests.cpp | 58 ++- 8 files changed, 715 insertions(+), 51 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 25603c9ed1ebe..289fda9c00550 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1436,7 +1436,8 @@ 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, + SupportsReferenceTags, std::move(Reply)); } void ClangdLSPServer::applyConfiguration( diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 267e05b71fc8d..a14891008799f 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -936,12 +936,27 @@ void ClangdServer::inlayHints(PathRef File, std::optional RestrictRange, } void ClangdServer::outgoingCalls( - const CallHierarchyItem &Item, + PathRef File, const CallHierarchyItem &Item, bool ComputeReferenceTags, Callback> CB) { - WorkScheduler->run("Outgoing Calls", "", - [CB = std::move(CB), Item, this]() mutable { - CB(clangd::outgoingCalls(Item, Index)); - }); + if (!ComputeReferenceTags) { + // Fast path: reference tags are not requested, so no AST is needed. + // Dispatch as an index-only task to avoid unnecessary parse latency. + WorkScheduler->run("Outgoing Calls", "", + [Item, CB = std::move(CB), this]() mutable { + CB(clangd::outgoingCalls(Item, Index, + /*AST=*/nullptr, + /*ComputeReferenceTags=*/false)); + }); + return; + } + 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, + /*ComputeReferenceTags=*/true)); + }; + 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 1989969331259..fe88281a967f9 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -307,7 +307,8 @@ class ClangdServer { Callback>); /// Resolve outgoing calls for a given call hierarchy item. - void outgoingCalls(const CallHierarchyItem &Item, + void outgoingCalls(PathRef File, const CallHierarchyItem &Item, + bool ComputeReferenceTags, 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 2cbcd89286dc8..b80990eaa789f 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1925,6 +1925,136 @@ static std::vector analyseParameterUsage(const FunctionDecl *FD, return Result; } +/// Visitor that determines read/write access patterns for a set of variables +/// in a single traversal of a function body. This avoids the O(N * body_size) +/// cost of calling analyseParameterUsage once per variable. +class BulkVarUsageVisitor + : public RecursiveASTVisitor { +public: + /// Per-symbol result: (hasRead, hasWrite). + llvm::DenseMap> Usage; + + BulkVarUsageVisitor(llvm::ArrayRef> + Targets) { + for (const auto &[ID, VD] : Targets) + Usage.try_emplace(ID, false, false); + } + + // Identifies write-only LHS expressions before visiting children. + bool TraverseBinaryOperator(BinaryOperator *BO) { + if (BO->isAssignmentOp()) + markLHSWrite(BO->getLHS(), BO->getOpcode()); + return RecursiveASTVisitor::TraverseBinaryOperator(BO); + } + + bool TraverseCompoundAssignOperator(CompoundAssignOperator *CAO) { + markLHSWrite(CAO->getLHS(), CAO->getOpcode()); + return RecursiveASTVisitor::TraverseCompoundAssignOperator(CAO); + } + + bool TraverseConstructorInitializer(CXXCtorInitializer *Init) { + if (Init) { + if (const FieldDecl *FD = Init->getMember()) { + if (auto ID = getSymbolID(FD)) { + if (auto Entry = Usage.find(ID); Entry != Usage.end()) + Entry->second.second = true; // hasWrite + } + } + } + return RecursiveASTVisitor::TraverseConstructorInitializer(Init); + } + + bool TraverseUnaryOperator(UnaryOperator *UO) { + if (UO->isIncrementDecrementOp()) { + const Expr *Sub = UO->getSubExpr()->IgnoreParenImpCasts(); + if (const ValueDecl *VD = getDeclFromExpr(Sub)) { + if (auto ID = getSymbolID(VD)) { + if (auto Entry = Usage.find(ID); Entry != Usage.end()) + Entry->second.second = true; // hasWrite (also hasRead via Visit*) + } + } + } + return RecursiveASTVisitor::TraverseUnaryOperator(UO); + } + + bool VisitDeclRefExpr(DeclRefExpr *DRE) { + if (auto ID = getSymbolID(DRE->getDecl())) { + if (auto Entry = Usage.find(ID); Entry != Usage.end()) { + if (!WriteOnlyExprs.count(DRE)) + Entry->second.first = true; // hasRead + } + } + return true; + } + + bool VisitMemberExpr(MemberExpr *ME) { + if (auto ID = getSymbolID(ME->getMemberDecl())) { + if (auto Entry = Usage.find(ID); Entry != Usage.end()) { + if (!WriteOnlyExprs.count(ME)) + Entry->second.first = true; // hasRead + } + } + return true; + } + +private: + llvm::SmallDenseSet WriteOnlyExprs; + + static const ValueDecl *getDeclFromExpr(const Expr *E) { + if (const auto *DRE = dyn_cast(E)) + return DRE->getDecl(); + if (const auto *ME = dyn_cast(E)) + return ME->getMemberDecl(); + return nullptr; + } + + void markLHSWrite(const Expr *LHS, BinaryOperatorKind Opcode) { + const Expr *Base = LHS ? LHS->IgnoreParenImpCasts() : nullptr; + if (!Base) + return; + const ValueDecl *VD = getDeclFromExpr(Base); + if (!VD) + return; + if (auto ID = getSymbolID(VD)) { + if (auto Entry = Usage.find(ID); Entry != Usage.end()) { + Entry->second.second = true; // hasWrite + // Pure assignment: the LHS itself is write-only (no prior read). + if (Opcode == BO_Assign) + WriteOnlyExprs.insert(Base); + } + } + } +}; + +/// Analyses read/write usage of \p Decls within \p FD in a single traversal. +/// Returns a map from SymbolID to the corresponding reference tags. +static llvm::DenseMap> +analyseAllVarUsages(const FunctionDecl *FD, + const llvm::DenseMap &Decls) { + llvm::SmallVector> Targets; + for (const auto &[ID, ND] : Decls) { + if (const auto *VD = llvm::dyn_cast(ND)) + Targets.emplace_back(ID, VD); + } + + BulkVarUsageVisitor Visitor(Targets); + if (const auto *Ctor = dyn_cast(FD)) + Visitor.TraverseDecl(const_cast(Ctor)); + else if (const Stmt *Body = FD->getBody()) + Visitor.TraverseStmt(const_cast(Body)); + + llvm::DenseMap> Result; + for (auto &[ID, RW] : Visitor.Usage) { + std::vector Tags; + if (RW.second) // hasWrite + Tags.push_back(ReferenceTag::Write); + if (RW.first) // hasRead + Tags.push_back(ReferenceTag::Read); + Result.try_emplace(ID, std::move(Tags)); + } + return Result; +} + template static std::optional declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { @@ -2727,7 +2857,8 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, } std::vector -outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + ParsedAST *AST, bool ComputeReferenceTags) { std::vector Results; if (!Index || Item.data.empty()) return Results; @@ -2736,15 +2867,48 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { elog("outgoingCalls failed to find symbol: {0}", ID.takeError()); return Results; } - // In this function, we find outgoing calls based on the index only. + + // Resolve the caller's FunctionDecl from the AST so that variable/field + // reference tags can be computed. Only attempted when the client opted in + // via ComputeReferenceTags and an AST for the caller's file is available. + const FunctionDecl *CallerFunc = nullptr; + if (ComputeReferenceTags && AST) { + LookupRequest CallerLookup; + CallerLookup.IDs.insert(*ID); + Index->lookup(CallerLookup, [&](const Symbol &Caller) { + if (Caller.ID != *ID || CallerFunc) + return; + if (const auto *ND = getNamedDeclFromSymbol(Caller, *AST)) + CallerFunc = llvm::dyn_cast(ND); + }); + + // If index-based lookup did not resolve the declaration, fall back to the + // request item's selection range in the current AST. + if (!CallerFunc && Item.uri.file() == AST->tuPath()) { + auto CurLoc = sourceLocationInMainFile(AST->getSourceManager(), + Item.selectionRange.start); + if (CurLoc) { + for (const NamedDecl *AtPos : getDeclAtPosition(*AST, *CurLoc, {})) { + if (const auto *FD = llvm::dyn_cast(AtPos)) { + CallerFunc = FD; + break; + } + } + } else { + llvm::consumeError(CurLoc.takeError()); + } + } + + if (CallerFunc) { + if (const auto *Def = CallerFunc->getDefinition()) + CallerFunc = Def; + } + } + + // Outgoing call edges for functions are collected from the index. ContainedRefsRequest Request; Request.ID = *ID; - // Initially store the ranges in a map keyed by SymbolID of the callee. - // This allows us to group different calls to the same function - // into the same CallHierarchyOutgoingCall. llvm::DenseMap> CallsOut; - // We can populate the ranges based on a refs request only. As we do so, we - // also accumulate the callee IDs into a lookup request. LookupRequest CallsOutLookup; Index->containedRefs(Request, [&](const auto &R) { auto Loc = indexToLSPLocation(R.Location, Item.uri.file()); @@ -2754,47 +2918,122 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { } auto It = CallsOut.try_emplace(R.Symbol, std::vector{}).first; It->second.push_back(*Loc); - CallsOutLookup.IDs.insert(R.Symbol); }); - // Perform the lookup request and combine its results with CallsOut to - // get complete CallHierarchyOutgoingCall objects. - Index->lookup(CallsOutLookup, [&](const Symbol &Callee) { - // The containedRefs request should only return symbols which are - // function-like, i.e. symbols for which references to them can be "calls". - using SK = index::SymbolKind; - auto Kind = Callee.SymInfo.Kind; - assert(Kind == SK::Function || Kind == SK::InstanceMethod || - Kind == SK::ClassMethod || Kind == SK::StaticMethod || - Kind == SK::Constructor || Kind == SK::Destructor || - Kind == SK::ConversionFunction); - (void)Kind; - (void)SK::Function; + Index->lookup(CallsOutLookup, [&](const Symbol &Callee) { auto It = CallsOut.find(Callee.ID); assert(It != CallsOut.end()); if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file())) { + if (CHI->kind != SymbolKind::Function && + CHI->kind != SymbolKind::Method && + CHI->kind != SymbolKind::Constructor) + return; std::vector FromRanges; for (const Location &L : It->second) { - if (L.uri != Item.uri) { - // Call location not in same file as the item that outgoingCalls was - // requested for. This can happen when Item is a declaration separate - // from the implementation. There's not much we can do, since the - // protocol only allows returning ranges interpreted as being in - // Item's file. + if (L.uri != Item.uri) continue; - } FromRanges.push_back(L.range); } Results.push_back( CallHierarchyOutgoingCall{std::move(*CHI), std::move(FromRanges)}); } }); - // Sort results by name of the callee. + + // When the client supports reference tags, also include variable/field + // references from the current function body and classify them as + // reads/writes. Function callees are still modeled as plain outgoing calls + // and do not receive reference tags. + // A single bulk traversal (BulkVarUsageVisitor / analyseAllVarUsages) is + // used to avoid O(N * body_size) cost when many variables are referenced. + if (ComputeReferenceTags && AST && CallerFunc && + Item.uri.file() == AST->tuPath()) { + llvm::DenseMap> NonCallRefs; + llvm::DenseMap NonCallDecls; + using SK = index::SymbolKind; + + findExplicitReferences( + CallerFunc, + [&](ReferenceLoc Ref) { + if (Ref.IsDecl) + return; + auto Loc = makeLocation(AST->getASTContext(), Ref.NameLoc, + Item.uri.file()); + if (!Loc) + return; + for (const Decl *D : Ref.Targets) { + if (index::isFunctionLocalSymbol(D) || D->isTemplateParameter()) + continue; + const auto *VD = llvm::dyn_cast(D); + if (!VD) + continue; + auto Kind = index::getSymbolInfo(D).Kind; + if (Kind != SK::Variable && Kind != SK::Field) + continue; + SymbolID DeclID = getSymbolID(D); + if (!DeclID) + continue; + NonCallDecls.try_emplace(DeclID, llvm::cast(D)); + NonCallRefs[DeclID].push_back(Loc->range); + } + }, + AST->getHeuristicResolver()); + + // Collect IDs already covered by index-based function call results so we + // don't emit a duplicate entry for a symbol that is both called and + // referenced as a variable. + llvm::DenseSet ProcessedIDs; + for (const auto &Call : Results) { + if (!Call.to.data.empty()) { + if (auto SID = SymbolID::fromStr(Call.to.data)) + ProcessedIDs.insert(*SID); + } + } + + // Single-pass bulk analysis of all referenced variables/fields. + auto TagMap = analyseAllVarUsages(CallerFunc, NonCallDecls); + + for (auto &Entry : NonCallRefs) { + const SymbolID &DeclID = Entry.first; + if (ProcessedIDs.count(DeclID)) + continue; + const auto *ND = NonCallDecls.lookup(DeclID); + if (!ND) + continue; + auto CHI = declToCallHierarchyItem(*ND, Item.uri.file()); + if (!CHI) + continue; + if (auto TagIt = TagMap.find(DeclID); TagIt != TagMap.end()) + CHI->referenceTags = TagIt->second; + CHI->data = DeclID.str(); + Results.push_back( + CallHierarchyOutgoingCall{std::move(*CHI), std::move(Entry.second)}); + } + } + + // Sort results by first source position so the order is deterministic and + // matches reading order. Entries without any fromRanges (e.g. cross-file + // callees) sort after all positioned entries, then alphabetically. llvm::sort(Results, [](const CallHierarchyOutgoingCall &A, const CallHierarchyOutgoingCall &B) { + auto FirstPos = [](const std::vector &Ranges) -> Position { + Position Best{INT_MAX, INT_MAX}; + for (const auto &R : Ranges) + if (R.start.line < Best.line || + (R.start.line == Best.line && + R.start.character < Best.character)) + Best = R.start; + return Best; + }; + Position APos = FirstPos(A.fromRanges); + Position BPos = FirstPos(B.fromRanges); + if (APos.line != BPos.line) + return APos.line < BPos.line; + if (APos.character != BPos.character) + return APos.character < BPos.character; return A.to.name < B.to.name; }); + return Results; } diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 4dfc530b4fe9c..6c5cb59e25b67 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -151,10 +151,13 @@ std::vector incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, ParsedAST &AST, bool ComputeReferenceTags = true); +/// \p AST may be null when \p ComputeReferenceTags is false, in which case +/// only index-based function call edges are returned. std::vector -outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); +outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, + ParsedAST *AST, bool ComputeReferenceTags = false); -/// Returns all decls that are referenced in the \p FD except local symbols. +/// Returns all decls that are referenced in \p FD except local symbols. llvm::DenseSet getNonLocalDeclRefs(ParsedAST &AST, const FunctionDecl *FD); } // namespace clangd diff --git a/clang-tools-extra/clangd/index/Ref.h b/clang-tools-extra/clangd/index/Ref.h index 1241cb8361384..8f1a8bf257b7c 100644 --- a/clang-tools-extra/clangd/index/Ref.h +++ b/clang-tools-extra/clangd/index/Ref.h @@ -69,12 +69,12 @@ enum class RefKind : uint8_t { All = Declaration | Definition | Reference | Spelled | Call, }; -inline RefKind operator|(RefKind L, RefKind R) { +inline constexpr RefKind operator|(RefKind L, RefKind R) { return static_cast(static_cast(L) | static_cast(R)); } inline RefKind &operator|=(RefKind &L, RefKind R) { return L = L | R; } -inline RefKind operator&(RefKind A, RefKind B) { +inline constexpr RefKind operator&(RefKind A, RefKind B) { return static_cast(static_cast(A) & static_cast(B)); } diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index ff6be2ca875ad..be1ebfeb4746a 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -302,7 +302,7 @@ 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( @@ -311,23 +311,73 @@ TEST(CallHierarchy, OutgoingOneFile) { 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"))), 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()); } +TEST(CallHierarchy, OutgoingWithReferenceTagsSupport) { + Annotations Source(R"cpp( + void callee(); + void ca^ller() { + $Callee[[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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre( + AllOf(to(AllOf(withName("callee"), + Field(&CallHierarchyItem::referenceTags, IsEmpty()))), + oFromRanges(Source.range("Callee"))))); +} + +TEST(CallHierarchy, OutgoingWithoutReferenceTagsSupport) { + Annotations Source(R"cpp( + void callee(); + void ca^ller() { + $Callee[[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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/false); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre( + AllOf(to(AllOf(withName("callee"), + Field(&CallHierarchyItem::referenceTags, IsEmpty()))), + oFromRanges(Source.range("Callee"))))); +} + TEST(CallHierarchy, MultiFileCpp) { // The test uses a .hh suffix for header files to get clang // to parse them in C++ mode. .h files are parsed in C mode @@ -439,7 +489,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. @@ -453,19 +503,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()); }; @@ -875,6 +925,309 @@ TEST(CallHierarchy, HierarchyOnHeaderVarWithWriteReference) { ElementsAre(AllOf(from(AllOf( withName("caller"), withReferenceTags(ReferenceTag::Write)))))); + +TEST(CallHierarchy, OutgoingFieldWithReadReference) { + Annotations Source(R"cpp( + struct Vars { + int var1 = 1; + }; + void ca^ller() { + Vars values; + int foo = values.var1; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT(OutgoingLevel1, + ElementsAre(AllOf(to(AllOf( + withName("var1"), withReferenceTags(ReferenceTag::Read)))))); +} + +TEST(CallHierarchy, OutgoingVarWithReadReference) { + Annotations Source(R"cpp( + int var = 1; + void ca^ller() { + int x = 0; + x = var; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT(OutgoingLevel1, + ElementsAre(AllOf(to(AllOf( + withName("var"), withReferenceTags(ReferenceTag::Read)))))); +} + +TEST(CallHierarchy, OutgoingVarWithWriteReference) { + Annotations Source(R"cpp( + int var = 1; + void ca^ller() { + var = 2; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT(OutgoingLevel1, + ElementsAre(AllOf(to(AllOf( + withName("var"), withReferenceTags(ReferenceTag::Write)))))); +} + +TEST(CallHierarchy, OutgoingVarWithoutReferenceTagsSupport) { + Annotations Source(R"cpp( + int var = 1; + void ca^ller() { + var = 2; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/false); + EXPECT_THAT(OutgoingLevel1, IsEmpty()); +} + +TEST(CallHierarchy, OutgoingClassMemberWithWriteReference) { + Annotations Source(R"cpp( + class MyClass { + public: + void ca^ller() { + $Write[[var]] = 2; + } + int var = 1; + }; + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre(AllOf( + to(AllOf(withName("var"), withReferenceTags(ReferenceTag::Write))), + oFromRanges(Source.range("Write"))))); +} + +TEST(CallHierarchy, OutgoingClassMemberWithWriteReferenceInCtorInitList) { + Annotations Source(R"cpp( + class MyClass { + int var; + public: + MyCl^ass() : $Write[[var]](1) {} + }; + )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("MyClass"))); + + auto OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre(AllOf( + to(AllOf(withName("var"), withReferenceTags(ReferenceTag::Write))), + oFromRanges(Source.range("Write"))))); +} + +TEST(CallHierarchy, OutgoingVarWithUnaryReadWriteReference) { + Annotations Source(R"cpp( + int var = 1; + void ca^ller() { + $ReadWrite[[var]]++; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre(AllOf( + to(AllOf(withName("var"), + withReferenceTags(ReferenceTag::Write, ReferenceTag::Read))), + oFromRanges(Source.range("ReadWrite"))))); +} + +TEST(CallHierarchy, OutgoingVarWithCompoundAssignmentReference) { + // Compound assignment (e.g. +=) is both a read and a write. + Annotations Source(R"cpp( + int var = 1; + void ca^ller() { + $ReadWrite[[var]] += 2; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre(AllOf( + to(AllOf(withName("var"), + withReferenceTags(ReferenceTag::Write, ReferenceTag::Read))), + oFromRanges(Source.range("ReadWrite"))))); +} + +TEST(CallHierarchy, OutgoingHeaderVarWithWriteReference) { + // Verifies that reference tags are computed even when the referenced symbol + // lives in a header file. + Annotations Header(R"cpp( + int var = 1; + )cpp"); + Annotations Source(R"cpp( + #include "HeaderSymbol.h" + void ca^ller() { + $Write[[var]] = 2; + } + )cpp"); + TestTU TU = TestTU::withCode(Source.code()); + TU.HeaderFilename = "HeaderSymbol.h"; + TU.HeaderCode = Header.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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre(AllOf( + to(AllOf(withName("var"), withReferenceTags(ReferenceTag::Write))), + oFromRanges(Source.range("Write"))))); +} + +TEST(CallHierarchy, OutgoingMixedVarAndFunctionKeepsFunctionUntagged) { + Annotations Source(R"cpp( int var = 0; + void callee(); + void ca^ller() { + $Write[[var]] = 1; + $Call[[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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + UnorderedElementsAre( + AllOf(to(AllOf(withName("var"), + withReferenceTags(ReferenceTag::Write))), + oFromRanges(Source.range("Write"))), + AllOf(to(AllOf(withName("callee"), + Field(&CallHierarchyItem::referenceTags, IsEmpty()))), + oFromRanges(Source.range("Call"))))); +} + +TEST(CallHierarchy, OutgoingVarAggregatesReadAndWriteAcrossStatements) { + Annotations Source(R"cpp( int var = 0; + void ca^ller() { + int x = $Read[[var]]; + $Write[[var]] = x; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre(AllOf( + to(AllOf(withName("var"), + withReferenceTags(ReferenceTag::Read, ReferenceTag::Write))), + oFromRanges(Source.range("Read"), Source.range("Write"))))); +} + +TEST(CallHierarchy, OutgoingVarRepeatedReadsProduceSingleItem) { + Annotations Source(R"cpp( int var = 0; + void ca^ller() { + int a = $Read1[[var]]; + int b = $Read2[[var]]; + (void)a; + (void)b; + } + )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 OutgoingLevel1 = outgoingCalls(Items[0], Index.get(), &AST, + /*ComputeReferenceTags=*/true); + ASSERT_THAT( + OutgoingLevel1, + ElementsAre(AllOf( + to(AllOf(withName("var"), withReferenceTags(ReferenceTag::Read))), + oFromRanges(Source.range("Read1"), Source.range("Read2"))))); } TEST(CallHierarchy, HierarchyOnEnumConstant) { diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp index bd5e07f24dcbc..4b4af97939a4c 100644 --- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -327,9 +327,9 @@ TEST_F(LSPTest, IncomingCallsWithReferenceTagsSupport) { Client.didOpen("foo.cpp", Code.code()); auto Items = Client .call("textDocument/prepareCallHierarchy", - llvm::json::Object{{ - "textDocument", Client.documentID("foo.cpp")}, - {"position", Code.point()}}) + llvm::json::Object{ + {"textDocument", Client.documentID("foo.cpp")}, + {"position", Code.point()}}) .takeValue(); auto FirstItem = (*Items.getAsArray())[0]; auto Calls = Client @@ -372,6 +372,58 @@ TEST_F(LSPTest, IncomingCallsWithoutReferenceTagsSupport) { EXPECT_FALSE(From.get("referenceTags")); } +TEST_F(LSPTest, OutgoingVarCallsWithReferenceTagsSupport) { + Annotations Code(R"cpp( + int var = 1; + void callee(); + void ca^ller() { + var = 2; + callee(); + } + )cpp"); + auto &Client = start(llvm::json::Object{{ + "capabilities", + llvm::json::Object{{ + "textDocument", + llvm::json::Object{{ + "callHierarchy", + llvm::json::Object{{"referenceTagsSupport", true}}, + }}, + }}, + }}); + Client.didOpen("foo.cpp", Code.code()); + auto Items = Client + .call("textDocument/prepareCallHierarchy", + llvm::json::Object{ + {"textDocument", Client.documentID("foo.cpp")}, + {"position", Code.point()}}) + .takeValue(); + auto FirstItem = (*Items.getAsArray())[0]; + auto Calls = Client + .call("callHierarchy/outgoingCalls", + llvm::json::Object{{"item", FirstItem}}) + .takeValue(); + auto CallArray = Calls.getAsArray(); + ASSERT_EQ(CallArray->size(), 2u); + + { + auto FirstCall = *(*CallArray)[0].getAsObject(); + auto To = *FirstCall["to"].getAsObject(); + + EXPECT_EQ(To["name"], "var"); + EXPECT_EQ(To["referenceTags"], llvm::json::Value(llvm::json::Array{ + static_cast(ReferenceTag::Write)})); + } + { + auto SecondCall = *(*CallArray)[1].getAsObject(); + auto To = *SecondCall["to"].getAsObject(); + + EXPECT_EQ(To["name"], "callee"); + // Function calls are neither Read nor Write accesses, so no referenceTags. + EXPECT_FALSE(To.get("referenceTags")); + } +} + TEST_F(LSPTest, CDBConfigIntegration) { auto CfgProvider = config::Provider::fromAncestorRelativeYAMLFiles(".clangd", FS); From 914239f311af3a32803fb1f708092b25812779bf Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 13 May 2026 18:11:55 +0200 Subject: [PATCH 10/13] [clangd] fix format. --- clang-tools-extra/clangd/ClangdServer.cpp | 15 +++++----- clang-tools-extra/clangd/Protocol.h | 1 - clang-tools-extra/clangd/XRefs.cpp | 20 ++++++------- .../clangd/unittests/CallHierarchyTests.cpp | 30 +++++++++++-------- .../clangd/unittests/ClangdLSPServerTests.cpp | 11 ++++--- 5 files changed, 38 insertions(+), 39 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index a14891008799f..dd164a0b501bb 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -918,8 +918,7 @@ void ClangdServer::incomingCalls( this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - CB(clangd::incomingCalls(Item, Index, InpAST->AST, - ComputeReferenceTags)); + CB(clangd::incomingCalls(Item, Index, InpAST->AST, ComputeReferenceTags)); }; WorkScheduler->runWithAST("Incoming Calls", File, std::move(Action)); } @@ -941,12 +940,12 @@ void ClangdServer::outgoingCalls( if (!ComputeReferenceTags) { // Fast path: reference tags are not requested, so no AST is needed. // Dispatch as an index-only task to avoid unnecessary parse latency. - WorkScheduler->run("Outgoing Calls", "", - [Item, CB = std::move(CB), this]() mutable { - CB(clangd::outgoingCalls(Item, Index, - /*AST=*/nullptr, - /*ComputeReferenceTags=*/false)); - }); + WorkScheduler->run( + "Outgoing Calls", "", [Item, CB = std::move(CB), this]() mutable { + CB(clangd::outgoingCalls(Item, Index, + /*AST=*/nullptr, + /*ComputeReferenceTags=*/false)); + }); return; } auto Action = [Item, CB = std::move(CB), diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 56738e3e93d57..27ac6659f7e3e 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1570,7 +1570,6 @@ struct TypeHierarchyItem { /// The kind of this item. SymbolKind kind; - /// 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 b80990eaa789f..56fbc031440bf 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1928,14 +1928,13 @@ static std::vector analyseParameterUsage(const FunctionDecl *FD, /// Visitor that determines read/write access patterns for a set of variables /// in a single traversal of a function body. This avoids the O(N * body_size) /// cost of calling analyseParameterUsage once per variable. -class BulkVarUsageVisitor - : public RecursiveASTVisitor { +class BulkVarUsageVisitor : public RecursiveASTVisitor { public: /// Per-symbol result: (hasRead, hasWrite). llvm::DenseMap> Usage; - BulkVarUsageVisitor(llvm::ArrayRef> - Targets) { + BulkVarUsageVisitor( + llvm::ArrayRef> Targets) { for (const auto &[ID, VD] : Targets) Usage.try_emplace(ID, false, false); } @@ -2688,8 +2687,7 @@ static const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym, const auto &SM = AST.getSourceManager(); auto CurLoc = sourceLocationInMainFile(SM, SymLoc->range.start); if (CurLoc) { - auto Decls = - getDeclAtPosition(const_cast(AST), *CurLoc, {}); + auto Decls = getDeclAtPosition(const_cast(AST), *CurLoc, {}); if (!Decls.empty()) return Decls[0]; } else { @@ -2793,7 +2791,8 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, llvm::consumeError(RefLoc.takeError()); continue; } - for (const NamedDecl *AtPos : getDeclAtPosition(AST, *RefLoc, {})) { + for (const NamedDecl *AtPos : + getDeclAtPosition(AST, *RefLoc, {})) { if (const auto *Resolved = llvm::dyn_cast(AtPos)) { VD = Resolved; @@ -2957,8 +2956,8 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, [&](ReferenceLoc Ref) { if (Ref.IsDecl) return; - auto Loc = makeLocation(AST->getASTContext(), Ref.NameLoc, - Item.uri.file()); + auto Loc = + makeLocation(AST->getASTContext(), Ref.NameLoc, Item.uri.file()); if (!Loc) return; for (const Decl *D : Ref.Targets) { @@ -3020,8 +3019,7 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index, Position Best{INT_MAX, INT_MAX}; for (const auto &R : Ranges) if (R.start.line < Best.line || - (R.start.line == Best.line && - R.start.character < Best.character)) + (R.start.line == Best.line && R.start.character < Best.character)) Best = R.start; return Best; }; diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index be1ebfeb4746a..ead7907c852f3 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -503,19 +503,22 @@ 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(), &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(), &AST); + 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(), &AST); + auto OutgoingLevel4 = + outgoingCalls(OutgoingLevel3[0].to, Index.get(), &AST); EXPECT_THAT(OutgoingLevel4, IsEmpty()); }; @@ -798,9 +801,9 @@ TEST(CallHierarchy, HierarchyOnVarWithoutReferenceTagsSupport) { /*ComputeReferenceTags=*/false); ASSERT_FALSE(IncomingLevel1.empty()); EXPECT_THAT(IncomingLevel1, - ElementsAre(AllOf(from(Field(&CallHierarchyItem::name, "caller")), - from(Field(&CallHierarchyItem::referenceTags, - IsEmpty()))))); + ElementsAre(AllOf( + from(Field(&CallHierarchyItem::name, "caller")), + from(Field(&CallHierarchyItem::referenceTags, IsEmpty()))))); } TEST(CallHierarchy, HierarchyOnClassMemberWithWriteReference) { @@ -893,9 +896,9 @@ TEST(CallHierarchy, HierarchyOnVarWithCompoundAssignmentReference) { ASSERT_FALSE(IncomingLevel1.empty()); EXPECT_THAT( IncomingLevel1, - UnorderedElementsAre(AllOf(from(AllOf( - withName("caller"), - withReferenceTags(ReferenceTag::Write, ReferenceTag::Read)))))); + UnorderedElementsAre(AllOf(from( + AllOf(withName("caller"), + withReferenceTags(ReferenceTag::Write, ReferenceTag::Read)))))); } TEST(CallHierarchy, HierarchyOnHeaderVarWithWriteReference) { @@ -921,10 +924,11 @@ TEST(CallHierarchy, HierarchyOnHeaderVarWithWriteReference) { ASSERT_THAT(Items, ElementsAre(withName("var"))); auto IncomingLevel1 = incomingCalls(Items[0], Index.get(), AST); ASSERT_FALSE(IncomingLevel1.empty()); - EXPECT_THAT(IncomingLevel1, - ElementsAre(AllOf(from(AllOf( - withName("caller"), - withReferenceTags(ReferenceTag::Write)))))); + EXPECT_THAT( + IncomingLevel1, + ElementsAre(AllOf(from( + AllOf(withName("caller"), withReferenceTags(ReferenceTag::Write)))))); +} TEST(CallHierarchy, OutgoingFieldWithReadReference) { Annotations Source(R"cpp( diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp index 4b4af97939a4c..dbf81923a26c3 100644 --- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -340,9 +340,8 @@ TEST_F(LSPTest, IncomingCallsWithReferenceTagsSupport) { auto From = *FirstCall["from"].getAsObject(); EXPECT_EQ(From["name"], "caller"); - EXPECT_EQ(From["referenceTags"], - llvm::json::Value(llvm::json::Array{static_cast( - ReferenceTag::Write)})); + EXPECT_EQ(From["referenceTags"], llvm::json::Value(llvm::json::Array{ + static_cast(ReferenceTag::Write)})); } TEST_F(LSPTest, IncomingCallsWithoutReferenceTagsSupport) { @@ -356,9 +355,9 @@ TEST_F(LSPTest, IncomingCallsWithoutReferenceTagsSupport) { Client.didOpen("foo.cpp", Code.code()); auto Items = Client .call("textDocument/prepareCallHierarchy", - llvm::json::Object{{ - "textDocument", Client.documentID("foo.cpp")}, - {"position", Code.point()}}) + llvm::json::Object{ + {"textDocument", Client.documentID("foo.cpp")}, + {"position", Code.point()}}) .takeValue(); auto FirstItem = (*Items.getAsArray())[0]; auto Calls = Client From d041feb1c9ce399eee6e23628e9a018a7999572b Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Sun, 24 May 2026 00:20:16 +0200 Subject: [PATCH 11/13] [clangd] Add LSP 3.18 textDocument/references referenceItems support Implement support for the LSP 3.18 PR #2226 textDocument/references Reference item response type. When the client announces textDocument.references.referenceItemsSupport=true, clangd returns Reference[] objects (with nested 'location' and optional 'referenceTags') instead of the legacy flat Location[] array. Changes: - Protocol.h/cpp: Add 'Reference' struct and JSON serializer; parse 'textDocument.references.referenceItemsSupport' client capability. - XRefs.h/cpp: Add 'ReferenceTags' field to ReferencesResult::Reference; introduce 6-arg findReferences() overload with ComputeReferenceTags flag; keep 5-arg overload for backward compatibility. - ClangdServer.h/cpp: Thread ComputeReferenceTags flag through to XRefs. - ClangdLSPServer.h/cpp: Store SupportsReferenceItems capability flag; in onReference() emit Reference[] or legacy ReferenceLocation[] depending on client capability. - Tests: Add LSPTest.ReferencesReturnReferenceItemsWithTags and LSPTest.ReferencesReturnLegacyLocationWithoutReferenceItemsSupport. --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 57 ++++++++----- clang-tools-extra/clangd/ClangdLSPServer.h | 5 +- clang-tools-extra/clangd/ClangdServer.cpp | 8 +- clang-tools-extra/clangd/ClangdServer.h | 3 +- clang-tools-extra/clangd/Protocol.cpp | 7 ++ clang-tools-extra/clangd/Protocol.h | 12 +++ clang-tools-extra/clangd/XRefs.cpp | 13 +++ clang-tools-extra/clangd/XRefs.h | 7 ++ .../clangd/unittests/ClangdLSPServerTests.cpp | 79 +++++++++++++++++++ 9 files changed, 165 insertions(+), 26 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 289fda9c00550..30448154862f3 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -538,6 +538,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, Params.capabilities.HierarchicalDocumentSymbol; SupportsReferenceContainer = Params.capabilities.ReferenceContainer; SupportsReferenceTags = Params.capabilities.ReferenceTagsSupport; + SupportsReferenceItems = Params.capabilities.ReferenceItemsSupport; SupportFileStatus = Params.initializationOptions.FileStatus; SupportsDocumentChanges = Params.capabilities.DocumentChanges; SupportsChangeAnnotation = Params.capabilities.ChangeAnnotation; @@ -1482,27 +1483,41 @@ void ClangdLSPServer::onChangeConfiguration( applyConfiguration(Params.settings); } -void ClangdLSPServer::onReference( - const ReferenceParams &Params, - Callback> Reply) { - Server->findReferences(Params.textDocument.uri.file(), Params.position, - Opts.ReferencesLimit, SupportsReferenceContainer, - [Reply = std::move(Reply), - IncludeDecl(Params.context.includeDeclaration)]( - llvm::Expected Refs) mutable { - if (!Refs) - return Reply(Refs.takeError()); - // Filter out declarations if the client asked. - std::vector Result; - Result.reserve(Refs->References.size()); - for (auto &Ref : Refs->References) { - bool IsDecl = - Ref.Attributes & ReferencesResult::Declaration; - if (IncludeDecl || !IsDecl) - Result.push_back(std::move(Ref.Loc)); - } - return Reply(std::move(Result)); - }); +void ClangdLSPServer::onReference(const ReferenceParams &Params, + Callback Reply) { + Server->findReferences( + Params.textDocument.uri.file(), Params.position, Opts.ReferencesLimit, + SupportsReferenceContainer, + /*ComputeReferenceTags=*/SupportsReferenceItems, + [Reply = std::move(Reply), IncludeDecl(Params.context.includeDeclaration), + ReturnReferenceItems(SupportsReferenceItems)]( + llvm::Expected Refs) mutable { + if (!Refs) + return Reply(Refs.takeError()); + // Filter out declarations if the client asked. + if (ReturnReferenceItems) { + std::vector Result; + Result.reserve(Refs->References.size()); + for (auto &Ref : Refs->References) { + bool IsDecl = Ref.Attributes & ReferencesResult::Declaration; + if (!(IncludeDecl || !IsDecl)) + continue; + Reference Item; + Item.location = std::move(Ref.Loc); + Item.referenceTags = std::move(Ref.ReferenceTags); + Result.push_back(std::move(Item)); + } + return Reply(std::move(Result)); + } + std::vector Result; + Result.reserve(Refs->References.size()); + for (auto &Ref : Refs->References) { + bool IsDecl = Ref.Attributes & ReferencesResult::Declaration; + if (IncludeDecl || !IsDecl) + Result.push_back(std::move(Ref.Loc)); + } + return Reply(std::move(Result)); + }); } void ClangdLSPServer::onGoToType(const TextDocumentPositionParams &Params, diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 860632476cbe4..ffd62968a2a7f 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -133,7 +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 &, @@ -301,6 +301,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks, bool SupportsReferenceContainer = false; /// Whether the client supports reference tags on hierarchy/reference items. bool SupportsReferenceTags = false; + /// Whether the client supports and prefers LSP 3.18 Reference[] results + /// for textDocument/references. + bool SupportsReferenceItems = false; /// Which kind of markup should we use in textDocument/hover responses. MarkupKind HoverContentFormat = MarkupKind::PlainText; /// Whether the client supports offsets for parameter info labels. diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index dd164a0b501bb..83fd94c2ce9c1 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -1026,13 +1026,15 @@ void ClangdServer::findImplementations( } void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit, - bool AddContainer, + bool AddContainer, bool ComputeReferenceTags, Callback CB) { - auto Action = [Pos, Limit, AddContainer, CB = std::move(CB), + auto Action = [Pos, Limit, AddContainer, ComputeReferenceTags, + CB = std::move(CB), this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - CB(clangd::findReferences(InpAST->AST, Pos, Limit, Index, AddContainer)); + CB(clangd::findReferences(InpAST->AST, Pos, Limit, Index, AddContainer, + ComputeReferenceTags)); }; WorkScheduler->runWithAST("References", File, std::move(Action)); diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index fe88281a967f9..20f3f8727840e 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -336,7 +336,8 @@ class ClangdServer { /// Retrieve locations for symbol references. void findReferences(PathRef File, Position Pos, uint32_t Limit, - bool AddContainer, Callback CB); + bool AddContainer, bool ComputeReferenceTags, + Callback CB); /// Run formatting for the \p File with content \p Code. /// If \p Rng is non-empty, formats only those regions. diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 60dddfcea62d8..3be351a83e89e 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -172,6 +172,13 @@ llvm::json::Value toJSON(const ReferenceLocation &P) { return Result; } +llvm::json::Value toJSON(const Reference &R) { + llvm::json::Object Result{{"location", R.location}}; + if (!R.referenceTags.empty()) + Result["referenceTags"] = R.referenceTags; + return Result; +} + llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const ReferenceLocation &L) { return OS << L.range << '@' << L.uri << " (container: " << L.containerName diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 27ac6659f7e3e..bb6a78b8c888d 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -44,6 +44,8 @@ namespace clang { namespace clangd { +enum class ReferenceTag; + enum class ErrorCode { // Defined by JSON RPC. ParseError = -32700, @@ -238,6 +240,16 @@ struct ReferenceLocation : Location { llvm::json::Value toJSON(const ReferenceLocation &); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const ReferenceLocation &); +/// LSP 3.18 reference item used by textDocument/references when supported. +struct Reference { + /// The location of this reference. + Location location; + + /// Optional, one or more tags describing this reference. + std::vector referenceTags; +}; +llvm::json::Value toJSON(const Reference &); + using ChangeAnnotationIdentifier = std::string; // A combination of a LSP standard TextEdit and AnnotatedTextEdit. struct TextEdit { diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 56fbc031440bf..6cb64b6b9cffe 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1486,6 +1486,13 @@ maybeFindIncludeReferences(ParsedAST &AST, Position Pos, ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit, const SymbolIndex *Index, bool AddContext) { + return findReferences(AST, Pos, Limit, Index, AddContext, + /*ComputeReferenceTags=*/false); +} + +ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit, + const SymbolIndex *Index, bool AddContext, + bool ComputeReferenceTags) { ReferencesResult Results; const SourceManager &SM = AST.getSourceManager(); auto MainFilePath = AST.tuPath(); @@ -1587,6 +1594,12 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit, ReferencesResult::Reference Result; Result.Loc.range = Ref.range(SM); Result.Loc.uri = URIMainFile; + if (ComputeReferenceTags) { + if (Ref.Role & static_cast(index::SymbolRole::Write)) + Result.ReferenceTags.push_back(ReferenceTag::Write); + if (Ref.Role & static_cast(index::SymbolRole::Read)) + Result.ReferenceTags.push_back(ReferenceTag::Read); + } if (AddContext) Result.Loc.containerName = stringifyContainerForMainFileRef(Ref.Container); diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 6c5cb59e25b67..d409e52c62f7d 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -89,6 +89,7 @@ struct ReferencesResult { }; struct Reference { ReferenceLocation Loc; + std::vector ReferenceTags; unsigned Attributes = 0; }; std::vector References; @@ -116,6 +117,12 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit, const SymbolIndex *Index = nullptr, bool AddContext = false); +/// Returns references and optionally computes read/write tags for main-file +/// references when \p ComputeReferenceTags is true. +ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit, + const SymbolIndex *Index, bool AddContext, + bool ComputeReferenceTags); + /// Get info about symbols at \p Pos. std::vector getSymbolInfo(ParsedAST &AST, Position Pos); diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp index dbf81923a26c3..5a83854880c6b 100644 --- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -423,6 +423,85 @@ TEST_F(LSPTest, OutgoingVarCallsWithReferenceTagsSupport) { } } +TEST_F(LSPTest, ReferencesReturnReferenceItemsWithTags) { + Annotations Code(R"cpp( + int var = 1; + void caller() { + $Write[[v^ar]] = 2; + int x = $Read[[var]]; + } + )cpp"); + auto &Client = start(llvm::json::Object{{ + "capabilities", + llvm::json::Object{{ + "textDocument", + llvm::json::Object{{ + "references", + llvm::json::Object{{"referenceItemsSupport", true}}, + }}, + }}, + }}); + Client.didOpen("foo.cpp", Code.code()); + auto Refs = Client + .call("textDocument/references", + llvm::json::Object{ + {"textDocument", Client.documentID("foo.cpp")}, + {"position", Code.point()}, + {"context", + llvm::json::Object{{"includeDeclaration", false}}}, + }) + .takeValue(); + + auto *RefArray = Refs.getAsArray(); + ASSERT_TRUE(RefArray); + ASSERT_EQ(RefArray->size(), 2u); + + auto First = *(*RefArray)[0].getAsObject(); + auto FirstLoc = *First["location"].getAsObject(); + EXPECT_EQ(FirstLoc["uri"], Client.uri("foo.cpp")); + EXPECT_EQ(FirstLoc["range"], llvm::json::Value(Code.range("Write"))); + EXPECT_EQ(First["referenceTags"], + llvm::json::Value( + llvm::json::Array{static_cast(ReferenceTag::Write)})); + + auto Second = *(*RefArray)[1].getAsObject(); + auto SecondLoc = *Second["location"].getAsObject(); + EXPECT_EQ(SecondLoc["uri"], Client.uri("foo.cpp")); + EXPECT_EQ(SecondLoc["range"], llvm::json::Value(Code.range("Read"))); + EXPECT_EQ(Second["referenceTags"], + llvm::json::Value( + llvm::json::Array{static_cast(ReferenceTag::Read)})); +} + +TEST_F(LSPTest, ReferencesReturnLegacyLocationWithoutReferenceItemsSupport) { + Annotations Code(R"cpp( + int var = 1; + void caller() { + $Write[[v^ar]] = 2; + } + )cpp"); + auto &Client = start(); + Client.didOpen("foo.cpp", Code.code()); + auto Refs = Client + .call("textDocument/references", + llvm::json::Object{ + {"textDocument", Client.documentID("foo.cpp")}, + {"position", Code.point()}, + {"context", + llvm::json::Object{{"includeDeclaration", false}}}, + }) + .takeValue(); + + auto *RefArray = Refs.getAsArray(); + ASSERT_TRUE(RefArray); + ASSERT_EQ(RefArray->size(), 1u); + auto First = *(*RefArray)[0].getAsObject(); + EXPECT_EQ(First["uri"], Client.uri("foo.cpp")); + EXPECT_EQ(First["range"], llvm::json::Value(Code.range("Write"))); + EXPECT_FALSE(First.get("location")); + EXPECT_FALSE(First.get("referenceTags")); +} + TEST_F(LSPTest, CDBConfigIntegration) { auto CfgProvider = config::Provider::fromAncestorRelativeYAMLFiles(".clangd", FS); From 2b5c632ca26eeb02ca99c0b97e5c7f5663fb8640 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 26 May 2026 09:12:49 +0200 Subject: [PATCH 12/13] [clangd] Test references with referenceItemsSupport and referenceTags Update references.test lit to verify the new LSP 3.18 reference items response format. Enable referenceItemsSupport capability in the initialize request, and verify that the response includes: - nested 'location' field instead of flat uri/range - 'referenceTags' array marking the reference as Read (tag value 1) This validates that clangd correctly returns Reference items with semantic information when the client supports it. --- clang-tools-extra/clangd/test/references.test | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clangd/test/references.test b/clang-tools-extra/clangd/test/references.test index 70e52f4c61535..a111d6eec527e 100644 --- a/clang-tools-extra/clangd/test/references.test +++ b/clang-tools-extra/clangd/test/references.test @@ -1,5 +1,5 @@ # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s -{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"references":{"referenceItemsSupport":true}}},"trace":"off"}} --- {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{ "uri":"test:///main.cpp", @@ -17,17 +17,22 @@ # CHECK-NEXT: "jsonrpc": "2.0", # CHECK-NEXT: "result": [ # CHECK-NEXT: { -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 16, -# CHECK-NEXT: "line": 0 +# CHECK-NEXT: "location": { +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 16, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 15, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } # CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 15, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } +# CHECK-NEXT: "uri": "{{.*}}/main.cpp" # CHECK-NEXT: }, -# CHECK-NEXT: "uri": "{{.*}}/main.cpp" +# CHECK-NEXT: "referenceTags": [ +# CHECK-NEXT: 1 +# CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ] --- From f3045f8f5c235500e88f65719b83436fa891c584 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 26 May 2026 11:30:44 +0200 Subject: [PATCH 13/13] [clangd] Extend references.test with Write tag and function reference cases Add two new textDocument/references lit tests to cover: - Write reference (referenceTags: [2]): variable assigned via 'w = 5' confirms tag value 2 (ReferenceTag::Write) is emitted. - Function call reference (no referenceTags): calling 'func()' produces a Reference item with only 'location'; function calls carry neither Read nor Write access semantics, so 'referenceTags' is absent. --- clang-tools-extra/clangd/test/references.test | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/test/references.test b/clang-tools-extra/clangd/test/references.test index a111d6eec527e..15f794d208008 100644 --- a/clang-tools-extra/clangd/test/references.test +++ b/clang-tools-extra/clangd/test/references.test @@ -36,6 +36,73 @@ # CHECK-NEXT: } # CHECK-NEXT: ] --- -{"jsonrpc":"2.0","id":3,"method":"shutdown"} +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{ + "uri":"test:///write.cpp", + "languageId":"cpp", + "version":1, + "text":"int w; void h() { w = 5; }" +}}} +--- +{"jsonrpc":"2.0","id":2,"method":"textDocument/references","params":{ + "textDocument":{"uri":"test:///write.cpp"}, + "position":{"line":0,"character":4}, + "context":{"includeDeclaration": false} +}} +# CHECK: "id": 2 +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "location": { +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 19, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 18, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "uri": "{{.*}}/write.cpp" +# CHECK-NEXT: }, +# CHECK-NEXT: "referenceTags": [ +# CHECK-NEXT: 2 +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: ] +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{ + "uri":"test:///func.cpp", + "languageId":"cpp", + "version":1, + "text":"void func(); void caller() { func(); }" +}}} +--- +{"jsonrpc":"2.0","id":4,"method":"textDocument/references","params":{ + "textDocument":{"uri":"test:///func.cpp"}, + "position":{"line":0,"character":5}, + "context":{"includeDeclaration": false} +}} +# CHECK: "id": 4 +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "location": { +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 33, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 29, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "uri": "{{.*}}/func.cpp" +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +--- +{"jsonrpc":"2.0","id":5,"method":"shutdown"} --- {"jsonrpc":"2.0","method":"exit"}