From 103b18fd135e523ee323ce6dc7e09d5ef2eb7f6a Mon Sep 17 00:00:00 2001 From: chouzz Date: Fri, 25 Oct 2024 17:42:04 +0800 Subject: [PATCH 01/26] [clangd] Support symbolTags for document symbol --- clang-tools-extra/clangd/AST.cpp | 63 ++++++++++++++ clang-tools-extra/clangd/AST.h | 31 +++++++ clang-tools-extra/clangd/FindSymbols.cpp | 30 +++++++ clang-tools-extra/clangd/Protocol.h | 29 ++++++- .../clangd/SemanticHighlighting.cpp | 85 ------------------- 5 files changed, 151 insertions(+), 87 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 0dcff2eae05e7..a4250bf9b313c 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -194,6 +194,69 @@ bool isImplementationDetail(const Decl *D) { D->getASTContext().getSourceManager()); } +// Whether T is const in a loose sense - is a variable with this type readonly? +bool isConst(QualType T) { + if (T.isNull()) + return false; + T = T.getNonReferenceType(); + if (T.isConstQualified()) + return true; + if (const auto *AT = T->getAsArrayTypeUnsafe()) + return isConst(AT->getElementType()); + if (isConst(T->getPointeeType())) + return true; + return false; +} + +bool isConst(const Decl *D) { + if (llvm::isa(D) || llvm::isa(D)) + return true; + if (llvm::isa(D) || llvm::isa(D) || + llvm::isa(D) || llvm::isa(D)) { + if (isConst(llvm::cast(D)->getType())) + return true; + } + if (const auto *OCPD = llvm::dyn_cast(D)) { + if (OCPD->isReadOnly()) + return true; + } + if (const auto *MPD = llvm::dyn_cast(D)) { + if (!MPD->hasSetter()) + return true; + } + if (const auto *CMD = llvm::dyn_cast(D)) { + if (CMD->isConst()) + return true; + } + return false; +} + +bool isStatic(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isStatic(); + if (const VarDecl *VD = llvm::dyn_cast(D)) + return VD->isStaticDataMember() || VD->isStaticLocal(); + if (const auto *OPD = llvm::dyn_cast(D)) + return OPD->isClassProperty(); + if (const auto *OMD = llvm::dyn_cast(D)) + return OMD->isClassMethod(); + return false; +} + +bool isAbstract(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isPureVirtual(); + if (const auto *CRD = llvm::dyn_cast(D)) + return CRD->hasDefinition() && CRD->isAbstract(); + return false; +} + +bool isVirtual(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isVirtual(); + return false; +} + SourceLocation nameLocation(const clang::Decl &D, const SourceManager &SM) { auto L = D.getLocation(); // For `- (void)foo` we want `foo` not the `-`. diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 2b83595e5b8e9..2fc0f8a7d2b6d 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -154,6 +154,37 @@ bool isImplicitTemplateInstantiation(const NamedDecl *D); /// explicit specialization. bool isExplicitTemplateSpecialization(const NamedDecl *D); +// Whether T is const in a loose sense - is a variable with this type readonly? +bool isConst(QualType T); + +// Whether D is const in a loose sense (should it be highlighted as such?) +// FIXME: This is separate from whether *a particular usage* can mutate D. +// We may want V in V.size() to be readonly even if V is mutable. +bool isConst(const Decl *D); + +// "Static" means many things in C++, only some get the "static" modifier. +// +// Meanings that do: +// - Members associated with the class rather than the instance. +// This is what 'static' most often means across languages. +// - static local variables +// These are similarly "detached from their context" by the static keyword. +// In practice, these are rarely used inside classes, reducing confusion. +// +// Meanings that don't: +// - Namespace-scoped variables, which have static storage class. +// This is implicit, so the keyword "static" isn't so strongly associated. +// If we want a modifier for these, "global scope" is probably the concept. +// - Namespace-scoped variables/functions explicitly marked "static". +// There the keyword changes *linkage* , which is a totally different concept. +// If we want to model this, "file scope" would be a nice modifier. +// +// This is confusing, and maybe we should use another name, but because "static" +// is a standard LSP modifier, having one with that name has advantages. +bool isStatic(const Decl *D); +bool isAbstract(const Decl *D); +bool isVirtual(const Decl *D); + /// Returns a nested name specifier loc of \p ND if it was present in the /// source, e.g. /// void ns::something::foo() -> returns 'ns::something' diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 7655a39d5ba1f..5b04adeb1e1f2 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -188,6 +188,35 @@ std::string getSymbolName(ASTContext &Ctx, const NamedDecl &ND) { return printName(Ctx, ND); } +std::vector getSymbolTags(const NamedDecl &ND) +{ + std::vector Tags; + if (ND.isDeprecated()) + Tags.push_back(SymbolTag::Deprecated); + if (isConst(&ND)) + Tags.push_back(SymbolTag::Constant); + if (isStatic(&ND)) + Tags.push_back(SymbolTag::Static); + if (const FieldDecl *FD = dyn_cast(&ND)) { + switch (FD->getAccess()) { + case AS_public: + Tags.push_back(SymbolTag::Public); + break; + case AS_protected: + Tags.push_back(SymbolTag::Protected); + break; + case AS_private: + Tags.push_back(SymbolTag::Private); + break; + default: + break; + } + } + if (isVirtual(&ND)) + Tags.push_back(SymbolTag::Virtual); + return Tags; +} + std::string getSymbolDetail(ASTContext &Ctx, const NamedDecl &ND) { PrintingPolicy P(Ctx.getPrintingPolicy()); P.SuppressScope = true; @@ -242,6 +271,7 @@ std::optional declToSym(ASTContext &Ctx, const NamedDecl &ND) { SI.range = Range{sourceLocToPosition(SM, SymbolRange->getBegin()), sourceLocToPosition(SM, SymbolRange->getEnd())}; SI.detail = getSymbolDetail(Ctx, ND); + SI.tags = getSymbolTags(ND); SourceLocation NameLoc = ND.getLocation(); SourceLocation FallbackNameLoc; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 2248572060431..2abdea4a86455 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1104,6 +1104,30 @@ struct CodeAction { }; llvm::json::Value toJSON(const CodeAction &); +enum class SymbolTag { + Deprecated = 1 , + Private = 2, + Package = 3, + Protected = 4, + Public = 5, + Internal= 6, + File = 7, + Static = 8, + Abstract = 9, + Final = 10, + Sealed = 11, + Constant = 12, + Transient = 13, + Volatile = 14, + Synchronized = 15, + Virtual = 16, + Nullable = 17, + NonNull = 18, + Declaration = 19, + Definition = 20, + ReadOnly = 21, +}; +llvm::json::Value toJSON(SymbolTag); /// Represents programming constructs like variables, classes, interfaces etc. /// that appear in a document. Document symbols can be hierarchical and they /// have two ranges: one that encloses its definition and one that points to its @@ -1121,6 +1145,9 @@ struct DocumentSymbol { /// Indicates if this symbol is deprecated. bool deprecated = false; + /// Tags for this symbol, e.g public, private, static, const etc. + std::vector tags; + /// The range enclosing this symbol not including leading/trailing whitespace /// but everything else like comments. This information is typically used to /// determine if the clients cursor is inside the symbol to reveal in the @@ -1572,8 +1599,6 @@ struct ResolveTypeHierarchyItemParams { bool fromJSON(const llvm::json::Value &, ResolveTypeHierarchyItemParams &, llvm::json::Path); -enum class SymbolTag { Deprecated = 1 }; -llvm::json::Value toJSON(SymbolTag); /// The parameter of a `textDocument/prepareCallHierarchy` request. struct CallHierarchyPrepareParams : public TextDocumentPositionParams {}; diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index ab720ebe6b47f..49d844719eb6b 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -192,91 +192,6 @@ std::optional kindForType(const Type *TP, return std::nullopt; } -// Whether T is const in a loose sense - is a variable with this type readonly? -bool isConst(QualType T) { - if (T.isNull()) - return false; - T = T.getNonReferenceType(); - if (T.isConstQualified()) - return true; - if (const auto *AT = T->getAsArrayTypeUnsafe()) - return isConst(AT->getElementType()); - if (isConst(T->getPointeeType())) - return true; - return false; -} - -// Whether D is const in a loose sense (should it be highlighted as such?) -// FIXME: This is separate from whether *a particular usage* can mutate D. -// We may want V in V.size() to be readonly even if V is mutable. -bool isConst(const Decl *D) { - if (llvm::isa(D) || llvm::isa(D)) - return true; - if (llvm::isa(D) || llvm::isa(D) || - llvm::isa(D) || llvm::isa(D)) { - if (isConst(llvm::cast(D)->getType())) - return true; - } - if (const auto *OCPD = llvm::dyn_cast(D)) { - if (OCPD->isReadOnly()) - return true; - } - if (const auto *MPD = llvm::dyn_cast(D)) { - if (!MPD->hasSetter()) - return true; - } - if (const auto *CMD = llvm::dyn_cast(D)) { - if (CMD->isConst()) - return true; - } - return false; -} - -// "Static" means many things in C++, only some get the "static" modifier. -// -// Meanings that do: -// - Members associated with the class rather than the instance. -// This is what 'static' most often means across languages. -// - static local variables -// These are similarly "detached from their context" by the static keyword. -// In practice, these are rarely used inside classes, reducing confusion. -// -// Meanings that don't: -// - Namespace-scoped variables, which have static storage class. -// This is implicit, so the keyword "static" isn't so strongly associated. -// If we want a modifier for these, "global scope" is probably the concept. -// - Namespace-scoped variables/functions explicitly marked "static". -// There the keyword changes *linkage* , which is a totally different concept. -// If we want to model this, "file scope" would be a nice modifier. -// -// This is confusing, and maybe we should use another name, but because "static" -// is a standard LSP modifier, having one with that name has advantages. -bool isStatic(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isStatic(); - if (const VarDecl *VD = llvm::dyn_cast(D)) - return VD->isStaticDataMember() || VD->isStaticLocal(); - if (const auto *OPD = llvm::dyn_cast(D)) - return OPD->isClassProperty(); - if (const auto *OMD = llvm::dyn_cast(D)) - return OMD->isClassMethod(); - return false; -} - -bool isAbstract(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isPureVirtual(); - if (const auto *CRD = llvm::dyn_cast(D)) - return CRD->hasDefinition() && CRD->isAbstract(); - return false; -} - -bool isVirtual(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isVirtual(); - return false; -} - bool isDependent(const Decl *D) { if (isa(D)) return true; From 18c71ac09c38b587b243ab0cb15bde80f0b7a590 Mon Sep 17 00:00:00 2001 From: chouzz Date: Mon, 28 Oct 2024 20:14:15 +0800 Subject: [PATCH 02/26] Fix access for class method --- clang-tools-extra/clangd/FindSymbols.cpp | 6 ++++-- clang-tools-extra/clangd/Protocol.cpp | 2 ++ clang-tools-extra/clangd/SemanticHighlighting.cpp | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 5b04adeb1e1f2..c44b3339b435f 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -197,6 +197,9 @@ std::vector getSymbolTags(const NamedDecl &ND) Tags.push_back(SymbolTag::Constant); if (isStatic(&ND)) Tags.push_back(SymbolTag::Static); + if (isVirtual(&ND)) + Tags.push_back(SymbolTag::Virtual); + if (const FieldDecl *FD = dyn_cast(&ND)) { switch (FD->getAccess()) { case AS_public: @@ -212,8 +215,7 @@ std::vector getSymbolTags(const NamedDecl &ND) break; } } - if (isVirtual(&ND)) - Tags.push_back(SymbolTag::Virtual); + return Tags; } diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 560b8e00ed377..9926f2dd63de5 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -964,6 +964,8 @@ llvm::json::Value toJSON(const DocumentSymbol &S) { Result["children"] = S.children; if (S.deprecated) Result["deprecated"] = true; + if (!S.tags.empty()) + Result["tags"] = S.tags; // FIXME: workaround for older gcc/clang return std::move(Result); } diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 49d844719eb6b..53d9946859582 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "SemanticHighlighting.h" +#include "AST.h" #include "Config.h" #include "FindTarget.h" #include "ParsedAST.h" From af6491c6e68b6717192682951215afdb724cc847 Mon Sep 17 00:00:00 2001 From: chouzz Date: Thu, 31 Oct 2024 19:47:47 +0800 Subject: [PATCH 03/26] Support Declaration and Definition tags --- clang-tools-extra/clangd/AST.cpp | 17 +++++++ clang-tools-extra/clangd/AST.h | 2 +- clang-tools-extra/clangd/FindSymbols.cpp | 46 ++++++++++--------- .../clangd/SemanticHighlighting.cpp | 17 ------- 4 files changed, 42 insertions(+), 40 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index a4250bf9b313c..7de45986a0522 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -257,6 +257,23 @@ bool isVirtual(const Decl *D) { return false; } +bool isUniqueDefinition(const NamedDecl *Decl) { + if (auto *Func = dyn_cast(Decl)) + return Func->isThisDeclarationADefinition(); + if (auto *Klass = dyn_cast(Decl)) + return Klass->isThisDeclarationADefinition(); + if (auto *Iface = dyn_cast(Decl)) + return Iface->isThisDeclarationADefinition(); + if (auto *Proto = dyn_cast(Decl)) + return Proto->isThisDeclarationADefinition(); + if (auto *Var = dyn_cast(Decl)) + return Var->isThisDeclarationADefinition(); + return isa(Decl) || + isa(Decl) || + isa(Decl) || isa(Decl) || + isa(Decl); +} + SourceLocation nameLocation(const clang::Decl &D, const SourceManager &SM) { auto L = D.getLocation(); // For `- (void)foo` we want `foo` not the `-`. diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 2fc0f8a7d2b6d..5fc667f3520db 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -184,7 +184,7 @@ bool isConst(const Decl *D); bool isStatic(const Decl *D); bool isAbstract(const Decl *D); bool isVirtual(const Decl *D); - +bool isUniqueDefinition(const NamedDecl *Decl); /// Returns a nested name specifier loc of \p ND if it was present in the /// source, e.g. /// void ns::something::foo() -> returns 'ns::something' diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index c44b3339b435f..7d978a2849d9b 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -188,34 +188,36 @@ std::string getSymbolName(ASTContext &Ctx, const NamedDecl &ND) { return printName(Ctx, ND); } -std::vector getSymbolTags(const NamedDecl &ND) -{ +std::vector getSymbolTags(const NamedDecl &ND) { std::vector Tags; - if (ND.isDeprecated()) + if (ND.isDeprecated()) Tags.push_back(SymbolTag::Deprecated); - if (isConst(&ND)) - Tags.push_back(SymbolTag::Constant); - if (isStatic(&ND)) - Tags.push_back(SymbolTag::Static); + if (isConst(&ND)) + Tags.push_back(SymbolTag::Constant); + if (isStatic(&ND)) + Tags.push_back(SymbolTag::Static); if (isVirtual(&ND)) - Tags.push_back(SymbolTag::Virtual); - + Tags.push_back(SymbolTag::Virtual); + if (!isa(ND)) + Tags.push_back(SymbolTag::Declaration); + if (isUniqueDefinition(&ND)) + Tags.push_back(SymbolTag::Definition); if (const FieldDecl *FD = dyn_cast(&ND)) { switch (FD->getAccess()) { - case AS_public: - Tags.push_back(SymbolTag::Public); - break; - case AS_protected: - Tags.push_back(SymbolTag::Protected); - break; - case AS_private: - Tags.push_back(SymbolTag::Private); - break; - default: - break; + case AS_public: + Tags.push_back(SymbolTag::Public); + break; + case AS_protected: + Tags.push_back(SymbolTag::Protected); + break; + case AS_private: + Tags.push_back(SymbolTag::Private); + break; + default: + break; } - } - + } + return Tags; } diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 53d9946859582..f53343951d7de 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -78,23 +78,6 @@ bool canHighlightName(DeclarationName Name) { llvm_unreachable("invalid name kind"); } -bool isUniqueDefinition(const NamedDecl *Decl) { - if (auto *Func = dyn_cast(Decl)) - return Func->isThisDeclarationADefinition(); - if (auto *Klass = dyn_cast(Decl)) - return Klass->isThisDeclarationADefinition(); - if (auto *Iface = dyn_cast(Decl)) - return Iface->isThisDeclarationADefinition(); - if (auto *Proto = dyn_cast(Decl)) - return Proto->isThisDeclarationADefinition(); - if (auto *Var = dyn_cast(Decl)) - return Var->isThisDeclarationADefinition(); - return isa(Decl) || - isa(Decl) || - isa(Decl) || isa(Decl) || - isa(Decl); -} - std::optional kindForType(const Type *TP, const HeuristicResolver *Resolver); std::optional kindForDecl(const NamedDecl *D, From baa29b1a6af59b0decf693cee9c4ddfec3f570ac Mon Sep 17 00:00:00 2001 From: chouzz Date: Mon, 18 Nov 2024 14:43:14 +0800 Subject: [PATCH 04/26] Remove constant tag --- clang-tools-extra/clangd/AST.cpp | 4 +++ clang-tools-extra/clangd/FindSymbols.cpp | 36 +++++++++++++----------- clang-tools-extra/clangd/Protocol.h | 21 +++++++------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 7de45986a0522..51ab1cdadd0e1 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -228,6 +228,8 @@ bool isConst(const Decl *D) { if (CMD->isConst()) return true; } + if (const auto *FD = llvm::dyn_cast(D)) + return isConst(FD->getReturnType()); return false; } @@ -240,6 +242,8 @@ bool isStatic(const Decl *D) { return OPD->isClassProperty(); if (const auto *OMD = llvm::dyn_cast(D)) return OMD->isClassMethod(); + if (const auto *FD = llvm::dyn_cast(D)) + return FD->isStatic(); return false; } diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 7d978a2849d9b..bdb79fde5a38a 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -193,29 +193,31 @@ std::vector getSymbolTags(const NamedDecl &ND) { if (ND.isDeprecated()) Tags.push_back(SymbolTag::Deprecated); if (isConst(&ND)) - Tags.push_back(SymbolTag::Constant); + Tags.push_back(SymbolTag::ReadOnly); if (isStatic(&ND)) Tags.push_back(SymbolTag::Static); if (isVirtual(&ND)) Tags.push_back(SymbolTag::Virtual); - if (!isa(ND)) - Tags.push_back(SymbolTag::Declaration); + if (isAbstract(&ND)) + Tags.push_back(SymbolTag::Abstract); + if (isUniqueDefinition(&ND)) Tags.push_back(SymbolTag::Definition); - if (const FieldDecl *FD = dyn_cast(&ND)) { - switch (FD->getAccess()) { - case AS_public: - Tags.push_back(SymbolTag::Public); - break; - case AS_protected: - Tags.push_back(SymbolTag::Protected); - break; - case AS_private: - Tags.push_back(SymbolTag::Private); - break; - default: - break; - } + else if (!isa(ND)) + Tags.push_back(SymbolTag::Declaration); + + switch (ND.getAccess()) { + case AS_public: + Tags.push_back(SymbolTag::Public); + break; + case AS_protected: + Tags.push_back(SymbolTag::Protected); + break; + case AS_private: + Tags.push_back(SymbolTag::Private); + break; + default: + break; } return Tags; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 2abdea4a86455..607551f1b2d9e 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1105,7 +1105,7 @@ struct CodeAction { llvm::json::Value toJSON(const CodeAction &); enum class SymbolTag { - Deprecated = 1 , + Deprecated = 1, Private = 2, Package = 3, Protected = 4, @@ -1116,16 +1116,15 @@ enum class SymbolTag { Abstract = 9, Final = 10, Sealed = 11, - Constant = 12, - Transient = 13, - Volatile = 14, - Synchronized = 15, - Virtual = 16, - Nullable = 17, - NonNull = 18, - Declaration = 19, - Definition = 20, - ReadOnly = 21, + Transient = 12, + Volatile = 13, + Synchronized = 14, + Virtual = 15, + Nullable = 16, + NonNull = 17, + Declaration = 18, + Definition = 19, + ReadOnly = 20, }; llvm::json::Value toJSON(SymbolTag); /// Represents programming constructs like variables, classes, interfaces etc. From 64d5a3642f17621fed8ad551378eca467694c538 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 11 Nov 2025 23:05:47 +0100 Subject: [PATCH 05/26] Fix test --- clang-tools-extra/clangd/test/symbols.test | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/test/symbols.test b/clang-tools-extra/clangd/test/symbols.test index af5d74123630e..40115f2496e50 100644 --- a/clang-tools-extra/clangd/test/symbols.test +++ b/clang-tools-extra/clangd/test/symbols.test @@ -56,7 +56,10 @@ # CHECK-NEXT: "character": {{.*}}, # CHECK-NEXT: "line": {{.*}} # CHECK-NEXT: } -# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 18 +# CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { # CHECK-NEXT: "detail": "int ()", @@ -81,7 +84,10 @@ # CHECK-NEXT: "character": {{.*}}, # CHECK-NEXT: "line": {{.*}} # CHECK-NEXT: } -# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 19 +# CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ] # CHECK-NEXT:} From 49551b5db5e1b7e6702d5691a44388c8be6d776c Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 12 Nov 2025 16:14:55 +0100 Subject: [PATCH 06/26] Tagging final methods and classes --- clang-tools-extra/clangd/AST.cpp | 10 ++++++++++ clang-tools-extra/clangd/AST.h | 1 + clang-tools-extra/clangd/FindSymbols.cpp | 8 ++++++++ 3 files changed, 19 insertions(+) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 51ab1cdadd0e1..b0957a4c0fc01 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -261,6 +261,16 @@ bool isVirtual(const Decl *D) { return false; } +bool isFinal(const Decl *D) { + if (const auto *CRD = dyn_cast(D)) + return CRD->hasAttr(); + + if (const auto *CRD = dyn_cast(D)) + return CRD->hasAttr(); + + return false; +} + bool isUniqueDefinition(const NamedDecl *Decl) { if (auto *Func = dyn_cast(Decl)) return Func->isThisDeclarationADefinition(); diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 5fc667f3520db..8d31dbe93ef3b 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -184,6 +184,7 @@ bool isConst(const Decl *D); bool isStatic(const Decl *D); bool isAbstract(const Decl *D); bool isVirtual(const Decl *D); +bool isFinal(const Decl *D); bool isUniqueDefinition(const NamedDecl *Decl); /// Returns a nested name specifier loc of \p ND if it was present in the /// source, e.g. diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index bdb79fde5a38a..e8a6bca17215c 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -190,17 +190,25 @@ std::string getSymbolName(ASTContext &Ctx, const NamedDecl &ND) { std::vector getSymbolTags(const NamedDecl &ND) { std::vector Tags; + if (ND.isDeprecated()) Tags.push_back(SymbolTag::Deprecated); + if (isConst(&ND)) Tags.push_back(SymbolTag::ReadOnly); + if (isStatic(&ND)) Tags.push_back(SymbolTag::Static); + if (isVirtual(&ND)) Tags.push_back(SymbolTag::Virtual); + if (isAbstract(&ND)) Tags.push_back(SymbolTag::Abstract); + if (isFinal(&ND)) + Tags.push_back(SymbolTag::Final); + if (isUniqueDefinition(&ND)) Tags.push_back(SymbolTag::Definition); else if (!isa(ND)) From 8a4b3149a2d7336ba4be0d4b02639daa7a44cff3 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 13 Nov 2025 13:04:11 +0100 Subject: [PATCH 07/26] Checks the extraction of symbol tags --- .../clangd/test/symbol-tags.test | 293 ++++++++++++++++++ 1 file changed, 293 insertions(+) create mode 100644 clang-tools-extra/clangd/test/symbol-tags.test diff --git a/clang-tools-extra/clangd/test/symbol-tags.test b/clang-tools-extra/clangd/test/symbol-tags.test new file mode 100644 index 0000000000000..3d4821ae32081 --- /dev/null +++ b/clang-tools-extra/clangd/test/symbol-tags.test @@ -0,0 +1,293 @@ +# COM: Checks the extraction of symbol tags. + +# RUN: clangd -lit-test < %s | FileCheck %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"documentSymbol":{"hierarchicalDocumentSymbolSupport":true}},"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}}}},"trace":"off"}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1, + "text": + "class AbstractClass{\n public:\n virtual ~AbstractClass() = default;\n virtual void f1() = 0;\n void f2() const;\n protected: \n void f3(){}\n private: \n static void f4(){} \n }; void AbstractClass::f2() const {} \n class ImplClass final: public AbstractClass { \n public: \n void f1() final {}};" +}}} +--- +{"jsonrpc":"2.0","id":2,"method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"test:///main.cpp"}}} +# CHECK: "id": 2, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "children": [ +# CHECK-NEXT: { +# CHECK-NEXT: "kind": 9, +# CHECK-NEXT: "name": "~AbstractClass", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 35, +# CHECK-NEXT: "line": 2 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 1, +# CHECK-NEXT: "line": 2 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 10, +# CHECK-NEXT: "line": 2 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 9, +# CHECK-NEXT: "line": 2 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 15, +# CHECK-NEXT: 19, +# CHECK-NEXT: 5 +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "detail": "void ()", +# CHECK-NEXT: "kind": 6, +# CHECK-NEXT: "name": "f1", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 22, +# CHECK-NEXT: "line": 3 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 1, +# CHECK-NEXT: "line": 3 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 16, +# CHECK-NEXT: "line": 3 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 14, +# CHECK-NEXT: "line": 3 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 15, +# CHECK-NEXT: 9, +# CHECK-NEXT: 18, +# CHECK-NEXT: 5 +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "detail": "void () const", +# CHECK-NEXT: "kind": 6, +# CHECK-NEXT: "name": "f2", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 16, +# CHECK-NEXT: "line": 4 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 1, +# CHECK-NEXT: "line": 4 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 8, +# CHECK-NEXT: "line": 4 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 6, +# CHECK-NEXT: "line": 4 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 20, +# CHECK-NEXT: 18, +# CHECK-NEXT: 5 +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "detail": "void ()", +# CHECK-NEXT: "kind": 6, +# CHECK-NEXT: "name": "f3", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 12, +# CHECK-NEXT: "line": 6 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 1, +# CHECK-NEXT: "line": 6 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 8, +# CHECK-NEXT: "line": 6 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 6, +# CHECK-NEXT: "line": 6 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 19, +# CHECK-NEXT: 4 +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "detail": "void ()", +# CHECK-NEXT: "kind": 6, +# CHECK-NEXT: "name": "f4", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 19, +# CHECK-NEXT: "line": 8 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 1, +# CHECK-NEXT: "line": 8 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 15, +# CHECK-NEXT: "line": 8 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 13, +# CHECK-NEXT: "line": 8 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 8, +# CHECK-NEXT: 19, +# CHECK-NEXT: 2 +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "detail": "class", +# CHECK-NEXT: "kind": 5, +# CHECK-NEXT: "name": "AbstractClass", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 2, +# CHECK-NEXT: "line": 9 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 0, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 19, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 6, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 9, +# CHECK-NEXT: 19 +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "detail": "void () const", +# CHECK-NEXT: "kind": 6, +# CHECK-NEXT: "name": "AbstractClass::f2", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 37, +# CHECK-NEXT: "line": 9 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 4, +# CHECK-NEXT: "line": 9 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 26, +# CHECK-NEXT: "line": 9 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 9 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 20, +# CHECK-NEXT: 19, +# CHECK-NEXT: 5 +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "children": [ +# CHECK-NEXT: { +# CHECK-NEXT: "detail": "void ()", +# CHECK-NEXT: "kind": 6, +# CHECK-NEXT: "name": "f1", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 19, +# CHECK-NEXT: "line": 12 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 1, +# CHECK-NEXT: "line": 12 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 8, +# CHECK-NEXT: "line": 12 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 6, +# CHECK-NEXT: "line": 12 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 15, +# CHECK-NEXT: 10, +# CHECK-NEXT: 19, +# CHECK-NEXT: 5 +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "detail": "class", +# CHECK-NEXT: "kind": 5, +# CHECK-NEXT: "name": "ImplClass", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 20, +# CHECK-NEXT: "line": 12 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 1, +# CHECK-NEXT: "line": 10 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "selectionRange": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 16, +# CHECK-NEXT: "line": 10 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 7, +# CHECK-NEXT: "line": 10 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 10, +# CHECK-NEXT: 19 +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT:} + +--- +{"jsonrpc":"2.0","id":3,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} From 49574d8671be536ca8c056f309dfbe8ec2b4754e Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 13 Nov 2025 16:40:17 +0100 Subject: [PATCH 08/26] Docu update --- clang-tools-extra/clangd/AST.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 8d31dbe93ef3b..ef1c95d26153e 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -182,9 +182,13 @@ bool isConst(const Decl *D); // This is confusing, and maybe we should use another name, but because "static" // is a standard LSP modifier, having one with that name has advantages. bool isStatic(const Decl *D); +// Indicates whether declaration D is abstract in cases where D is a struct or a class. bool isAbstract(const Decl *D); +// Indicates whether declaration D is virtual in cases where D is a method. bool isVirtual(const Decl *D); +// Indicates whether declaration D is final in cases where D is a struct, class or method. bool isFinal(const Decl *D); +// Indicates whether declaration D is a unique definition (as opposed to a declaration). bool isUniqueDefinition(const NamedDecl *Decl); /// Returns a nested name specifier loc of \p ND if it was present in the /// source, e.g. From a5a85556ad05fc68dab7ca311a68c784e7c8891f Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 18 Nov 2025 11:02:49 +0100 Subject: [PATCH 09/26] Fix code format. --- clang-tools-extra/clangd/AST.h | 9 ++++++--- clang-tools-extra/clangd/Protocol.h | 23 +++++++++++------------ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index ef1c95d26153e..3b4e6eb43c44b 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -182,13 +182,16 @@ bool isConst(const Decl *D); // This is confusing, and maybe we should use another name, but because "static" // is a standard LSP modifier, having one with that name has advantages. bool isStatic(const Decl *D); -// Indicates whether declaration D is abstract in cases where D is a struct or a class. +// Indicates whether declaration D is abstract in cases where D is a struct or a +// class. bool isAbstract(const Decl *D); // Indicates whether declaration D is virtual in cases where D is a method. bool isVirtual(const Decl *D); -// Indicates whether declaration D is final in cases where D is a struct, class or method. +// Indicates whether declaration D is final in cases where D is a struct, class +// or method. bool isFinal(const Decl *D); -// Indicates whether declaration D is a unique definition (as opposed to a declaration). +// Indicates whether declaration D is a unique definition (as opposed to a +// declaration). bool isUniqueDefinition(const NamedDecl *Decl); /// Returns a nested name specifier loc of \p ND if it was present in the /// source, e.g. diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 607551f1b2d9e..43e68dd38c57d 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -281,7 +281,7 @@ struct TextDocumentEdit { /// The text document to change. VersionedTextDocumentIdentifier textDocument; - /// The edits to be applied. + /// The edits to be applied. /// FIXME: support the AnnotatedTextEdit variant. std::vector edits; }; @@ -560,7 +560,7 @@ struct ClientCapabilities { /// The client supports versioned document changes for WorkspaceEdit. bool DocumentChanges = false; - + /// The client supports change annotations on text edits, bool ChangeAnnotation = false; @@ -1027,12 +1027,12 @@ struct WorkspaceEdit { /// Versioned document edits. /// /// If a client neither supports `documentChanges` nor - /// `workspace.workspaceEdit.resourceOperations` then only plain `TextEdit`s - /// using the `changes` property are supported. + /// `workspace.workspaceEdit.resourceOperations` then only plain `TextEdit`s + /// using the `changes` property are supported. std::optional> documentChanges; - + /// A map of change annotations that can be referenced in - /// AnnotatedTextEdit. + /// AnnotatedTextEdit. std::map changeAnnotations; }; bool fromJSON(const llvm::json::Value &, WorkspaceEdit &, llvm::json::Path); @@ -1104,13 +1104,13 @@ struct CodeAction { }; llvm::json::Value toJSON(const CodeAction &); -enum class SymbolTag { +enum class SymbolTag { Deprecated = 1, Private = 2, Package = 3, Protected = 4, Public = 5, - Internal= 6, + Internal = 6, File = 7, Static = 8, Abstract = 9, @@ -1314,13 +1314,13 @@ enum class InsertTextFormat { /// Additional details for a completion item label. struct CompletionItemLabelDetails { /// An optional string which is rendered less prominently directly after label - /// without any spacing. Should be used for function signatures or type + /// without any spacing. Should be used for function signatures or type /// annotations. std::string detail; /// An optional string which is rendered less prominently after - /// CompletionItemLabelDetails.detail. Should be used for fully qualified - /// names or file path. + /// CompletionItemLabelDetails.detail. Should be used for fully qualified + /// names or file path. std::string description; }; llvm::json::Value toJSON(const CompletionItemLabelDetails &); @@ -1598,7 +1598,6 @@ struct ResolveTypeHierarchyItemParams { bool fromJSON(const llvm::json::Value &, ResolveTypeHierarchyItemParams &, llvm::json::Path); - /// The parameter of a `textDocument/prepareCallHierarchy` request. struct CallHierarchyPrepareParams : public TextDocumentPositionParams {}; From 8131e6f09e7d51bcfd992f7c8817980e6f862172 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 27 Nov 2025 15:13:55 +0100 Subject: [PATCH 10/26] Fill symbol tags for the response CallHierarchyItem of prepareCallHierarchy. --- clang-tools-extra/clangd/FindSymbols.cpp | 86 ++++++++++++------------ clang-tools-extra/clangd/FindSymbols.h | 5 ++ clang-tools-extra/clangd/Protocol.h | 8 ++- clang-tools-extra/clangd/XRefs.cpp | 1 + 4 files changed, 56 insertions(+), 44 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index e8a6bca17215c..28bee28634aeb 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -30,6 +30,49 @@ namespace clang { namespace clangd { +std::vector getSymbolTags(const NamedDecl &ND) { + std::vector Tags; + + if (ND.isDeprecated()) + Tags.push_back(SymbolTag::Deprecated); + + if (isConst(&ND)) + Tags.push_back(SymbolTag::ReadOnly); + + if (isStatic(&ND)) + Tags.push_back(SymbolTag::Static); + + if (isVirtual(&ND)) + Tags.push_back(SymbolTag::Virtual); + + if (isAbstract(&ND)) + Tags.push_back(SymbolTag::Abstract); + + if (isFinal(&ND)) + Tags.push_back(SymbolTag::Final); + + if (isUniqueDefinition(&ND)) + Tags.push_back(SymbolTag::Definition); + else if (!isa(ND)) + Tags.push_back(SymbolTag::Declaration); + + switch (ND.getAccess()) { + case AS_public: + Tags.push_back(SymbolTag::Public); + break; + case AS_protected: + Tags.push_back(SymbolTag::Protected); + break; + case AS_private: + Tags.push_back(SymbolTag::Private); + break; + default: + break; + } + + return Tags; +} + namespace { using ScoredSymbolInfo = std::pair; struct ScoredSymbolGreater { @@ -188,49 +231,6 @@ std::string getSymbolName(ASTContext &Ctx, const NamedDecl &ND) { return printName(Ctx, ND); } -std::vector getSymbolTags(const NamedDecl &ND) { - std::vector Tags; - - if (ND.isDeprecated()) - Tags.push_back(SymbolTag::Deprecated); - - if (isConst(&ND)) - Tags.push_back(SymbolTag::ReadOnly); - - if (isStatic(&ND)) - Tags.push_back(SymbolTag::Static); - - if (isVirtual(&ND)) - Tags.push_back(SymbolTag::Virtual); - - if (isAbstract(&ND)) - Tags.push_back(SymbolTag::Abstract); - - if (isFinal(&ND)) - Tags.push_back(SymbolTag::Final); - - if (isUniqueDefinition(&ND)) - Tags.push_back(SymbolTag::Definition); - else if (!isa(ND)) - Tags.push_back(SymbolTag::Declaration); - - switch (ND.getAccess()) { - case AS_public: - Tags.push_back(SymbolTag::Public); - break; - case AS_protected: - Tags.push_back(SymbolTag::Protected); - break; - case AS_private: - Tags.push_back(SymbolTag::Private); - break; - default: - break; - } - - return Tags; -} - std::string getSymbolDetail(ASTContext &Ctx, const NamedDecl &ND) { PrintingPolicy P(Ctx.getPrintingPolicy()); P.SuppressScope = true; diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 5fb116b13d113..075bf09f1a75b 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -15,6 +15,7 @@ #include "Protocol.h" #include "index/Symbol.h" #include "llvm/ADT/StringRef.h" +#include "clang/AST/Decl.h" namespace clang { namespace clangd { @@ -47,6 +48,10 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, /// same order that they appear. llvm::Expected> getDocumentSymbols(ParsedAST &AST); +/// Returns the symbol tags for the given declaration. +/// \p ND The declaration to get tags for. +std::vector getSymbolTags(const NamedDecl &ND); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 43e68dd38c57d..b8d18ce9eee79 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1144,7 +1144,7 @@ struct DocumentSymbol { /// Indicates if this symbol is deprecated. bool deprecated = false; - /// Tags for this symbol, e.g public, private, static, const etc. + /// The tags for this symbol. std::vector tags; /// The range enclosing this symbol not including leading/trailing whitespace @@ -1172,6 +1172,9 @@ struct SymbolInformation { /// The kind of this symbol. SymbolKind kind; + /// Tags for this symbol, e.g public, private, static, const etc. + std::vector tags; + /// The location of this symbol. Location location; @@ -1539,6 +1542,9 @@ struct TypeHierarchyItem { /// The kind of this item. SymbolKind kind; + /// The symbol tags for this item. + std::vector tags; + /// More detail for this item, e.g. the signature of a function. std::optional detail; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index ef45acf501612..ced5f5b21be81 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1781,6 +1781,7 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { HI.name = printName(Ctx, ND); // FIXME: Populate HI.detail the way we do in symbolToHierarchyItem? HI.kind = SK; + HI.tags = getSymbolTags(ND); HI.range = Range{sourceLocToPosition(SM, DeclRange->getBegin()), sourceLocToPosition(SM, DeclRange->getEnd())}; HI.selectionRange = Range{NameBegin, NameEnd}; From 317eb1d3c0d685c5b46b6b477dc9e74ef7f8d79e Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Fri, 28 Nov 2025 09:06:40 +0100 Subject: [PATCH 11/26] Fix call-hierarchy.test Added tags node in the expected JSON output. --- clang-tools-extra/clangd/test/call-hierarchy.test | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang-tools-extra/clangd/test/call-hierarchy.test b/clang-tools-extra/clangd/test/call-hierarchy.test index 6548ea0068a8d..a25731f03b4c5 100644 --- a/clang-tools-extra/clangd/test/call-hierarchy.test +++ b/clang-tools-extra/clangd/test/call-hierarchy.test @@ -31,6 +31,9 @@ # CHECK-NEXT: "line": 0 # CHECK-NEXT: } # CHECK-NEXT: }, +# CHECK-NEXT: "tags": [ +# CHECK-NEXT: 18 +# CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" # CHECK-NEXT: } --- From 4117ba177cd26042552a1e63355592c95163692e Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Fri, 9 Jan 2026 10:30:46 +0100 Subject: [PATCH 12/26] Reduced AST API, added unit-test introduced symbol-tag bitmask. --- clang-tools-extra/clangd/AST.cpp | 137 ++++++------------ clang-tools-extra/clangd/AST.h | 45 +----- clang-tools-extra/clangd/FindSymbols.cpp | 35 ++--- .../clangd/SemanticHighlighting.cpp | 96 +++++++++++- .../clangd/SemanticHighlighting.h | 45 ++++++ .../clangd/unittests/FindSymbolsTests.cpp | 24 +++ 6 files changed, 229 insertions(+), 153 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index c1bc1651c1370..412b0a21b63a9 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -8,6 +8,7 @@ #include "AST.h" +#include "SemanticHighlighting.h" #include "SourceCode.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTTypeTraits.h" @@ -193,100 +194,6 @@ bool isImplementationDetail(const Decl *D) { D->getASTContext().getSourceManager()); } -// Whether T is const in a loose sense - is a variable with this type readonly? -bool isConst(QualType T) { - if (T.isNull()) - return false; - T = T.getNonReferenceType(); - if (T.isConstQualified()) - return true; - if (const auto *AT = T->getAsArrayTypeUnsafe()) - return isConst(AT->getElementType()); - if (isConst(T->getPointeeType())) - return true; - return false; -} - -bool isConst(const Decl *D) { - if (llvm::isa(D) || llvm::isa(D)) - return true; - if (llvm::isa(D) || llvm::isa(D) || - llvm::isa(D) || llvm::isa(D)) { - if (isConst(llvm::cast(D)->getType())) - return true; - } - if (const auto *OCPD = llvm::dyn_cast(D)) { - if (OCPD->isReadOnly()) - return true; - } - if (const auto *MPD = llvm::dyn_cast(D)) { - if (!MPD->hasSetter()) - return true; - } - if (const auto *CMD = llvm::dyn_cast(D)) { - if (CMD->isConst()) - return true; - } - if (const auto *FD = llvm::dyn_cast(D)) - return isConst(FD->getReturnType()); - return false; -} - -bool isStatic(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isStatic(); - if (const VarDecl *VD = llvm::dyn_cast(D)) - return VD->isStaticDataMember() || VD->isStaticLocal(); - if (const auto *OPD = llvm::dyn_cast(D)) - return OPD->isClassProperty(); - if (const auto *OMD = llvm::dyn_cast(D)) - return OMD->isClassMethod(); - if (const auto *FD = llvm::dyn_cast(D)) - return FD->isStatic(); - return false; -} - -bool isAbstract(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isPureVirtual(); - if (const auto *CRD = llvm::dyn_cast(D)) - return CRD->hasDefinition() && CRD->isAbstract(); - return false; -} - -bool isVirtual(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isVirtual(); - return false; -} - -bool isFinal(const Decl *D) { - if (const auto *CRD = dyn_cast(D)) - return CRD->hasAttr(); - - if (const auto *CRD = dyn_cast(D)) - return CRD->hasAttr(); - - return false; -} - -bool isUniqueDefinition(const NamedDecl *Decl) { - if (auto *Func = dyn_cast(Decl)) - return Func->isThisDeclarationADefinition(); - if (auto *Klass = dyn_cast(Decl)) - return Klass->isThisDeclarationADefinition(); - if (auto *Iface = dyn_cast(Decl)) - return Iface->isThisDeclarationADefinition(); - if (auto *Proto = dyn_cast(Decl)) - return Proto->isThisDeclarationADefinition(); - if (auto *Var = dyn_cast(Decl)) - return Var->isThisDeclarationADefinition(); - return isa(Decl) || - isa(Decl) || - isa(Decl) || isa(Decl) || - isa(Decl); -} - SourceLocation nameLocation(const clang::Decl &D, const SourceManager &SM) { auto L = D.getLocation(); // For `- (void)foo` we want `foo` not the `-`. @@ -1208,5 +1115,47 @@ searchConstructorsInForwardingFunction(const FunctionDecl *FD) { return Result; } +SymbolTags computeSymbolTags(const NamedDecl &ND) { + SymbolTags result = 0; + + if (ND.isDeprecated()) + result |= 1 << static_cast(SymbolTag::Deprecated); + + if (isConst(&ND)) + result |= 1 << static_cast(SymbolTag::ReadOnly); + + if (isStatic(&ND)) + result |= 1 << static_cast(SymbolTag::Static); + + if (isVirtual(&ND)) + result |= 1 << static_cast(SymbolTag::Virtual); + + if (isAbstract(&ND)) + result |= 1 << static_cast(SymbolTag::Abstract); + + if (isFinal(&ND)) + result |= 1 << static_cast(SymbolTag::Final); + + if (isUniqueDefinition(&ND)) + result |= 1 << static_cast(SymbolTag::Definition); + else if (!isa(ND)) + result |= 1 << static_cast(SymbolTag::Declaration); + + switch (ND.getAccess()) { + case AS_public: + result |= 1 << static_cast(SymbolTag::Public); + break; + case AS_protected: + result |= 1 << static_cast(SymbolTag::Protected); + break; + case AS_private: + result |= 1 << static_cast(SymbolTag::Private); + break; + default: + break; + } + + return result; +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 90e95c85085b0..45ff42c3391ac 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -34,6 +34,9 @@ class HeuristicResolver; namespace clangd { +/// A bitmask type representing symbol tags. +using SymbolTags = uint32_t; + /// Returns true if the declaration is considered implementation detail based on /// heuristics. For example, a declaration whose name is not explicitly spelled /// in code is considered implementation detail. @@ -153,45 +156,6 @@ bool isImplicitTemplateInstantiation(const NamedDecl *D); /// explicit specialization. bool isExplicitTemplateSpecialization(const NamedDecl *D); -// Whether T is const in a loose sense - is a variable with this type readonly? -bool isConst(QualType T); - -// Whether D is const in a loose sense (should it be highlighted as such?) -// FIXME: This is separate from whether *a particular usage* can mutate D. -// We may want V in V.size() to be readonly even if V is mutable. -bool isConst(const Decl *D); - -// "Static" means many things in C++, only some get the "static" modifier. -// -// Meanings that do: -// - Members associated with the class rather than the instance. -// This is what 'static' most often means across languages. -// - static local variables -// These are similarly "detached from their context" by the static keyword. -// In practice, these are rarely used inside classes, reducing confusion. -// -// Meanings that don't: -// - Namespace-scoped variables, which have static storage class. -// This is implicit, so the keyword "static" isn't so strongly associated. -// If we want a modifier for these, "global scope" is probably the concept. -// - Namespace-scoped variables/functions explicitly marked "static". -// There the keyword changes *linkage* , which is a totally different concept. -// If we want to model this, "file scope" would be a nice modifier. -// -// This is confusing, and maybe we should use another name, but because "static" -// is a standard LSP modifier, having one with that name has advantages. -bool isStatic(const Decl *D); -// Indicates whether declaration D is abstract in cases where D is a struct or a -// class. -bool isAbstract(const Decl *D); -// Indicates whether declaration D is virtual in cases where D is a method. -bool isVirtual(const Decl *D); -// Indicates whether declaration D is final in cases where D is a struct, class -// or method. -bool isFinal(const Decl *D); -// Indicates whether declaration D is a unique definition (as opposed to a -// declaration). -bool isUniqueDefinition(const NamedDecl *Decl); /// Returns a nested name specifier loc of \p ND if it was present in the /// source, e.g. /// void ns::something::foo() -> returns 'ns::something' @@ -300,6 +264,9 @@ bool isLikelyForwardingFunction(const FunctionTemplateDecl *FT); SmallVector searchConstructorsInForwardingFunction(const FunctionDecl *FD); +/// Computes symbol tags for a given NamedDecl. +SymbolTags computeSymbolTags(const NamedDecl &ND); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 28bee28634aeb..7412a58a16fdd 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -31,44 +31,41 @@ namespace clang { namespace clangd { std::vector getSymbolTags(const NamedDecl &ND) { + const auto SymbolTags = computeSymbolTags(ND); std::vector Tags; - if (ND.isDeprecated()) + if (SymbolTags & 1 << static_cast(SymbolTag::Deprecated)) Tags.push_back(SymbolTag::Deprecated); - if (isConst(&ND)) + if (SymbolTags & 1 << static_cast(SymbolTag::ReadOnly)) Tags.push_back(SymbolTag::ReadOnly); - if (isStatic(&ND)) + if (SymbolTags & 1 << static_cast(SymbolTag::Static)) Tags.push_back(SymbolTag::Static); - if (isVirtual(&ND)) + if (SymbolTags & 1 << static_cast(SymbolTag::Virtual)) Tags.push_back(SymbolTag::Virtual); - if (isAbstract(&ND)) + if (SymbolTags & 1 << static_cast(SymbolTag::Abstract)) Tags.push_back(SymbolTag::Abstract); - if (isFinal(&ND)) + if (SymbolTags & 1 << static_cast(SymbolTag::Final)) Tags.push_back(SymbolTag::Final); - if (isUniqueDefinition(&ND)) + if (SymbolTags & 1 << static_cast(SymbolTag::Definition)) Tags.push_back(SymbolTag::Definition); - else if (!isa(ND)) + + if (SymbolTags & 1 << static_cast(SymbolTag::Declaration)) Tags.push_back(SymbolTag::Declaration); - switch (ND.getAccess()) { - case AS_public: - Tags.push_back(SymbolTag::Public); - break; - case AS_protected: + if (SymbolTags & 1 << static_cast(SymbolTag::Public)) + Tags.push_back(SymbolTag::Public); + + if (SymbolTags & 1 << static_cast(SymbolTag::Protected)) Tags.push_back(SymbolTag::Protected); - break; - case AS_private: + + if (SymbolTags & 1 << static_cast(SymbolTag::Private)) Tags.push_back(SymbolTag::Private); - break; - default: - break; - } return Tags; } diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index f53343951d7de..44fe71fb3e28e 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "SemanticHighlighting.h" -#include "AST.h" #include "Config.h" #include "FindTarget.h" #include "ParsedAST.h" @@ -1461,5 +1460,100 @@ std::vector getInactiveRegions(ParsedAST &AST) { return InactiveRegions; } + +// Whether T is const in a loose sense - is a variable with this type readonly? +bool isConst(QualType T) { + if (T.isNull()) + return false; + T = T.getNonReferenceType(); + if (T.isConstQualified()) + return true; + if (const auto *AT = T->getAsArrayTypeUnsafe()) + return isConst(AT->getElementType()); + if (isConst(T->getPointeeType())) + return true; + return false; +} + +bool isConst(const Decl *D) { + if (llvm::isa(D) || llvm::isa(D)) + return true; + if (llvm::isa(D) || llvm::isa(D) || + llvm::isa(D) || llvm::isa(D)) { + if (isConst(llvm::cast(D)->getType())) + return true; + } + if (const auto *OCPD = llvm::dyn_cast(D)) { + if (OCPD->isReadOnly()) + return true; + } + if (const auto *MPD = llvm::dyn_cast(D)) { + if (!MPD->hasSetter()) + return true; + } + if (const auto *CMD = llvm::dyn_cast(D)) { + if (CMD->isConst()) + return true; + } + if (const auto *FD = llvm::dyn_cast(D)) + return isConst(FD->getReturnType()); + return false; +} + +bool isStatic(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isStatic(); + if (const VarDecl *VD = llvm::dyn_cast(D)) + return VD->isStaticDataMember() || VD->isStaticLocal(); + if (const auto *OPD = llvm::dyn_cast(D)) + return OPD->isClassProperty(); + if (const auto *OMD = llvm::dyn_cast(D)) + return OMD->isClassMethod(); + if (const auto *FD = llvm::dyn_cast(D)) + return FD->isStatic(); + return false; +} + +bool isAbstract(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isPureVirtual(); + if (const auto *CRD = llvm::dyn_cast(D)) + return CRD->hasDefinition() && CRD->isAbstract(); + return false; +} + +bool isVirtual(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isVirtual(); + return false; +} + +bool isFinal(const Decl *D) { + if (const auto *CRD = dyn_cast(D)) + return CRD->hasAttr(); + + if (const auto *CRD = dyn_cast(D)) + return CRD->hasAttr(); + + return false; +} + +bool isUniqueDefinition(const NamedDecl *Decl) { + if (auto *Func = dyn_cast(Decl)) + return Func->isThisDeclarationADefinition(); + if (auto *Klass = dyn_cast(Decl)) + return Klass->isThisDeclarationADefinition(); + if (auto *Iface = dyn_cast(Decl)) + return Iface->isThisDeclarationADefinition(); + if (auto *Proto = dyn_cast(Decl)) + return Proto->isThisDeclarationADefinition(); + if (auto *Var = dyn_cast(Decl)) + return Var->isThisDeclarationADefinition(); + return isa(Decl) || + isa(Decl) || + isa(Decl) || isa(Decl) || + isa(Decl); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h index 59d742b83ee52..d4bae8bab8836 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -21,10 +21,14 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICHIGHLIGHTING_H #include "Protocol.h" + +#include "clang/AST/TypeBase.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/raw_ostream.h" namespace clang { +class NamedDecl; + namespace clangd { class ParsedAST; @@ -126,6 +130,47 @@ std::vector diffTokens(llvm::ArrayRef Before, // are not included. std::vector getInactiveRegions(ParsedAST &AST); + +// Whether T is const in a loose sense - is a variable with this type readonly? +bool isConst(QualType T); + +// Whether D is const in a loose sense (should it be highlighted as such?) +// FIXME: This is separate from whether *a particular usage* can mutate D. +// We may want V in V.size() to be readonly even if V is mutable. +bool isConst(const Decl *D); + +// "Static" means many things in C++, only some get the "static" modifier. +// +// Meanings that do: +// - Members associated with the class rather than the instance. +// This is what 'static' most often means across languages. +// - static local variables +// These are similarly "detached from their context" by the static keyword. +// In practice, these are rarely used inside classes, reducing confusion. +// +// Meanings that don't: +// - Namespace-scoped variables, which have static storage class. +// This is implicit, so the keyword "static" isn't so strongly associated. +// If we want a modifier for these, "global scope" is probably the concept. +// - Namespace-scoped variables/functions explicitly marked "static". +// There the keyword changes *linkage* , which is a totally different concept. +// If we want to model this, "file scope" would be a nice modifier. +// +// This is confusing, and maybe we should use another name, but because "static" +// is a standard LSP modifier, having one with that name has advantages. +bool isStatic(const Decl *D); +// Indicates whether declaration D is abstract in cases where D is a struct or a +// class. +bool isAbstract(const Decl *D); +// Indicates whether declaration D is virtual in cases where D is a method. +bool isVirtual(const Decl *D); +// Indicates whether declaration D is final in cases where D is a struct, class +// or method. +bool isFinal(const Decl *D); +// Indicates whether declaration D is a unique definition (as opposed to a +// declaration). +bool isUniqueDefinition(const NamedDecl *Decl); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index 5b1630eb00cb1..cd9e4321aa56e 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -35,6 +35,8 @@ MATCHER_P(withName, N, "") { return arg.name == N; } MATCHER_P(withKind, Kind, "") { return arg.kind == Kind; } MATCHER_P(withDetail, Detail, "") { return arg.detail == Detail; } MATCHER_P(symRange, Range, "") { return arg.range == Range; } +MATCHER_P(withSymbolTag, Tag, "") { return llvm::is_contained(arg.tags, Tag); } + // GMock helpers for matching DocumentSymbol. MATCHER_P(symNameRange, Range, "") { return arg.selectionRange == Range; } @@ -1131,6 +1133,28 @@ TEST(DocumentSymbolsTest, PragmaMarkGroupsNoNesting) { withName("Core"), withName("coreMethod"))); } +TEST(DocumentSymbolsTest, SymbolTags) { + TestTU TU; + Annotations Main(R"( + class A { + virtual void f() const = 0; + }; + )"); + + TU.Code = Main.code().str(); + auto Symbols = getSymbols(TU.build()); + EXPECT_THAT( + Symbols, + ElementsAre(AllOf( + withName("A"), withSymbolTag(SymbolTag::Abstract), + withSymbolTag(SymbolTag::Definition), + children(AllOf(withName("f"), withSymbolTag(SymbolTag::ReadOnly), + withSymbolTag(SymbolTag::Virtual), + withSymbolTag(SymbolTag::Abstract), + withSymbolTag(SymbolTag::Declaration), + withSymbolTag(SymbolTag::Private)))))); +} + } // namespace } // namespace clangd } // namespace clang From e1810827de5f70e486fb8b3d7605d48c0482fe89 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 12 Jan 2026 09:49:33 +0100 Subject: [PATCH 13/26] Use parts of result of the function computeSymbolTags in SemanticHighlighting. --- clang-tools-extra/clangd/AST.cpp | 29 ++++++++++++------- clang-tools-extra/clangd/FindSymbols.cpp | 28 +++++++++++------- .../clangd/SemanticHighlighting.cpp | 29 ++++++++++++++----- 3 files changed, 57 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 412b0a21b63a9..6d8e919e9536e 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -618,6 +618,10 @@ class DeducedTypeVisitor : public RecursiveASTVisitor { QualType DeducedType; }; + +SymbolTags toSymbolTagBitmask(const SymbolTag ST) { + return (1 << static_cast(ST)); +} } // namespace std::optional getDeducedType(ASTContext &ASTCtx, @@ -1119,37 +1123,40 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) { SymbolTags result = 0; if (ND.isDeprecated()) - result |= 1 << static_cast(SymbolTag::Deprecated); + result |= toSymbolTagBitmask(SymbolTag::Deprecated); if (isConst(&ND)) - result |= 1 << static_cast(SymbolTag::ReadOnly); + result |= toSymbolTagBitmask(SymbolTag::ReadOnly); if (isStatic(&ND)) - result |= 1 << static_cast(SymbolTag::Static); + result |= toSymbolTagBitmask(SymbolTag::Static); if (isVirtual(&ND)) - result |= 1 << static_cast(SymbolTag::Virtual); + result |= toSymbolTagBitmask(SymbolTag::Virtual); if (isAbstract(&ND)) - result |= 1 << static_cast(SymbolTag::Abstract); + result |= toSymbolTagBitmask(SymbolTag::Abstract); if (isFinal(&ND)) - result |= 1 << static_cast(SymbolTag::Final); + result |= toSymbolTagBitmask(SymbolTag::Final); + // Do not treat an UnresolvedUsingValueDecl as a declaration. + // It's more common to think of it as a reference to the + // underlying declaration. if (isUniqueDefinition(&ND)) - result |= 1 << static_cast(SymbolTag::Definition); + result |= toSymbolTagBitmask(SymbolTag::Definition); else if (!isa(ND)) - result |= 1 << static_cast(SymbolTag::Declaration); + result |= toSymbolTagBitmask(SymbolTag::Declaration); switch (ND.getAccess()) { case AS_public: - result |= 1 << static_cast(SymbolTag::Public); + result |= toSymbolTagBitmask(SymbolTag::Public); break; case AS_protected: - result |= 1 << static_cast(SymbolTag::Protected); + result |= toSymbolTagBitmask(SymbolTag::Protected); break; case AS_private: - result |= 1 << static_cast(SymbolTag::Private); + result |= toSymbolTagBitmask(SymbolTag::Private); break; default: break; diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 7412a58a16fdd..077accb7d2ee7 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -30,41 +30,47 @@ namespace clang { namespace clangd { +namespace { +SymbolTags toSymbolTagBitmask(const SymbolTag ST) { + return (1 << static_cast(ST)); +} +} + std::vector getSymbolTags(const NamedDecl &ND) { const auto SymbolTags = computeSymbolTags(ND); std::vector Tags; - if (SymbolTags & 1 << static_cast(SymbolTag::Deprecated)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) Tags.push_back(SymbolTag::Deprecated); - if (SymbolTags & 1 << static_cast(SymbolTag::ReadOnly)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::ReadOnly)) Tags.push_back(SymbolTag::ReadOnly); - if (SymbolTags & 1 << static_cast(SymbolTag::Static)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Static)) Tags.push_back(SymbolTag::Static); - if (SymbolTags & 1 << static_cast(SymbolTag::Virtual)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Virtual)) Tags.push_back(SymbolTag::Virtual); - if (SymbolTags & 1 << static_cast(SymbolTag::Abstract)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Abstract)) Tags.push_back(SymbolTag::Abstract); - if (SymbolTags & 1 << static_cast(SymbolTag::Final)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Final)) Tags.push_back(SymbolTag::Final); - if (SymbolTags & 1 << static_cast(SymbolTag::Definition)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition)) Tags.push_back(SymbolTag::Definition); - if (SymbolTags & 1 << static_cast(SymbolTag::Declaration)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration)) Tags.push_back(SymbolTag::Declaration); - if (SymbolTags & 1 << static_cast(SymbolTag::Public)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Public)) Tags.push_back(SymbolTag::Public); - if (SymbolTags & 1 << static_cast(SymbolTag::Protected)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Protected)) Tags.push_back(SymbolTag::Protected); - if (SymbolTags & 1 << static_cast(SymbolTag::Private)) + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Private)) Tags.push_back(SymbolTag::Private); return Tags; diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 44fe71fb3e28e..0a5f81fe3b029 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -32,6 +32,8 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" + +#include #include #include @@ -1024,6 +1026,10 @@ class CollectExtraHighlightings private: HighlightingsBuilder &H; }; + +SymbolTags toSymbolTagBitmask(const SymbolTag ST) { + return (1 << static_cast(ST)); +} } // namespace std::vector @@ -1055,22 +1061,31 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { } if (auto Mod = scopeModifier(Decl)) Tok.addModifier(*Mod); - if (isConst(Decl)) + + const auto SymbolTags = computeSymbolTags(*Decl); + + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) + Tok.addModifier(HighlightingModifier::Deprecated); + + if (SymbolTags & toSymbolTagBitmask(SymbolTag::ReadOnly)) Tok.addModifier(HighlightingModifier::Readonly); - if (isStatic(Decl)) + + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Static)) Tok.addModifier(HighlightingModifier::Static); - if (isAbstract(Decl)) - Tok.addModifier(HighlightingModifier::Abstract); - if (isVirtual(Decl)) + + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Virtual)) Tok.addModifier(HighlightingModifier::Virtual); + + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Abstract)) + Tok.addModifier(HighlightingModifier::Abstract); + if (isDependent(Decl)) Tok.addModifier(HighlightingModifier::DependentName); if (isDefaultLibrary(Decl)) Tok.addModifier(HighlightingModifier::DefaultLibrary); - if (Decl->isDeprecated()) - Tok.addModifier(HighlightingModifier::Deprecated); if (isa(Decl)) Tok.addModifier(HighlightingModifier::ConstructorOrDestructor); + if (R.IsDecl) { // Do not treat an UnresolvedUsingValueDecl as a declaration. // It's more common to think of it as a reference to the From 6f0deffa9b9c1dbe983928e2a1e18ebe3ba5ee49 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 14 Jan 2026 09:07:51 +0100 Subject: [PATCH 14/26] Overload computeSymbolTags to encapsulate the proper determination of Declarations and Definitions. --- clang-tools-extra/clangd/AST.cpp | 45 +++++++++++-------- clang-tools-extra/clangd/AST.h | 4 ++ clang-tools-extra/clangd/FindSymbols.cpp | 6 +-- .../clangd/SemanticHighlighting.cpp | 19 ++++---- .../clangd/test/symbol-tags.test | 9 +++- clang-tools-extra/clangd/test/symbols.test | 3 +- 6 files changed, 53 insertions(+), 33 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 6d8e919e9536e..7c7b1c0f437c9 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1119,50 +1119,59 @@ searchConstructorsInForwardingFunction(const FunctionDecl *FD) { return Result; } +// Backwards-compatible default behavior: determine whether this NamedDecl is +// definition based on `isUniqueDefinition`, assuming that ND is a declaration. SymbolTags computeSymbolTags(const NamedDecl &ND) { - SymbolTags result = 0; + const auto IsDef = isUniqueDefinition(&ND); + return computeSymbolTags(ND, true, IsDef); +} + +SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl, bool IsDef) { + SymbolTags Result = 0; if (ND.isDeprecated()) - result |= toSymbolTagBitmask(SymbolTag::Deprecated); + Result |= toSymbolTagBitmask(SymbolTag::Deprecated); if (isConst(&ND)) - result |= toSymbolTagBitmask(SymbolTag::ReadOnly); + Result |= toSymbolTagBitmask(SymbolTag::ReadOnly); if (isStatic(&ND)) - result |= toSymbolTagBitmask(SymbolTag::Static); + Result |= toSymbolTagBitmask(SymbolTag::Static); if (isVirtual(&ND)) - result |= toSymbolTagBitmask(SymbolTag::Virtual); + Result |= toSymbolTagBitmask(SymbolTag::Virtual); if (isAbstract(&ND)) - result |= toSymbolTagBitmask(SymbolTag::Abstract); + Result |= toSymbolTagBitmask(SymbolTag::Abstract); if (isFinal(&ND)) - result |= toSymbolTagBitmask(SymbolTag::Final); + Result |= toSymbolTagBitmask(SymbolTag::Final); - // Do not treat an UnresolvedUsingValueDecl as a declaration. - // It's more common to think of it as a reference to the - // underlying declaration. - if (isUniqueDefinition(&ND)) - result |= toSymbolTagBitmask(SymbolTag::Definition); - else if (!isa(ND)) - result |= toSymbolTagBitmask(SymbolTag::Declaration); + if (IsDecl && not isa(ND)) { + // Do not treat an UnresolvedUsingValueDecl as a declaration. + // It's more common to think of it as a reference to the + // underlying declaration. + Result |= toSymbolTagBitmask(SymbolTag::Declaration); + + if (IsDef) + Result |= toSymbolTagBitmask(SymbolTag::Definition); + } switch (ND.getAccess()) { case AS_public: - result |= toSymbolTagBitmask(SymbolTag::Public); + Result |= toSymbolTagBitmask(SymbolTag::Public); break; case AS_protected: - result |= toSymbolTagBitmask(SymbolTag::Protected); + Result |= toSymbolTagBitmask(SymbolTag::Protected); break; case AS_private: - result |= toSymbolTagBitmask(SymbolTag::Private); + Result |= toSymbolTagBitmask(SymbolTag::Private); break; default: break; } - return result; + return Result; } } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 45ff42c3391ac..53270126cf348 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -267,6 +267,10 @@ searchConstructorsInForwardingFunction(const FunctionDecl *FD); /// Computes symbol tags for a given NamedDecl. SymbolTags computeSymbolTags(const NamedDecl &ND); +/// Computes symbol tags for a given NamedDecl, with additional context about +/// whether it's a declaration or definition. +SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl, bool IsDef); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 077accb7d2ee7..693f3f59116cf 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -34,7 +34,7 @@ namespace { SymbolTags toSymbolTagBitmask(const SymbolTag ST) { return (1 << static_cast(ST)); } -} +} // namespace std::vector getSymbolTags(const NamedDecl &ND) { const auto SymbolTags = computeSymbolTags(ND); @@ -64,8 +64,8 @@ std::vector getSymbolTags(const NamedDecl &ND) { if (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration)) Tags.push_back(SymbolTag::Declaration); - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Public)) - Tags.push_back(SymbolTag::Public); + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Public)) + Tags.push_back(SymbolTag::Public); if (SymbolTags & toSymbolTagBitmask(SymbolTag::Protected)) Tags.push_back(SymbolTag::Protected); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 0a5f81fe3b029..4865f44993f61 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -10,6 +10,7 @@ #include "Config.h" #include "FindTarget.h" #include "ParsedAST.h" +#include "AST.h" #include "Protocol.h" #include "SourceCode.h" #include "support/Logger.h" @@ -1062,7 +1063,7 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { if (auto Mod = scopeModifier(Decl)) Tok.addModifier(*Mod); - const auto SymbolTags = computeSymbolTags(*Decl); + const auto SymbolTags = computeSymbolTags(*Decl, R.IsDecl, isUniqueDefinition(Decl)); if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) Tok.addModifier(HighlightingModifier::Deprecated); @@ -1081,20 +1082,18 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { if (isDependent(Decl)) Tok.addModifier(HighlightingModifier::DependentName); + if (isDefaultLibrary(Decl)) Tok.addModifier(HighlightingModifier::DefaultLibrary); + if (isa(Decl)) Tok.addModifier(HighlightingModifier::ConstructorOrDestructor); - if (R.IsDecl) { - // Do not treat an UnresolvedUsingValueDecl as a declaration. - // It's more common to think of it as a reference to the - // underlying declaration. - if (!isa(Decl)) - Tok.addModifier(HighlightingModifier::Declaration); - if (isUniqueDefinition(Decl)) - Tok.addModifier(HighlightingModifier::Definition); - } + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration)) + Tok.addModifier(HighlightingModifier::Declaration); + + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition)) + Tok.addModifier(HighlightingModifier::Definition); } }, AST.getHeuristicResolver()); diff --git a/clang-tools-extra/clangd/test/symbol-tags.test b/clang-tools-extra/clangd/test/symbol-tags.test index 3d4821ae32081..b59c5181cce9b 100644 --- a/clang-tools-extra/clangd/test/symbol-tags.test +++ b/clang-tools-extra/clangd/test/symbol-tags.test @@ -40,6 +40,7 @@ # CHECK-NEXT: "tags": [ # CHECK-NEXT: 15, # CHECK-NEXT: 19, +# CHECK-NEXT: 18, # CHECK-NEXT: 5 # CHECK-NEXT: ] # CHECK-NEXT: }, @@ -130,6 +131,7 @@ # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ # CHECK-NEXT: 19, +# CHECK-NEXT: 18, # CHECK-NEXT: 4 # CHECK-NEXT: ] # CHECK-NEXT: }, @@ -160,6 +162,7 @@ # CHECK-NEXT: "tags": [ # CHECK-NEXT: 8, # CHECK-NEXT: 19, +# CHECK-NEXT: 18, # CHECK-NEXT: 2 # CHECK-NEXT: ] # CHECK-NEXT: } @@ -189,7 +192,8 @@ # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ # CHECK-NEXT: 9, -# CHECK-NEXT: 19 +# CHECK-NEXT: 19, +# CHECK-NEXT: 18 # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { @@ -219,6 +223,7 @@ # CHECK-NEXT: "tags": [ # CHECK-NEXT: 20, # CHECK-NEXT: 19, +# CHECK-NEXT: 18, # CHECK-NEXT: 5 # CHECK-NEXT: ] # CHECK-NEXT: }, @@ -252,6 +257,7 @@ # CHECK-NEXT: 15, # CHECK-NEXT: 10, # CHECK-NEXT: 19, +# CHECK-NEXT: 18, # CHECK-NEXT: 5 # CHECK-NEXT: ] # CHECK-NEXT: } @@ -282,6 +288,7 @@ # CHECK-NEXT: "tags": [ # CHECK-NEXT: 10, # CHECK-NEXT: 19 +# CHECK-NEXT: 18 # CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ] diff --git a/clang-tools-extra/clangd/test/symbols.test b/clang-tools-extra/clangd/test/symbols.test index 40115f2496e50..4d818d9d1dcf4 100644 --- a/clang-tools-extra/clangd/test/symbols.test +++ b/clang-tools-extra/clangd/test/symbols.test @@ -86,7 +86,8 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ -# CHECK-NEXT: 19 +# CHECK-NEXT: 19, +# CHECK-NEXT: 18 # CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ] From ea5fd1c8887428058b07f76df3dcf952293cfa2e Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 14 Jan 2026 12:26:14 +0100 Subject: [PATCH 15/26] Relocated computeSymbolTags to the module FindSymbols. Refactoring on getSymbolTags and computeSymbolTags. Improved SymbolTags test. --- clang-tools-extra/clangd/AST.cpp | 57 ----- clang-tools-extra/clangd/AST.h | 10 - clang-tools-extra/clangd/FindSymbols.cpp | 209 +++++++++++++++--- clang-tools-extra/clangd/FindSymbols.h | 11 + .../clangd/SemanticHighlighting.cpp | 97 +------- .../clangd/SemanticHighlighting.h | 37 ---- .../clangd/unittests/FindSymbolsTests.cpp | 22 +- 7 files changed, 210 insertions(+), 233 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 7c7b1c0f437c9..3f96faba18cf3 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -619,9 +619,6 @@ class DeducedTypeVisitor : public RecursiveASTVisitor { QualType DeducedType; }; -SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return (1 << static_cast(ST)); -} } // namespace std::optional getDeducedType(ASTContext &ASTCtx, @@ -1119,59 +1116,5 @@ searchConstructorsInForwardingFunction(const FunctionDecl *FD) { return Result; } -// Backwards-compatible default behavior: determine whether this NamedDecl is -// definition based on `isUniqueDefinition`, assuming that ND is a declaration. -SymbolTags computeSymbolTags(const NamedDecl &ND) { - const auto IsDef = isUniqueDefinition(&ND); - return computeSymbolTags(ND, true, IsDef); -} - -SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl, bool IsDef) { - SymbolTags Result = 0; - - if (ND.isDeprecated()) - Result |= toSymbolTagBitmask(SymbolTag::Deprecated); - - if (isConst(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::ReadOnly); - - if (isStatic(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Static); - - if (isVirtual(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Virtual); - - if (isAbstract(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Abstract); - - if (isFinal(&ND)) - Result |= toSymbolTagBitmask(SymbolTag::Final); - - if (IsDecl && not isa(ND)) { - // Do not treat an UnresolvedUsingValueDecl as a declaration. - // It's more common to think of it as a reference to the - // underlying declaration. - Result |= toSymbolTagBitmask(SymbolTag::Declaration); - - if (IsDef) - Result |= toSymbolTagBitmask(SymbolTag::Definition); - } - - switch (ND.getAccess()) { - case AS_public: - Result |= toSymbolTagBitmask(SymbolTag::Public); - break; - case AS_protected: - Result |= toSymbolTagBitmask(SymbolTag::Protected); - break; - case AS_private: - Result |= toSymbolTagBitmask(SymbolTag::Private); - break; - default: - break; - } - - return Result; -} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 53270126cf348..2bb4943b6de0b 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -34,9 +34,6 @@ class HeuristicResolver; namespace clangd { -/// A bitmask type representing symbol tags. -using SymbolTags = uint32_t; - /// Returns true if the declaration is considered implementation detail based on /// heuristics. For example, a declaration whose name is not explicitly spelled /// in code is considered implementation detail. @@ -264,13 +261,6 @@ bool isLikelyForwardingFunction(const FunctionTemplateDecl *FT); SmallVector searchConstructorsInForwardingFunction(const FunctionDecl *FD); -/// Computes symbol tags for a given NamedDecl. -SymbolTags computeSymbolTags(const NamedDecl &ND); - -/// Computes symbol tags for a given NamedDecl, with additional context about -/// whether it's a declaration or definition. -SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl, bool IsDef); - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 693f3f59116cf..bfb6ca3462dcd 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -36,43 +36,202 @@ SymbolTags toSymbolTagBitmask(const SymbolTag ST) { } } // namespace -std::vector getSymbolTags(const NamedDecl &ND) { - const auto SymbolTags = computeSymbolTags(ND); - std::vector Tags; - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) - Tags.push_back(SymbolTag::Deprecated); - if (SymbolTags & toSymbolTagBitmask(SymbolTag::ReadOnly)) - Tags.push_back(SymbolTag::ReadOnly); +// "Static" means many things in C++, only some get the "static" modifier. +// +// Meanings that do: +// - Members associated with the class rather than the instance. +// This is what 'static' most often means across languages. +// - static local variables +// These are similarly "detached from their context" by the static keyword. +// In practice, these are rarely used inside classes, reducing confusion. +// +// Meanings that don't: +// - Namespace-scoped variables, which have static storage class. +// This is implicit, so the keyword "static" isn't so strongly associated. +// If we want a modifier for these, "global scope" is probably the concept. +// - Namespace-scoped variables/functions explicitly marked "static". +// There the keyword changes *linkage* , which is a totally different concept. +// If we want to model this, "file scope" would be a nice modifier. +// +// This is confusing, and maybe we should use another name, but because "static" +// is a standard LSP modifier, having one with that name has advantages. +bool isStatic(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isStatic(); + if (const VarDecl *VD = llvm::dyn_cast(D)) + return VD->isStaticDataMember() || VD->isStaticLocal(); + if (const auto *OPD = llvm::dyn_cast(D)) + return OPD->isClassProperty(); + if (const auto *OMD = llvm::dyn_cast(D)) + return OMD->isClassMethod(); + if (const auto *FD = llvm::dyn_cast(D)) + return FD->isStatic(); + return false; +} - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Static)) - Tags.push_back(SymbolTag::Static); +// Whether T is const in a loose sense - is a variable with this type readonly? +bool isConst(QualType T) { + if (T.isNull()) + return false; + T = T.getNonReferenceType(); + if (T.isConstQualified()) + return true; + if (const auto *AT = T->getAsArrayTypeUnsafe()) + return isConst(AT->getElementType()); + if (isConst(T->getPointeeType())) + return true; + return false; +} - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Virtual)) - Tags.push_back(SymbolTag::Virtual); +// Whether D is const in a loose sense (should it be highlighted as such?) +// FIXME: This is separate from whether *a particular usage* can mutate D. +// We may want V in V.size() to be readonly even if V is mutable. +bool isConst(const Decl *D) { + if (llvm::isa(D) || llvm::isa(D)) + return true; + if (llvm::isa(D) || llvm::isa(D) || + llvm::isa(D) || llvm::isa(D)) { + if (isConst(llvm::cast(D)->getType())) + return true; + } + if (const auto *OCPD = llvm::dyn_cast(D)) { + if (OCPD->isReadOnly()) + return true; + } + if (const auto *MPD = llvm::dyn_cast(D)) { + if (!MPD->hasSetter()) + return true; + } + if (const auto *CMD = llvm::dyn_cast(D)) { + if (CMD->isConst()) + return true; + } + if (const auto *FD = llvm::dyn_cast(D)) + return isConst(FD->getReturnType()); + return false; +} - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Abstract)) - Tags.push_back(SymbolTag::Abstract); +// Indicates whether declaration D is abstract in cases where D is a struct or a +// class. +bool isAbstract(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isPureVirtual(); + if (const auto *CRD = llvm::dyn_cast(D)) + return CRD->hasDefinition() && CRD->isAbstract(); + return false; +} - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Final)) - Tags.push_back(SymbolTag::Final); +// Indicates whether declaration D is virtual in cases where D is a method. +bool isVirtual(const Decl *D) { + if (const auto *CMD = llvm::dyn_cast(D)) + return CMD->isVirtual(); + return false; +} - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition)) - Tags.push_back(SymbolTag::Definition); +// Indicates whether declaration D is final in cases where D is a struct, class +// or method. +bool isFinal(const Decl *D) { + if (const auto *CRD = dyn_cast(D)) + return CRD->hasAttr(); - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration)) - Tags.push_back(SymbolTag::Declaration); + if (const auto *CRD = dyn_cast(D)) + return CRD->hasAttr(); - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Public)) - Tags.push_back(SymbolTag::Public); + return false; +} - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Protected)) - Tags.push_back(SymbolTag::Protected); - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Private)) - Tags.push_back(SymbolTag::Private); +bool isUniqueDefinition(const NamedDecl *Decl) { + if (auto *Func = dyn_cast(Decl)) + return Func->isThisDeclarationADefinition(); + if (auto *Klass = dyn_cast(Decl)) + return Klass->isThisDeclarationADefinition(); + if (auto *Iface = dyn_cast(Decl)) + return Iface->isThisDeclarationADefinition(); + if (auto *Proto = dyn_cast(Decl)) + return Proto->isThisDeclarationADefinition(); + if (auto *Var = dyn_cast(Decl)) + return Var->isThisDeclarationADefinition(); + return isa(Decl) || + isa(Decl) || + isa(Decl) || isa(Decl) || + isa(Decl); +} + +// Backwards-compatible default behavior: determine whether this NamedDecl is +// definition based on `isUniqueDefinition`, assuming that ND is a declaration. +SymbolTags computeSymbolTags(const NamedDecl &ND) { + return computeSymbolTags(ND, true); +} + +SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl) { + SymbolTags Result = 0; + const auto IsDef = isUniqueDefinition(&ND); + if (ND.isDeprecated()) + Result |= toSymbolTagBitmask(SymbolTag::Deprecated); + + if (isConst(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::ReadOnly); + + if (isStatic(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Static); + + if (isVirtual(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Virtual); + + if (isAbstract(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Abstract); + + if (isFinal(&ND)) + Result |= toSymbolTagBitmask(SymbolTag::Final); + + if (IsDecl && not isa(ND)) { + // Do not treat an UnresolvedUsingValueDecl as a declaration. + // It's more common to think of it as a reference to the + // underlying declaration. + Result |= toSymbolTagBitmask(SymbolTag::Declaration); + + if (IsDef) + Result |= toSymbolTagBitmask(SymbolTag::Definition); + } + + switch (ND.getAccess()) { + case AS_public: + Result |= toSymbolTagBitmask(SymbolTag::Public); + break; + case AS_protected: + Result |= toSymbolTagBitmask(SymbolTag::Protected); + break; + case AS_private: + Result |= toSymbolTagBitmask(SymbolTag::Private); + break; + default: + break; + } + + return Result; +} + +std::vector getSymbolTags(const NamedDecl &ND) { + const auto symbolTags = computeSymbolTags(ND); + std::vector Tags; + + if (symbolTags == 0) + return Tags; + + // Iterate through SymbolTag enum values and collect any that are present in + // the bitmask. SymbolTag values are in the numeric range + // [Deprecated .. ReadOnly]. + constexpr unsigned MinTag = static_cast(SymbolTag::Deprecated); + constexpr unsigned MaxTag = static_cast(SymbolTag::ReadOnly); + for (unsigned I = MinTag; I <= MaxTag; ++I) { + auto ST = static_cast(I); + if (symbolTags & toSymbolTagBitmask(ST)) + Tags.push_back(ST); + } return Tags; } diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 075bf09f1a75b..81683cf13d8a6 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -22,6 +22,9 @@ namespace clangd { class ParsedAST; class SymbolIndex; +/// A bitmask type representing symbol tags. +using SymbolTags = uint32_t; + /// Helper function for deriving an LSP Location from an index SymbolLocation. llvm::Expected indexToLSPLocation(const SymbolLocation &Loc, llvm::StringRef TUPath); @@ -52,6 +55,14 @@ llvm::Expected> getDocumentSymbols(ParsedAST &AST); /// \p ND The declaration to get tags for. std::vector getSymbolTags(const NamedDecl &ND); + +/// Computes symbol tags for a given NamedDecl. +SymbolTags computeSymbolTags(const NamedDecl &ND); + +/// Computes symbol tags for a given NamedDecl, with additional context about +/// whether it's a declaration or definition. +SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 4865f44993f61..54e19bc7146be 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -8,6 +8,7 @@ #include "SemanticHighlighting.h" #include "Config.h" +#include "FindSymbols.h" #include "FindTarget.h" #include "ParsedAST.h" #include "AST.h" @@ -1063,7 +1064,7 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { if (auto Mod = scopeModifier(Decl)) Tok.addModifier(*Mod); - const auto SymbolTags = computeSymbolTags(*Decl, R.IsDecl, isUniqueDefinition(Decl)); + const auto SymbolTags = computeSymbolTags(*Decl, R.IsDecl); if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) Tok.addModifier(HighlightingModifier::Deprecated); @@ -1475,99 +1476,5 @@ std::vector getInactiveRegions(ParsedAST &AST) { } -// Whether T is const in a loose sense - is a variable with this type readonly? -bool isConst(QualType T) { - if (T.isNull()) - return false; - T = T.getNonReferenceType(); - if (T.isConstQualified()) - return true; - if (const auto *AT = T->getAsArrayTypeUnsafe()) - return isConst(AT->getElementType()); - if (isConst(T->getPointeeType())) - return true; - return false; -} - -bool isConst(const Decl *D) { - if (llvm::isa(D) || llvm::isa(D)) - return true; - if (llvm::isa(D) || llvm::isa(D) || - llvm::isa(D) || llvm::isa(D)) { - if (isConst(llvm::cast(D)->getType())) - return true; - } - if (const auto *OCPD = llvm::dyn_cast(D)) { - if (OCPD->isReadOnly()) - return true; - } - if (const auto *MPD = llvm::dyn_cast(D)) { - if (!MPD->hasSetter()) - return true; - } - if (const auto *CMD = llvm::dyn_cast(D)) { - if (CMD->isConst()) - return true; - } - if (const auto *FD = llvm::dyn_cast(D)) - return isConst(FD->getReturnType()); - return false; -} - -bool isStatic(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isStatic(); - if (const VarDecl *VD = llvm::dyn_cast(D)) - return VD->isStaticDataMember() || VD->isStaticLocal(); - if (const auto *OPD = llvm::dyn_cast(D)) - return OPD->isClassProperty(); - if (const auto *OMD = llvm::dyn_cast(D)) - return OMD->isClassMethod(); - if (const auto *FD = llvm::dyn_cast(D)) - return FD->isStatic(); - return false; -} - -bool isAbstract(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isPureVirtual(); - if (const auto *CRD = llvm::dyn_cast(D)) - return CRD->hasDefinition() && CRD->isAbstract(); - return false; -} - -bool isVirtual(const Decl *D) { - if (const auto *CMD = llvm::dyn_cast(D)) - return CMD->isVirtual(); - return false; -} - -bool isFinal(const Decl *D) { - if (const auto *CRD = dyn_cast(D)) - return CRD->hasAttr(); - - if (const auto *CRD = dyn_cast(D)) - return CRD->hasAttr(); - - return false; -} - -bool isUniqueDefinition(const NamedDecl *Decl) { - if (auto *Func = dyn_cast(Decl)) - return Func->isThisDeclarationADefinition(); - if (auto *Klass = dyn_cast(Decl)) - return Klass->isThisDeclarationADefinition(); - if (auto *Iface = dyn_cast(Decl)) - return Iface->isThisDeclarationADefinition(); - if (auto *Proto = dyn_cast(Decl)) - return Proto->isThisDeclarationADefinition(); - if (auto *Var = dyn_cast(Decl)) - return Var->isThisDeclarationADefinition(); - return isa(Decl) || - isa(Decl) || - isa(Decl) || isa(Decl) || - isa(Decl); -} - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h index d4bae8bab8836..7ba8f423308fb 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -130,43 +130,6 @@ std::vector diffTokens(llvm::ArrayRef Before, // are not included. std::vector getInactiveRegions(ParsedAST &AST); - -// Whether T is const in a loose sense - is a variable with this type readonly? -bool isConst(QualType T); - -// Whether D is const in a loose sense (should it be highlighted as such?) -// FIXME: This is separate from whether *a particular usage* can mutate D. -// We may want V in V.size() to be readonly even if V is mutable. -bool isConst(const Decl *D); - -// "Static" means many things in C++, only some get the "static" modifier. -// -// Meanings that do: -// - Members associated with the class rather than the instance. -// This is what 'static' most often means across languages. -// - static local variables -// These are similarly "detached from their context" by the static keyword. -// In practice, these are rarely used inside classes, reducing confusion. -// -// Meanings that don't: -// - Namespace-scoped variables, which have static storage class. -// This is implicit, so the keyword "static" isn't so strongly associated. -// If we want a modifier for these, "global scope" is probably the concept. -// - Namespace-scoped variables/functions explicitly marked "static". -// There the keyword changes *linkage* , which is a totally different concept. -// If we want to model this, "file scope" would be a nice modifier. -// -// This is confusing, and maybe we should use another name, but because "static" -// is a standard LSP modifier, having one with that name has advantages. -bool isStatic(const Decl *D); -// Indicates whether declaration D is abstract in cases where D is a struct or a -// class. -bool isAbstract(const Decl *D); -// Indicates whether declaration D is virtual in cases where D is a method. -bool isVirtual(const Decl *D); -// Indicates whether declaration D is final in cases where D is a struct, class -// or method. -bool isFinal(const Decl *D); // Indicates whether declaration D is a unique definition (as opposed to a // declaration). bool isUniqueDefinition(const NamedDecl *Decl); diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index 8b61bdaebadc9..b0cd6a8368982 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -35,8 +35,6 @@ MATCHER_P(withName, N, "") { return arg.name == N; } MATCHER_P(withKind, Kind, "") { return arg.kind == Kind; } MATCHER_P(withDetail, Detail, "") { return arg.detail == Detail; } MATCHER_P(symRange, Range, "") { return arg.range == Range; } -MATCHER_P(withSymbolTag, Tag, "") { return llvm::is_contained(arg.tags, Tag); } - // GMock helpers for matching DocumentSymbol. MATCHER_P(symNameRange, Range, "") { return arg.selectionRange == Range; } @@ -45,6 +43,12 @@ ::testing::Matcher children(ChildMatchers... ChildrenM) { return Field(&DocumentSymbol::children, UnorderedElementsAre(ChildrenM...)); } +template +::testing::Matcher withSymbolTags(Tags... tags) { + // Matches the tags vector ignoring element order. + return Field(&DocumentSymbol::tags, UnorderedElementsAre(tags...)); +} + std::vector getSymbols(TestTU &TU, llvm::StringRef Query, int Limit = 0) { auto SymbolInfos = getWorkspaceSymbols(Query, Limit, TU.index().get(), @@ -1147,13 +1151,13 @@ TEST(DocumentSymbolsTest, SymbolTags) { EXPECT_THAT( Symbols, ElementsAre(AllOf( - withName("A"), withSymbolTag(SymbolTag::Abstract), - withSymbolTag(SymbolTag::Definition), - children(AllOf(withName("f"), withSymbolTag(SymbolTag::ReadOnly), - withSymbolTag(SymbolTag::Virtual), - withSymbolTag(SymbolTag::Abstract), - withSymbolTag(SymbolTag::Declaration), - withSymbolTag(SymbolTag::Private)))))); + withName("A"), withSymbolTags(SymbolTag::Abstract, SymbolTag::Definition, SymbolTag::Declaration), + children(AllOf(withName("f"), + withSymbolTags(SymbolTag::ReadOnly, + SymbolTag::Virtual, + SymbolTag::Abstract, + SymbolTag::Declaration, + SymbolTag::Private)))))); } } // namespace From 438b02fb7029abad0ef78ccd3044193b57d80967 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 14 Jan 2026 12:26:35 +0100 Subject: [PATCH 16/26] Fixed tests. --- .../clangd/test/symbol-tags.test | 40 +++++++++---------- clang-tools-extra/clangd/test/symbols.test | 4 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/clang-tools-extra/clangd/test/symbol-tags.test b/clang-tools-extra/clangd/test/symbol-tags.test index b59c5181cce9b..5739c355b01d7 100644 --- a/clang-tools-extra/clangd/test/symbol-tags.test +++ b/clang-tools-extra/clangd/test/symbol-tags.test @@ -38,10 +38,10 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ +# CHECK-NEXT: 5, # CHECK-NEXT: 15, -# CHECK-NEXT: 19, # CHECK-NEXT: 18, -# CHECK-NEXT: 5 +# CHECK-NEXT: 19 # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { @@ -69,10 +69,10 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ -# CHECK-NEXT: 15, +# CHECK-NEXT: 5, # CHECK-NEXT: 9, -# CHECK-NEXT: 18, -# CHECK-NEXT: 5 +# CHECK-NEXT: 15, +# CHECK-NEXT: 18 # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { @@ -100,9 +100,9 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ -# CHECK-NEXT: 20, +# CHECK-NEXT: 5, # CHECK-NEXT: 18, -# CHECK-NEXT: 5 +# CHECK-NEXT: 20 # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { @@ -130,9 +130,9 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ -# CHECK-NEXT: 19, +# CHECK-NEXT: 4, # CHECK-NEXT: 18, -# CHECK-NEXT: 4 +# CHECK-NEXT: 19 # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { @@ -160,10 +160,10 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ +# CHECK-NEXT: 2, # CHECK-NEXT: 8, -# CHECK-NEXT: 19, # CHECK-NEXT: 18, -# CHECK-NEXT: 2 +# CHECK-NEXT: 19 # CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ], @@ -192,8 +192,8 @@ # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ # CHECK-NEXT: 9, -# CHECK-NEXT: 19, -# CHECK-NEXT: 18 +# CHECK-NEXT: 18, +# CHECK-NEXT: 19 # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { @@ -221,10 +221,10 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ -# CHECK-NEXT: 20, -# CHECK-NEXT: 19, +# CHECK-NEXT: 5, # CHECK-NEXT: 18, -# CHECK-NEXT: 5 +# CHECK-NEXT: 19, +# CHECK-NEXT: 20 # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: { @@ -254,11 +254,11 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ -# CHECK-NEXT: 15, +# CHECK-NEXT: 5, # CHECK-NEXT: 10, -# CHECK-NEXT: 19, +# CHECK-NEXT: 15, # CHECK-NEXT: 18, -# CHECK-NEXT: 5 +# CHECK-NEXT: 19 # CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ], @@ -287,8 +287,8 @@ # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ # CHECK-NEXT: 10, +# CHECK-NEXT: 18, # CHECK-NEXT: 19 -# CHECK-NEXT: 18 # CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ] diff --git a/clang-tools-extra/clangd/test/symbols.test b/clang-tools-extra/clangd/test/symbols.test index 4d818d9d1dcf4..a16a226e48c05 100644 --- a/clang-tools-extra/clangd/test/symbols.test +++ b/clang-tools-extra/clangd/test/symbols.test @@ -86,8 +86,8 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "tags": [ -# CHECK-NEXT: 19, -# CHECK-NEXT: 18 +# CHECK-NEXT: 18, +# CHECK-NEXT: 19 # CHECK-NEXT: ] # CHECK-NEXT: } # CHECK-NEXT: ] From 53a8c8fdd0c79f7a7fb010aaaf86803259c1418c Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 14 Jan 2026 12:26:58 +0100 Subject: [PATCH 17/26] Includes clean-up. --- clang-tools-extra/clangd/FindSymbols.cpp | 1 - clang-tools-extra/clangd/SemanticHighlighting.cpp | 2 -- clang-tools-extra/clangd/SemanticHighlighting.h | 1 - 3 files changed, 4 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index bfb6ca3462dcd..b338cd1d9d5d6 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -23,7 +23,6 @@ #include "llvm/ADT/StringRef.h" #include #include -#include #define DEBUG_TYPE "FindSymbols" diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 54e19bc7146be..d40a98216edd4 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -11,7 +11,6 @@ #include "FindSymbols.h" #include "FindTarget.h" #include "ParsedAST.h" -#include "AST.h" #include "Protocol.h" #include "SourceCode.h" #include "support/Logger.h" @@ -25,7 +24,6 @@ #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" -#include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Sema/HeuristicResolver.h" diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h index 7ba8f423308fb..d352988888a24 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -24,7 +24,6 @@ #include "clang/AST/TypeBase.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/raw_ostream.h" namespace clang { class NamedDecl; From 31dc8a5b5de10e6c63fc67c84ab4c86a3a23846a Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 14 Jan 2026 14:21:37 +0100 Subject: [PATCH 18/26] Removed computeSymbolTags(const NamedDecl &ND) from public API. --- clang-tools-extra/clangd/FindSymbols.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 81683cf13d8a6..7b65425509b39 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -55,12 +55,8 @@ llvm::Expected> getDocumentSymbols(ParsedAST &AST); /// \p ND The declaration to get tags for. std::vector getSymbolTags(const NamedDecl &ND); - /// Computes symbol tags for a given NamedDecl. -SymbolTags computeSymbolTags(const NamedDecl &ND); - -/// Computes symbol tags for a given NamedDecl, with additional context about -/// whether it's a declaration or definition. +/// \p IsDecl indicates whether ND is a declaration (true). SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl); } // namespace clangd From 04144e5cae16323b47401a61ef25c278ea6f8e93 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 15 Jan 2026 09:43:14 +0100 Subject: [PATCH 19/26] Fix format. --- clang-tools-extra/clangd/FindSymbols.cpp | 5 +---- clang-tools-extra/clangd/FindSymbols.h | 2 +- .../clangd/SemanticHighlighting.cpp | 13 +++++-------- .../clangd/unittests/FindSymbolsTests.cpp | 17 +++++++++-------- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index b338cd1d9d5d6..2c5acad01166a 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -35,8 +35,6 @@ SymbolTags toSymbolTagBitmask(const SymbolTag ST) { } } // namespace - - // "Static" means many things in C++, only some get the "static" modifier. // // Meanings that do: @@ -94,7 +92,7 @@ bool isConst(const Decl *D) { llvm::isa(D) || llvm::isa(D)) { if (isConst(llvm::cast(D)->getType())) return true; - } + } if (const auto *OCPD = llvm::dyn_cast(D)) { if (OCPD->isReadOnly()) return true; @@ -141,7 +139,6 @@ bool isFinal(const Decl *D) { return false; } - bool isUniqueDefinition(const NamedDecl *Decl) { if (auto *Func = dyn_cast(Decl)) return Func->isThisDeclarationADefinition(); diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 7b65425509b39..015c01ebc9f64 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -14,8 +14,8 @@ #include "Protocol.h" #include "index/Symbol.h" -#include "llvm/ADT/StringRef.h" #include "clang/AST/Decl.h" +#include "llvm/ADT/StringRef.h" namespace clang { namespace clangd { diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index d40a98216edd4..974e6239473ab 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -132,10 +132,9 @@ std::optional kindForDecl(const NamedDecl *D, if (auto *VD = dyn_cast(D)) { if (isa(VD)) // e.g. ObjC Self return std::nullopt; - return VD->isStaticDataMember() - ? HighlightingKind::StaticField - : VD->isLocalVarDecl() ? HighlightingKind::LocalVariable - : HighlightingKind::Variable; + return VD->isStaticDataMember() ? HighlightingKind::StaticField + : VD->isLocalVarDecl() ? HighlightingKind::LocalVariable + : HighlightingKind::Variable; } if (const auto *BD = dyn_cast(D)) return BD->getDeclContext()->isFunctionOrMethod() @@ -1412,9 +1411,8 @@ llvm::StringRef toSemanticTokenModifier(HighlightingModifier Modifier) { llvm_unreachable("unhandled HighlightingModifier"); } -std::vector -diffTokens(llvm::ArrayRef Old, - llvm::ArrayRef New) { +std::vector diffTokens(llvm::ArrayRef Old, + llvm::ArrayRef New) { // For now, just replace everything from the first-last modification. // FIXME: use a real diff instead, this is bad with include-insertion. @@ -1473,6 +1471,5 @@ std::vector getInactiveRegions(ParsedAST &AST) { return InactiveRegions; } - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index b0cd6a8368982..6b3a4ff56a4b8 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -1150,14 +1150,15 @@ TEST(DocumentSymbolsTest, SymbolTags) { auto Symbols = getSymbols(TU.build()); EXPECT_THAT( Symbols, - ElementsAre(AllOf( - withName("A"), withSymbolTags(SymbolTag::Abstract, SymbolTag::Definition, SymbolTag::Declaration), - children(AllOf(withName("f"), - withSymbolTags(SymbolTag::ReadOnly, - SymbolTag::Virtual, - SymbolTag::Abstract, - SymbolTag::Declaration, - SymbolTag::Private)))))); + ElementsAre( + AllOf(withName("A"), + withSymbolTags(SymbolTag::Abstract, SymbolTag::Definition, + SymbolTag::Declaration), + children(AllOf( + withName("f"), + withSymbolTags(SymbolTag::ReadOnly, SymbolTag::Virtual, + SymbolTag::Abstract, SymbolTag::Declaration, + SymbolTag::Private)))))); } } // namespace From 64978c3f534c1670b8901c164a3b0705ad6f485e Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 15 Jan 2026 16:08:33 +0100 Subject: [PATCH 20/26] Review findings: Refactored unit and integration tests. Robust SymbolTag declaration. Static assert of the symbol tag bit-mask's size. And other minor improvements... --- clang-tools-extra/clangd/FindSymbols.cpp | 10 +- clang-tools-extra/clangd/FindSymbols.h | 8 +- clang-tools-extra/clangd/Protocol.h | 6 + .../clangd/SemanticHighlighting.h | 4 - clang-tools-extra/clangd/XRefs.cpp | 1 - .../clangd/test/symbol-tags.test | 355 ++++-------------- .../clangd/unittests/FindSymbolsTests.cpp | 66 +++- 7 files changed, 142 insertions(+), 308 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 2c5acad01166a..887c228bbf0df 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -33,7 +33,6 @@ namespace { SymbolTags toSymbolTagBitmask(const SymbolTag ST) { return (1 << static_cast(ST)); } -} // namespace // "Static" means many things in C++, only some get the "static" modifier. // @@ -139,6 +138,8 @@ bool isFinal(const Decl *D) { return false; } +// Indicates whether declaration D is a unique definition (as opposed to a +// declaration). bool isUniqueDefinition(const NamedDecl *Decl) { if (auto *Func = dyn_cast(Decl)) return Func->isThisDeclarationADefinition(); @@ -155,6 +156,7 @@ bool isUniqueDefinition(const NamedDecl *Decl) { isa(Decl) || isa(Decl) || isa(Decl); } +} // namespace // Backwards-compatible default behavior: determine whether this NamedDecl is // definition based on `isUniqueDefinition`, assuming that ND is a declaration. @@ -220,9 +222,9 @@ std::vector getSymbolTags(const NamedDecl &ND) { // Iterate through SymbolTag enum values and collect any that are present in // the bitmask. SymbolTag values are in the numeric range - // [Deprecated .. ReadOnly]. - constexpr unsigned MinTag = static_cast(SymbolTag::Deprecated); - constexpr unsigned MaxTag = static_cast(SymbolTag::ReadOnly); + // [FirstTag .. LastTag]. + constexpr unsigned MinTag = static_cast(SymbolTag::FirstTag); + constexpr unsigned MaxTag = static_cast(SymbolTag::LastTag); for (unsigned I = MinTag; I <= MaxTag; ++I) { auto ST = static_cast(I); if (symbolTags & toSymbolTagBitmask(ST)) diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 015c01ebc9f64..0040cc6e5c078 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -22,8 +22,14 @@ namespace clangd { class ParsedAST; class SymbolIndex; -/// A bitmask type representing symbol tags. +/// A bitmask type representing symbol tags supported by LSP. +/// \see +/// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#symbolTag using SymbolTags = uint32_t; +/// Ensure we have enough bits to represent all SymbolTag values. +static_assert(static_cast(SymbolTag::LastTag) <= 32, + "Too many SymbolTags to fit in uint32_t. Change to uint64_t if " + "we ever have more than 32 tags."); /// Helper function for deriving an LSP Location from an index SymbolLocation. llvm::Expected indexToLSPLocation(const SymbolLocation &Loc, diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index b8d18ce9eee79..15faec19bc6a4 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1104,6 +1104,8 @@ struct CodeAction { }; llvm::json::Value toJSON(const CodeAction &); +/// Symbol tags are extra annotations that can be attached to a symbol. +/// \see https://github.com/microsoft/language-server-protocol/pull/2003 enum class SymbolTag { Deprecated = 1, Private = 2, @@ -1125,6 +1127,10 @@ enum class SymbolTag { Declaration = 18, Definition = 19, ReadOnly = 20, + + // Update as needed + FirstTag = Deprecated, + LastTag = ReadOnly }; llvm::json::Value toJSON(SymbolTag); /// Represents programming constructs like variables, classes, interfaces etc. diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h index d352988888a24..9a806c438f597 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -129,10 +129,6 @@ std::vector diffTokens(llvm::ArrayRef Before, // are not included. std::vector getInactiveRegions(ParsedAST &AST); -// Indicates whether declaration D is a unique definition (as opposed to a -// declaration). -bool isUniqueDefinition(const NamedDecl *Decl); - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 2c99a54e08c96..8a24d19a7d129 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1812,7 +1812,6 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { HI.name = printName(Ctx, ND); // FIXME: Populate HI.detail the way we do in symbolToHierarchyItem? HI.kind = SK; - HI.tags = getSymbolTags(ND); HI.range = Range{sourceLocToPosition(SM, DeclRange->getBegin()), sourceLocToPosition(SM, DeclRange->getEnd())}; HI.selectionRange = Range{NameBegin, NameEnd}; diff --git a/clang-tools-extra/clangd/test/symbol-tags.test b/clang-tools-extra/clangd/test/symbol-tags.test index 5739c355b01d7..6b17dc994d029 100644 --- a/clang-tools-extra/clangd/test/symbol-tags.test +++ b/clang-tools-extra/clangd/test/symbol-tags.test @@ -5,294 +5,79 @@ --- {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1, "text": - "class AbstractClass{\n public:\n virtual ~AbstractClass() = default;\n virtual void f1() = 0;\n void f2() const;\n protected: \n void f3(){}\n private: \n static void f4(){} \n }; void AbstractClass::f2() const {} \n class ImplClass final: public AbstractClass { \n public: \n void f1() final {}};" + "class A {\n virtual void f() const = 0;\n};" }}} --- {"jsonrpc":"2.0","id":2,"method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"test:///main.cpp"}}} -# CHECK: "id": 2, -# CHECK-NEXT: "jsonrpc": "2.0", -# CHECK-NEXT: "result": [ -# CHECK-NEXT: { -# CHECK-NEXT: "children": [ -# CHECK-NEXT: { -# CHECK-NEXT: "kind": 9, -# CHECK-NEXT: "name": "~AbstractClass", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 35, -# CHECK-NEXT: "line": 2 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 1, -# CHECK-NEXT: "line": 2 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 10, -# CHECK-NEXT: "line": 2 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 9, -# CHECK-NEXT: "line": 2 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 5, -# CHECK-NEXT: 15, -# CHECK-NEXT: 18, -# CHECK-NEXT: 19 -# CHECK-NEXT: ] -# CHECK-NEXT: }, -# CHECK-NEXT: { -# CHECK-NEXT: "detail": "void ()", -# CHECK-NEXT: "kind": 6, -# CHECK-NEXT: "name": "f1", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 22, -# CHECK-NEXT: "line": 3 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 1, -# CHECK-NEXT: "line": 3 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 16, -# CHECK-NEXT: "line": 3 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 14, -# CHECK-NEXT: "line": 3 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 5, -# CHECK-NEXT: 9, -# CHECK-NEXT: 15, -# CHECK-NEXT: 18 -# CHECK-NEXT: ] -# CHECK-NEXT: }, -# CHECK-NEXT: { -# CHECK-NEXT: "detail": "void () const", -# CHECK-NEXT: "kind": 6, -# CHECK-NEXT: "name": "f2", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 16, -# CHECK-NEXT: "line": 4 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 1, -# CHECK-NEXT: "line": 4 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 8, -# CHECK-NEXT: "line": 4 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 6, -# CHECK-NEXT: "line": 4 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 5, -# CHECK-NEXT: 18, -# CHECK-NEXT: 20 -# CHECK-NEXT: ] -# CHECK-NEXT: }, -# CHECK-NEXT: { -# CHECK-NEXT: "detail": "void ()", -# CHECK-NEXT: "kind": 6, -# CHECK-NEXT: "name": "f3", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 12, -# CHECK-NEXT: "line": 6 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 1, -# CHECK-NEXT: "line": 6 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 8, -# CHECK-NEXT: "line": 6 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 6, -# CHECK-NEXT: "line": 6 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 4, -# CHECK-NEXT: 18, -# CHECK-NEXT: 19 -# CHECK-NEXT: ] -# CHECK-NEXT: }, -# CHECK-NEXT: { -# CHECK-NEXT: "detail": "void ()", -# CHECK-NEXT: "kind": 6, -# CHECK-NEXT: "name": "f4", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 19, -# CHECK-NEXT: "line": 8 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 1, -# CHECK-NEXT: "line": 8 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 15, -# CHECK-NEXT: "line": 8 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 13, -# CHECK-NEXT: "line": 8 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 2, -# CHECK-NEXT: 8, -# CHECK-NEXT: 18, -# CHECK-NEXT: 19 -# CHECK-NEXT: ] -# CHECK-NEXT: } -# CHECK-NEXT: ], -# CHECK-NEXT: "detail": "class", -# CHECK-NEXT: "kind": 5, -# CHECK-NEXT: "name": "AbstractClass", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 2, -# CHECK-NEXT: "line": 9 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 19, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 6, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 9, -# CHECK-NEXT: 18, -# CHECK-NEXT: 19 -# CHECK-NEXT: ] -# CHECK-NEXT: }, -# CHECK-NEXT: { -# CHECK-NEXT: "detail": "void () const", -# CHECK-NEXT: "kind": 6, -# CHECK-NEXT: "name": "AbstractClass::f2", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 37, -# CHECK-NEXT: "line": 9 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 4, -# CHECK-NEXT: "line": 9 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 26, -# CHECK-NEXT: "line": 9 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 24, -# CHECK-NEXT: "line": 9 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 5, -# CHECK-NEXT: 18, -# CHECK-NEXT: 19, -# CHECK-NEXT: 20 -# CHECK-NEXT: ] -# CHECK-NEXT: }, -# CHECK-NEXT: { -# CHECK-NEXT: "children": [ -# CHECK-NEXT: { -# CHECK-NEXT: "detail": "void ()", -# CHECK-NEXT: "kind": 6, -# CHECK-NEXT: "name": "f1", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 19, -# CHECK-NEXT: "line": 12 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 1, -# CHECK-NEXT: "line": 12 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 8, -# CHECK-NEXT: "line": 12 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 6, -# CHECK-NEXT: "line": 12 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 5, -# CHECK-NEXT: 10, -# CHECK-NEXT: 15, -# CHECK-NEXT: 18, -# CHECK-NEXT: 19 -# CHECK-NEXT: ] -# CHECK-NEXT: } -# CHECK-NEXT: ], -# CHECK-NEXT: "detail": "class", -# CHECK-NEXT: "kind": 5, -# CHECK-NEXT: "name": "ImplClass", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 20, -# CHECK-NEXT: "line": 12 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 1, -# CHECK-NEXT: "line": 10 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "selectionRange": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 16, -# CHECK-NEXT: "line": 10 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 7, -# CHECK-NEXT: "line": 10 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 10, -# CHECK-NEXT: 18, -# CHECK-NEXT: 19 -# CHECK-NEXT: ] -# CHECK-NEXT: } -# CHECK-NEXT: ] -# CHECK-NEXT:} +# CHECK: "id": 2 +# CHECK: "jsonrpc": "2.0", +# CHECK: "result": [ +# CHECK: { +# CHECK: "children": [ +# CHECK: { +# CHECK: "detail": "void () const", +# CHECK: "kind": 6, +# CHECK: "name": "f", +# CHECK: "range": { +# CHECK: "end": { +# CHECK: "character": 27, +# CHECK: "line": 1 +# CHECK: }, +# CHECK: "start": { +# CHECK: "character": 1, +# CHECK: "line": 1 +# CHECK: } +# CHECK: }, +# CHECK: "selectionRange": { +# CHECK: "end": { +# CHECK: "character": 15, +# CHECK: "line": 1 +# CHECK: }, +# CHECK: "start": { +# CHECK: "character": 14, +# CHECK: "line": 1 +# CHECK: } +# CHECK: }, +# CHECK: "tags": [ +# CHECK: 2, +# CHECK: 9, +# CHECK: 15, +# CHECK: 18, +# CHECK: 20 +# CHECK: ] +# CHECK: } +# CHECK: ], +# CHECK: "detail": "class", +# CHECK: "kind": 5, +# CHECK: "name": "A", +# CHECK: "range": { +# CHECK: "end": { +# CHECK: "character": 1, +# CHECK: "line": 2 +# CHECK: }, +# CHECK: "start": { +# CHECK: "character": 0, +# CHECK: "line": 0 +# CHECK: } +# CHECK: }, +# CHECK: "selectionRange": { +# CHECK: "end": { +# CHECK: "character": 7, +# CHECK: "line": 0 +# CHECK: }, +# CHECK: "start": { +# CHECK: "character": 6, +# CHECK: "line": 0 +# CHECK: } +# CHECK: }, +# CHECK: "tags": [ +# CHECK: 9, +# CHECK: 18, +# CHECK: 19 +# CHECK: ] +# CHECK: } +# CHECK: ] +# CHECK: } --- {"jsonrpc":"2.0","id":3,"method":"shutdown"} diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index 6b3a4ff56a4b8..d0dd5d0f8f434 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -1140,25 +1140,65 @@ TEST(DocumentSymbolsTest, PragmaMarkGroupsNoNesting) { TEST(DocumentSymbolsTest, SymbolTags) { TestTU TU; - Annotations Main(R"( - class A { - virtual void f() const = 0; - }; - )"); + Annotations Main(R"cpp( + class AbstractClass { + public: + virtual ~AbstractClass() = default; + virtual void f1() = 0; + void f2() const; + protected: + void f3(){} + private: + static void f4(){} + }; + + void AbstractClass::f2() const {} + + class ImplClass final: public AbstractClass { + public: + void f1() final {} + }; + )cpp"); TU.Code = Main.code().str(); auto Symbols = getSymbols(TU.build()); EXPECT_THAT( Symbols, - ElementsAre( - AllOf(withName("A"), - withSymbolTags(SymbolTag::Abstract, SymbolTag::Definition, - SymbolTag::Declaration), + UnorderedElementsAre( + AllOf( + withName("AbstractClass"), + withSymbolTags(SymbolTag::Abstract, SymbolTag::Declaration, + SymbolTag::Definition), + children( + AllOf(withName("~AbstractClass"), + withSymbolTags(SymbolTag::Public, SymbolTag::Virtual, + SymbolTag::Declaration, + SymbolTag::Definition)), + AllOf(withName("f1"), + withSymbolTags(SymbolTag::Public, SymbolTag::Abstract, + SymbolTag::Virtual, + SymbolTag::Declaration)), + AllOf(withName("f2"), withSymbolTags(SymbolTag::Public, + SymbolTag::Declaration, + SymbolTag::ReadOnly)), + AllOf(withName("f3"), withSymbolTags(SymbolTag::Protected, + SymbolTag::Declaration, + SymbolTag::Definition)), + AllOf(withName("f4"), + withSymbolTags(SymbolTag::Private, SymbolTag::Static, + SymbolTag::Declaration, + SymbolTag::Definition)))), + AllOf(withName("AbstractClass::f2"), + withSymbolTags(SymbolTag::Public, SymbolTag::Declaration, + SymbolTag::Definition, SymbolTag::ReadOnly)), + AllOf(withName("ImplClass"), + withSymbolTags(SymbolTag::Final, SymbolTag::Declaration, + SymbolTag::Definition), children(AllOf( - withName("f"), - withSymbolTags(SymbolTag::ReadOnly, SymbolTag::Virtual, - SymbolTag::Abstract, SymbolTag::Declaration, - SymbolTag::Private)))))); + withName("f1"), + withSymbolTags(SymbolTag::Public, SymbolTag::Final, + SymbolTag::Virtual, SymbolTag::Declaration, + SymbolTag::Definition)))))); } } // namespace From b265a9c6d3ba3c94ffd6b10f11f0cbca079f154b Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 15 Jan 2026 21:33:45 +0100 Subject: [PATCH 21/26] Fix test. --- clang-tools-extra/clangd/test/call-hierarchy.test | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/clang-tools-extra/clangd/test/call-hierarchy.test b/clang-tools-extra/clangd/test/call-hierarchy.test index a25731f03b4c5..c81a26bdcd672 100644 --- a/clang-tools-extra/clangd/test/call-hierarchy.test +++ b/clang-tools-extra/clangd/test/call-hierarchy.test @@ -30,10 +30,7 @@ # CHECK-NEXT: "character": 5, # CHECK-NEXT: "line": 0 # CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "tags": [ -# CHECK-NEXT: 18 -# CHECK-NEXT: ], +# CHECK-NEXT: } # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" # CHECK-NEXT: } --- From bd233a463dcf757d359c7bf1b4f4093c9000733d Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Mon, 19 Jan 2026 09:55:36 +0100 Subject: [PATCH 22/26] Review findings: refactored computeSymbolTags - no IsDecl is needed, refactored getSemanticHighlightings - intro map for tags and modifiers, --- clang-tools-extra/clangd/FindSymbols.cpp | 8 +--- clang-tools-extra/clangd/FindSymbols.h | 3 +- .../clangd/SemanticHighlighting.cpp | 42 ++++++++++--------- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 887c228bbf0df..6cf5927725eae 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -158,13 +158,7 @@ bool isUniqueDefinition(const NamedDecl *Decl) { } } // namespace -// Backwards-compatible default behavior: determine whether this NamedDecl is -// definition based on `isUniqueDefinition`, assuming that ND is a declaration. SymbolTags computeSymbolTags(const NamedDecl &ND) { - return computeSymbolTags(ND, true); -} - -SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl) { SymbolTags Result = 0; const auto IsDef = isUniqueDefinition(&ND); @@ -186,7 +180,7 @@ SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl) { if (isFinal(&ND)) Result |= toSymbolTagBitmask(SymbolTag::Final); - if (IsDecl && not isa(ND)) { + if (not isa(ND)) { // Do not treat an UnresolvedUsingValueDecl as a declaration. // It's more common to think of it as a reference to the // underlying declaration. diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 0040cc6e5c078..7fefead17056f 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -62,8 +62,7 @@ llvm::Expected> getDocumentSymbols(ParsedAST &AST); std::vector getSymbolTags(const NamedDecl &ND); /// Computes symbol tags for a given NamedDecl. -/// \p IsDecl indicates whether ND is a declaration (true). -SymbolTags computeSymbolTags(const NamedDecl &ND, bool IsDecl); +SymbolTags computeSymbolTags(const NamedDecl &ND); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 974e6239473ab..70243027f83f5 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1061,22 +1061,30 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { if (auto Mod = scopeModifier(Decl)) Tok.addModifier(*Mod); - const auto SymbolTags = computeSymbolTags(*Decl, R.IsDecl); - - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) - Tok.addModifier(HighlightingModifier::Deprecated); - - if (SymbolTags & toSymbolTagBitmask(SymbolTag::ReadOnly)) - Tok.addModifier(HighlightingModifier::Readonly); - - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Static)) - Tok.addModifier(HighlightingModifier::Static); + const auto SymbolTags = computeSymbolTags(*Decl); + + static const llvm::DenseMap + TagModifierMap = { + {SymbolTag::Deprecated, HighlightingModifier::Deprecated}, + {SymbolTag::ReadOnly, HighlightingModifier::Readonly}, + {SymbolTag::Static, HighlightingModifier::Static}, + {SymbolTag::Virtual, HighlightingModifier::Virtual}, + {SymbolTag::Abstract, HighlightingModifier::Abstract}, + // Declaration and Definition are handled separately below. + }; + + for (const auto &[Tag, Modifier] : TagModifierMap) { + if (SymbolTags & toSymbolTagBitmask(Tag)) + Tok.addModifier(Modifier); + } - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Virtual)) - Tok.addModifier(HighlightingModifier::Virtual); + if (R.IsDecl && + (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration))) { + Tok.addModifier(HighlightingModifier::Declaration); - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Abstract)) - Tok.addModifier(HighlightingModifier::Abstract); + if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition)) + Tok.addModifier(HighlightingModifier::Definition); + } if (isDependent(Decl)) Tok.addModifier(HighlightingModifier::DependentName); @@ -1086,12 +1094,6 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { if (isa(Decl)) Tok.addModifier(HighlightingModifier::ConstructorOrDestructor); - - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration)) - Tok.addModifier(HighlightingModifier::Declaration); - - if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition)) - Tok.addModifier(HighlightingModifier::Definition); } }, AST.getHeuristicResolver()); From 2eb7d3bed3705b349aca8d5e2671ff96cd44eeed Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 20 Jan 2026 14:02:29 +0100 Subject: [PATCH 23/26] Review findings: - obsolete includes, - public toSymbolTagBitmask, - removed tags from TypeHierarchyItem, - thread_local map in getSemanticHighlightings - minor improvements --- clang-tools-extra/clangd/AST.cpp | 2 -- clang-tools-extra/clangd/AST.h | 1 - clang-tools-extra/clangd/FindSymbols.cpp | 7 ++++--- clang-tools-extra/clangd/FindSymbols.h | 11 ++++++++--- clang-tools-extra/clangd/Protocol.h | 3 --- clang-tools-extra/clangd/SemanticHighlighting.cpp | 7 +------ clang-tools-extra/clangd/test/call-hierarchy.test | 2 +- 7 files changed, 14 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 3f96faba18cf3..3bcc89d360cdb 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -8,7 +8,6 @@ #include "AST.h" -#include "SemanticHighlighting.h" #include "SourceCode.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTTypeTraits.h" @@ -618,7 +617,6 @@ class DeducedTypeVisitor : public RecursiveASTVisitor { QualType DeducedType; }; - } // namespace std::optional getDeducedType(ASTContext &ASTCtx, diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 2bb4943b6de0b..193c122222684 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -17,7 +17,6 @@ #include "index/Symbol.h" #include "index/SymbolID.h" #include "clang/AST/Decl.h" -#include "clang/AST/DeclObjC.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/MacroInfo.h" diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 6cf5927725eae..243746056aed0 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -30,9 +30,6 @@ namespace clang { namespace clangd { namespace { -SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return (1 << static_cast(ST)); -} // "Static" means many things in C++, only some get the "static" modifier. // @@ -158,6 +155,10 @@ bool isUniqueDefinition(const NamedDecl *Decl) { } } // namespace +SymbolTags toSymbolTagBitmask(const SymbolTag ST) { + return (1 << static_cast(ST)); +} + SymbolTags computeSymbolTags(const NamedDecl &ND) { SymbolTags Result = 0; const auto IsDef = isUniqueDefinition(&ND); diff --git a/clang-tools-extra/clangd/FindSymbols.h b/clang-tools-extra/clangd/FindSymbols.h index 7fefead17056f..97b99af4f35e6 100644 --- a/clang-tools-extra/clangd/FindSymbols.h +++ b/clang-tools-extra/clangd/FindSymbols.h @@ -57,13 +57,18 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, /// same order that they appear. llvm::Expected> getDocumentSymbols(ParsedAST &AST); -/// Returns the symbol tags for the given declaration. -/// \p ND The declaration to get tags for. -std::vector getSymbolTags(const NamedDecl &ND); +/// Converts a single SymbolTag to a bitmask. +SymbolTags toSymbolTagBitmask(SymbolTag ST); /// Computes symbol tags for a given NamedDecl. SymbolTags computeSymbolTags(const NamedDecl &ND); +/// Returns the symbol tags for the given declaration. +/// This is a wrapper around computeSymbolTags() which unpacks +/// the tags into a vector. +/// \p ND The declaration to get tags for. +std::vector getSymbolTags(const NamedDecl &ND); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 15faec19bc6a4..a88c9a391f97a 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1548,9 +1548,6 @@ struct TypeHierarchyItem { /// The kind of this item. SymbolKind kind; - /// The symbol tags for this item. - std::vector tags; - /// More detail for this item, e.g. the signature of a function. std::optional detail; diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 70243027f83f5..ce1f51a681ff4 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -22,7 +22,6 @@ #include "clang/AST/DeclarationName.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" -#include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" @@ -1025,10 +1024,6 @@ class CollectExtraHighlightings private: HighlightingsBuilder &H; }; - -SymbolTags toSymbolTagBitmask(const SymbolTag ST) { - return (1 << static_cast(ST)); -} } // namespace std::vector @@ -1063,7 +1058,7 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { const auto SymbolTags = computeSymbolTags(*Decl); - static const llvm::DenseMap + static const thread_local llvm::DenseMap TagModifierMap = { {SymbolTag::Deprecated, HighlightingModifier::Deprecated}, {SymbolTag::ReadOnly, HighlightingModifier::Readonly}, diff --git a/clang-tools-extra/clangd/test/call-hierarchy.test b/clang-tools-extra/clangd/test/call-hierarchy.test index c81a26bdcd672..6548ea0068a8d 100644 --- a/clang-tools-extra/clangd/test/call-hierarchy.test +++ b/clang-tools-extra/clangd/test/call-hierarchy.test @@ -30,7 +30,7 @@ # CHECK-NEXT: "character": 5, # CHECK-NEXT: "line": 0 # CHECK-NEXT: } -# CHECK-NEXT: } +# CHECK-NEXT: }, # CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp" # CHECK-NEXT: } --- From 333355020e5176eff65d7458ece58f96b6433986 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Tue, 20 Jan 2026 20:12:48 +0100 Subject: [PATCH 24/26] Fix format. --- clang-tools-extra/clangd/SemanticHighlighting.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index ce1f51a681ff4..79446a166d0d8 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "SemanticHighlighting.h" +#include "AST.h" #include "Config.h" #include "FindSymbols.h" #include "FindTarget.h" @@ -32,7 +33,6 @@ #include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" -#include #include #include @@ -1058,7 +1058,8 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) { const auto SymbolTags = computeSymbolTags(*Decl); - static const thread_local llvm::DenseMap + static const thread_local llvm::DenseMap TagModifierMap = { {SymbolTag::Deprecated, HighlightingModifier::Deprecated}, {SymbolTag::ReadOnly, HighlightingModifier::Readonly}, From 49a4a783b16d6ec3dbe5fd26bf6cc0c5196e3f6c Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Wed, 21 Jan 2026 09:54:11 +0100 Subject: [PATCH 25/26] Review findings: - removed obsolete forward-decl and includes - restored include --- clang-tools-extra/clangd/AST.h | 1 + clang-tools-extra/clangd/SemanticHighlighting.h | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 193c122222684..2bb4943b6de0b 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -17,6 +17,7 @@ #include "index/Symbol.h" #include "index/SymbolID.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclObjC.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/MacroInfo.h" diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h index 9a806c438f597..ba25c9a7ee6be 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -21,13 +21,9 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICHIGHLIGHTING_H #include "Protocol.h" - -#include "clang/AST/TypeBase.h" #include "llvm/ADT/StringRef.h" namespace clang { -class NamedDecl; - namespace clangd { class ParsedAST; From 585210a280947e8d96fe6e04999925069ef85e66 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz Date: Thu, 22 Jan 2026 21:32:14 +0100 Subject: [PATCH 26/26] Restored include. --- clang-tools-extra/clangd/SemanticHighlighting.h | 1 + 1 file changed, 1 insertion(+) diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h index ba25c9a7ee6be..59d742b83ee52 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -22,6 +22,7 @@ #include "Protocol.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/raw_ostream.h" namespace clang { namespace clangd {