Author: Christian Kandeler Date: 2026-09-16T12:08:31+02:00 New Revision: fd63167bbd7f0193f21129abe49604b98f777cdc
URL: https://github.com/llvm/llvm-project/commit/fd63167bbd7f0193f21129abe49604b98f777cdc DIFF: https://github.com/llvm/llvm-project/commit/fd63167bbd7f0193f21129abe49604b98f777cdc.diff LOG: [clangd] Include the operator name in documentHighlight (#220518) ## Summary - Placing the cursor on an overloaded operator's declaration (e.g. `operator new`, `operator[]`) and requesting `textDocument/documentHighlight` 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); this reuses 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. ## Test plan - [x] `ninja check-clangd` passes - [x] New cases added to `HighlightsTest.All` in `clang-tools-extra/clangd/unittests/XRefsTests.cpp` covering `operator new`, `operator delete` (clicking on either the keyword or the name), and a multi-token operator (`operator()`) Added: Modified: clang-tools-extra/clangd/XRefs.cpp clang-tools-extra/clangd/unittests/XRefsTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 73d8eb5d00569..e68e6a08adce5 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" @@ -1014,6 +1015,110 @@ std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST) { namespace { +/// Returns the locations of the spelled tokens overlapping [Range.getBegin(), +/// Range.getEnd()], in order. Returns an empty list unless both ends of +/// \p Range are file locations in the same file: unlike a spelling or +/// expansion location, a macro location isn't something TokenBuffer (or the +/// FileID/offset arithmetic below) can make sense of, e.g. if part of an +/// operator name comes from a macro (`#define PLUS + ... operator PLUS(int)`). +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() || + !Range.getBegin().isFileID() || !Range.getEnd().isFileID()) + 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; +} + +/// 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 *OE = llvm::dyn_cast_or_null<OverloadExpr>(ASTNode.OrigE)) + // An UnresolvedMemberExpr/UnresolvedLookupExpr: overload resolution for + // this call is dependent (e.g. on a template parameter), so it reports a + // reference to every candidate at the same (real, spelled) location. + NameInfo = OE->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: @@ -1091,6 +1196,13 @@ class ReferenceFinder : public index::IndexDataConsumer { } else if (auto *OMD = llvm::dyn_cast_or_null<ObjCMethodDecl>(ASTNode.OrigD)) { OMD->getSelectorLocs(Locs); + } 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 d5ba2bc093c9c..9d1a34f23955c 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -135,6 +135,70 @@ 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", + 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", + R"cpp(// Regression: an operator name coming from a macro expansion must not + // crash. Since a macro location isn't something we can safely treat + // as spelled tokens, we fall back to highlighting just `operator`. + #define PLUS + + struct S { void [[operator]] PLU^S(int); }; + )cpp", + R"cpp(// Regression: an overloaded operator called with dependent arguments + // (so overload resolution is deferred, producing an + // UnresolvedMemberExpr with several candidates at one location) + // should still have its whole name highlighted, not just `operator`. + struct S { + void operator+(int); + void [[operat^or]] [[+]](double); + }; + template <typename T> + void foo(S s, T t) { + s.[[operator]] [[+]](t); + } + )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
