https://github.com/ckandeler updated https://github.com/llvm/llvm-project/pull/220518
>From 7d6b09b9438f26434bc73fb266d95afa47ccb5db Mon Sep 17 00:00:00 2001 From: Christian Kandeler <[email protected]> Date: Tue, 1 Sep 2026 18:44:14 +0200 Subject: [PATCH 1/2] [clangd] Include the operator name in documentHighlight Placing the cursor on an overloaded operator's declaration (e.g. `operator new`, `operator[]`) and requesting textDocument/document Highlight only highlighted the `operator` keyword itself, not the name or symbol that follows it (`new`, `[]`, etc.), even though that name is what's actually significant to the user. ReferenceFinder already splits some references into several spelled tokens (used for Objective-C's split selector syntax); reuse that mechanism for the operator name too, extracting the tokens spelled in the operator name's source range. This only applies to the declaration's own occurrence. Assisted-by: Claude --- clang-tools-extra/clangd/XRefs.cpp | 37 +++++++++++++++++++ .../clangd/unittests/XRefsTests.cpp | 26 +++++++++++++ 2 files changed, 63 insertions(+) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 73d8eb5d005690..bbaaf1305823c7 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1014,6 +1014,30 @@ std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST) { namespace { +/// Returns the locations of the spelled tokens overlapping [Range.getBegin(), +/// Range.getEnd()], in order. Both ends of \p Range must be file locations +/// in the same file. +llvm::SmallVector<SourceLocation, 4> +tokensSpelledInRange(const syntax::TokenBuffer &TB, const SourceManager &SM, + SourceRange Range) { + llvm::SmallVector<SourceLocation, 4> Locs; + if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid()) + return Locs; + FileID FID = SM.getFileID(Range.getBegin()); + if (FID != SM.getFileID(Range.getEnd())) + return Locs; + unsigned EndOffset = SM.getFileOffset(Range.getEnd()); + llvm::ArrayRef<syntax::Token> Toks = TB.spelledTokens(FID); + auto It = llvm::partition_point(Toks, [&](const syntax::Token &Tok) { + return SM.getFileOffset(Tok.location()) < + SM.getFileOffset(Range.getBegin()); + }); + for (; It != Toks.end() && SM.getFileOffset(It->location()) <= EndOffset; + ++It) + Locs.push_back(It->location()); + return Locs; +} + /// Collects references to symbols within the main file. class ReferenceFinder : public index::IndexDataConsumer { public: @@ -1091,6 +1115,19 @@ class ReferenceFinder : public index::IndexDataConsumer { } else if (auto *OMD = llvm::dyn_cast_or_null<ObjCMethodDecl>(ASTNode.OrigD)) { OMD->getSelectorLocs(Locs); + } else if (auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(D); + FD && FD->isOverloadedOperator() && + isInsideMainFile(FD->getNameInfo().getLoc(), SM)) { + // The operator name (e.g. `new`, `[]`, `<<`) is a separate token (or + // tokens) from the `operator` keyword itself; report both so the + // whole name gets highlighted, not just the keyword. Only do this + // when the declaration itself is in the main file: TB only has + // spelled tokens for the main file, and this is only useful anyway + // when we're looking at the occurrence at the declaration itself + // (checked below). + Locs.push_back(FD->getNameInfo().getLoc()); + auto OpNameRange = FD->getNameInfo().getCXXOperatorNameRange(); + llvm::append_range(Locs, tokensSpelledInRange(TB, SM, OpNameRange)); } // Sanity check: we expect the *first* token to match the reported loc. // Otherwise, maybe it was e.g. some other kind of reference to a Decl. diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index d5ba2bc093c9c9..5641b716f552f5 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -135,6 +135,32 @@ TEST(HighlightsTest, All) { return 1; } )cpp", + R"cpp(// Overloaded operator: the whole name, not just `operator`, is highlighted. + using size_t = decltype(sizeof(0)); + struct S { + static void *[[operator]] [[n^ew]](size_t); + static void operator delete(void *); + }; + )cpp", + R"cpp(// Same, with the cursor on the operator keyword itself. + using size_t = decltype(sizeof(0)); + struct S { + static void *[[^operator]] [[new]](size_t); + static void operator delete(void *); + }; + )cpp", + R"cpp(// Same, for operator delete. + using size_t = decltype(sizeof(0)); + struct S { + static void *operator new(size_t); + static void [[operator]] [[del^ete]](void *); + }; + )cpp", + R"cpp(// Overloaded operator spanning multiple tokens. + struct S { + void [[operator]] [[^(]][[)]](int); + }; + )cpp", }; for (const char *Test : Tests) { Annotations T(Test); >From 7491567917832fc2b42893e95488dd8623552b5e Mon Sep 17 00:00:00 2001 From: Christian Kandeler <[email protected]> Date: Fri, 11 Sep 2026 16:29:29 +0200 Subject: [PATCH 2/2] [clangd] Address review: extend the fix to explicit operator-call syntax, literal and conversion operators The previous commit only handled the operator's own declaration. Two review comments pointed out related gaps: - Explicit operator-call syntax (e.g. `a.operator+(b)`) still only highlighted `operator`, not the name that follows, because the code always pulled the name range from the *target declaration*, not from the referring expression. Generalize handleDeclOccurrence to read the DeclarationNameInfo off whichever AST node represents the current occurrence (MemberExpr, DeclRefExpr, and their dependent-context counterparts), falling back to the declaration only when the occurrence has none of its own (e.g. a `new`/`delete` expression, which references its operator without an expression of its own to carry a name). This incidentally still prevents reaching into another file's tokens, since a mismatch between the found name's location and the occurrence's own location now means we don't actually know how (or whether) the name is spelled here. - Literal operators (`operator""_x`) and conversion operators (`operator int()`) weren't covered at all, since neither is an "overloaded operator" in Clang's sense. Add both: for a literal operator, the suffix is lexed together with the preceding `""` as a single token, so the whole token is used; for a conversion operator, the target type's token range is used, which naturally handles arbitrarily complex types. Reviewers also noted that find-all-references/definition still only cover the `operator` keyword, and that clicking directly on a conversion operator's target type doesn't resolve to the declaration at all. Both are out of scope here: the former was already deliberately excluded from the previous commit (a merged multi-token reference can span multiple lines, which clients might not handle properly), and the latter is a pre-existing SelectionTree/targetDecl resolution gap unrelated to this code path. Assisted-by: Claude --- clang-tools-extra/clangd/XRefs.cpp | 92 ++++++++++++++++--- .../clangd/unittests/XRefsTests.cpp | 19 ++++ 2 files changed, 98 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index bbaaf1305823c7..5a158099572058 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -41,6 +41,7 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtVisitor.h" #include "clang/AST/Type.h" +#include "clang/AST/TypeLoc.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/Module.h" #include "clang/Basic/SourceLocation.h" @@ -1038,6 +1039,77 @@ tokensSpelledInRange(const syntax::TokenBuffer &TB, const SourceManager &SM, return Locs; } +/// If this occurrence is spelled as (part of) an "operator"-shaped name -- +/// `operator+`, `operator[]`, a literal operator like `operator""_x`, or a +/// conversion operator like `operator int()` -- returns the location of the +/// `operator` keyword followed by the locations of the tokens that make up +/// the rest of the name, so callers can highlight (or otherwise report) the +/// whole name instead of just the keyword. Returns an empty list otherwise, +/// including when \p Loc is some other occurrence of \p D (e.g. an implicit +/// operator call like `a + b`, which has no `operator` token to extend). +llvm::SmallVector<SourceLocation, 4> +operatorNameTokens(const Decl *D, + const index::IndexDataConsumer::ASTNodeInfo &ASTNode, + SourceLocation Loc, const syntax::TokenBuffer &TB, + const SourceManager &SM) { + std::optional<DeclarationNameInfo> NameInfo; + if (auto *ME = llvm::dyn_cast_or_null<MemberExpr>(ASTNode.OrigE)) + NameInfo = ME->getMemberNameInfo(); + else if (auto *DRE = llvm::dyn_cast_or_null<DeclRefExpr>(ASTNode.OrigE)) + NameInfo = DRE->getNameInfo(); + else if (auto *DSME = llvm::dyn_cast_or_null<CXXDependentScopeMemberExpr>( + ASTNode.OrigE)) + NameInfo = DSME->getMemberNameInfo(); + else if (auto *DSDRE = + llvm::dyn_cast_or_null<DependentScopeDeclRefExpr>(ASTNode.OrigE)) + NameInfo = DSDRE->getNameInfo(); + else if (auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(D)) + NameInfo = FD->getNameInfo(); + // Not every occurrence carries its own name info. Most ways of invoking an + // operator without writing the `operator` keyword (e.g. `a + b`) still + // reference it through a real, if implicit, MemberExpr/DeclRefExpr callee + // that's handled by the cases above (and later filtered out below, since + // that implicit callee has no `operator` text to report). A `new T(...)` + // or `delete p;` *expression* is the odd one out: unlike a call, it has no + // callee sub-expression at all -- CXXNewExpr/CXXDeleteExpr just store the + // resolved FunctionDecl directly -- so indexing it passes no RefE, and we + // fall through to D's info above. There, NameInfo does not actually + // describe how the name is spelled at *this* occurrence -- it may belong + // to a distant reference, or even a declaration in another file entirely + // -- so its location won't match Loc. + if (!NameInfo || NameInfo->getLoc() != Loc) + return {}; + + SourceRange ExtraRange; + switch (NameInfo->getName().getNameKind()) { + case DeclarationName::CXXOperatorName: + ExtraRange = NameInfo->getCXXOperatorNameRange(); + break; + case DeclarationName::CXXLiteralOperatorName: { + // The suffix (e.g. `_test` in `operator""_test`) is lexed as part of a + // single string-literal-with-suffix token, so its location isn't a + // token's own start; look up the (whole) token that contains it. + SourceLocation SuffixLoc = NameInfo->getCXXLiteralOperatorNameLoc(); + if (SuffixLoc.isValid()) + if (const auto *Tok = TB.spelledTokenContaining(SM.getFileLoc(SuffixLoc))) + ExtraRange = SourceRange(Tok->location(), Tok->location()); + break; + } + case DeclarationName::CXXConversionFunctionName: + if (TypeSourceInfo *TInfo = NameInfo->getNamedTypeInfo()) + ExtraRange = TInfo->getTypeLoc().getSourceRange(); + break; + default: + break; + } + if (ExtraRange.getBegin().isInvalid()) + return {}; + + llvm::SmallVector<SourceLocation, 4> Result{Loc}; + llvm::append_range(Result, tokensSpelledInRange(TB, SM, ExtraRange)); + return Result; +} + /// Collects references to symbols within the main file. class ReferenceFinder : public index::IndexDataConsumer { public: @@ -1115,19 +1187,13 @@ class ReferenceFinder : public index::IndexDataConsumer { } else if (auto *OMD = llvm::dyn_cast_or_null<ObjCMethodDecl>(ASTNode.OrigD)) { OMD->getSelectorLocs(Locs); - } else if (auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(D); - FD && FD->isOverloadedOperator() && - isInsideMainFile(FD->getNameInfo().getLoc(), SM)) { - // The operator name (e.g. `new`, `[]`, `<<`) is a separate token (or - // tokens) from the `operator` keyword itself; report both so the - // whole name gets highlighted, not just the keyword. Only do this - // when the declaration itself is in the main file: TB only has - // spelled tokens for the main file, and this is only useful anyway - // when we're looking at the occurrence at the declaration itself - // (checked below). - Locs.push_back(FD->getNameInfo().getLoc()); - auto OpNameRange = FD->getNameInfo().getCXXOperatorNameRange(); - llvm::append_range(Locs, tokensSpelledInRange(TB, SM, OpNameRange)); + } else { + // An "operator"-shaped name (operator+, operator""_x, operator + // int()) has a name that's a separate token (or tokens) from the + // `operator` keyword itself; report both so the whole name gets + // highlighted, not just the keyword. This covers both the + // declaration and explicit references to it, e.g. `a.operator+(b)`. + Locs = operatorNameTokens(D, ASTNode, Loc, TB, SM); } // Sanity check: we expect the *first* token to match the reported loc. // Otherwise, maybe it was e.g. some other kind of reference to a Decl. diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 5641b716f552f5..7f9ff8c0ecbf5d 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -161,6 +161,25 @@ TEST(HighlightsTest, All) { void [[operator]] [[^(]][[)]](int); }; )cpp", + R"cpp(// Explicit operator-call syntax also highlights the whole name. + struct S { + S [[operator]] [[+]](S); + }; + void f(S a) { + a.[[operator]] [[^+]](a); + } + )cpp", + R"cpp(// Literal operator: the suffix is lexed together with the preceding + // `""` as a single token, so the whole thing is highlighted. + long double [[operator]] [[""_te^st]](long double); + )cpp", + R"cpp(// Conversion operator: the target type name is highlighted too. + // (Clicking on `int` itself doesn't resolve to the declaration at + // all, a separate limitation.) + struct S { + [[^operator]] [[int]](); + }; + )cpp", }; for (const char *Test : Tests) { Annotations T(Test); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
