Symbol tags in SymbolInformation, WorkspaceSymbol, CallHierarchyItem and TypeHierarchyItem

clang-tools-extra/clangd/ClangdLSPServer.h

@@ -243,7 +243,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
   /// Used to indicate the ClangdLSPServer is being destroyed.
   std::atomic<bool> IsBeingDestroyed = {false};
 
-  // FIXME: The caching is a temporary solution to get corresponding clangd 
+  // FIXME: The caching is a temporary solution to get corresponding clangd
   // diagnostic from a LSP diagnostic.
   // Ideally, ClangdServer can generate an identifier for each diagnostic,
   // emit them via the LSP's data field (which was newly added in LSP 3.16).

clang-tools-extra/clangd/FindSymbols.cpp

@@ -13,7 +13,10 @@
 #include "Quality.h"
 #include "SourceCode.h"
 #include "index/Index.h"
+#include "index/Symbol.h"
+#include "index/SymbolLocation.h"
 #include "support/Logger.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Index/IndexSymbol.h"
@@ -117,6 +120,16 @@ bool isAbstract(const Decl *D) {
 }
 
 // Indicates whether declaration D is virtual in cases where D is a method.
+// We want to treat a method as virtual if it is declared virtual, even if it
+// is not implemented in this class, or if it overrides/implements a
+// base-class method. This is because the "virtual" modifier is still relevant
+// to the method's behavior and how it should be highlighted, even if it is
+// not itself a virtual method in the strictest sense. For example, a method
+// that overrides a virtual method from a base class is still considered
+// virtual, even if it is not declared as such in the derived class.
+// Similarly, a method that implements a pure virtual method from a base class
+// is also considered virtual, even if it is not declared as such in the
+// derived class.
 bool isVirtual(const Decl *D) {
   if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D))
     return CMD->isVirtual();
@@ -135,6 +148,42 @@ bool isFinal(const Decl *D) {
   return false;
 }
 
+// A method "overrides" if:
+// 1. It overrides at least one method
+// 2. At least one of the overridden methods is virtual (but NOT pure
+// virtual)
+bool isOverrides(const NamedDecl *ND) {
+  if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(ND)) {
+    if (MD->size_overridden_methods() == 0)
+      return false;
+
+    for (const auto *Overridden : MD->overridden_methods()) {
+      // Check if the overridden method is virtual but not pure virtual
+      if (Overridden->isVirtual() && !Overridden->isPureVirtual())
+        return true;
+    }
+  }
+  return false;
+}
+
+// A method "implements" pure virtual methods from base classes if:
+// 1. It overrides at least one method
+// 2. It is NOT itself pure virtual (i.e., it has a concrete implementation)
+// 3. ALL overridden methods are pure virtual
+bool isImplements(const NamedDecl *ND) {
+  if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(ND)) {
+    if (MD->size_overridden_methods() == 0 || MD->isPureVirtual())
+      return false;
+
+    for (const auto *Overridden : MD->overridden_methods()) {
+      if (!Overridden->isPureVirtual())
+        return false;
+    }
+    return true;
+  }
+  return false;
+}
+
 // Indicates whether declaration D is a unique definition (as opposed to a
 // declaration).
 bool isUniqueDefinition(const NamedDecl *Decl) {
@@ -153,6 +202,63 @@ bool isUniqueDefinition(const NamedDecl *Decl) {
          isa<TemplateTemplateParmDecl>(Decl) || isa<ObjCCategoryDecl>(Decl) ||
          isa<ObjCImplDecl>(Decl);
 }
+
+// Filter symbol tags based on the presence of other tags and the kind of
+// symbol. This is needed to avoid redundant tags, e.g. final implies override,
+// override implies virtual, etc.
+SymbolTags filterSymbolTags(const NamedDecl &ND, const SymbolTags ST) {
+  SymbolTags Result = ST;
+
+  if (not isa<CXXMethodDecl>(ND))
+    return Result;
+
+  if (ST & toSymbolTagBitmask(SymbolTag::Overrides)) {
+    // Overrides means that ND overrides an existing implementation of a virtual
+    // method in a base class. If a symbol is marked as Overrides, the tags
+    // Virtual, Declaration and Definition should be removed, as the Overrides
+    // tag implies that the symbol has/is virtual/declaration/definition.
+    Result &= ~toSymbolTagBitmask(SymbolTag::Virtual);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Declaration);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Definition);
+  }
+  if (ST & toSymbolTagBitmask(SymbolTag::Implements)) {
+    // Implements means that ND implements an existing pure virtual method in a
+    // base class. If a symbol is marked as Implements, the tags Virtual,
+    // Declaration, Definition and Overrides should be removed, as the
+    // Implements tag implies that the symbol is virtual, is a declaration, is a
+    // definition, and overrides a method.
+    Result &= ~toSymbolTagBitmask(SymbolTag::Virtual);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Declaration);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Definition);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Overrides);
+  }
+  if (ST & toSymbolTagBitmask(SymbolTag::Virtual)) {
+    // Virtual means that ND is a virtual method that does not override any
+    // method in a base class. If a symbol is marked as Virtual, the tags
+    // Declaration and Definition should be removed, as the Virtual tag implies
+    // that the symbol is a declaration/definition.
+    Result &= ~toSymbolTagBitmask(SymbolTag::Declaration);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Definition);
+  }
+  if (ST & toSymbolTagBitmask(SymbolTag::Abstract)) {
+    // Abstract means that ND is a pure virtual method. If a symbol is marked as
+    // Abstract, the tags Virtual, Declaration and Definition should be removed,
+    // as the Abstract tag implies that the symbol is virtual and a
+    // declaration/definition.
+    Result &= ~toSymbolTagBitmask(SymbolTag::Virtual);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Declaration);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Definition);
+  }
+  if (ST & toSymbolTagBitmask(SymbolTag::Final)) {
+    // Final means that ND is a method that cannot be overridden by any method
+    // in a derived class. If a symbol is marked as Final, the tags Virtual and
+    // Overrides should be removed, as the Final tag implies that the symbol is
+    // virtual.
+    Result &= ~toSymbolTagBitmask(SymbolTag::Virtual);
+    Result &= ~toSymbolTagBitmask(SymbolTag::Overrides);
+  }
+  return Result;
+}
 } // namespace
 
 SymbolTags toSymbolTagBitmask(const SymbolTag ST) {
@@ -178,9 +284,15 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) {
   if (isAbstract(&ND))
     Result |= toSymbolTagBitmask(SymbolTag::Abstract);
 
+  if (isOverrides(&ND))
+    Result |= toSymbolTagBitmask(SymbolTag::Overrides);
+
   if (isFinal(&ND))
     Result |= toSymbolTagBitmask(SymbolTag::Final);
 
+  if (isImplements(&ND))
+    Result |= toSymbolTagBitmask(SymbolTag::Implements);
+
   if (not isa<UnresolvedUsingValueDecl>(ND)) {
     // Do not treat an UnresolvedUsingValueDecl as a declaration.
     // It's more common to think of it as a reference to the
@@ -208,21 +320,47 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) {
   return Result;
 }
 
+std::vector<SymbolTag> expandTagBitmask(const SymbolTags STGS) {
+  std::vector<SymbolTag> Tags;
+
+  if (STGS == 0)
+    return Tags;
+
+  // No filtering required since this function is only used for Symbols from the
+  // index, which have already been filtered in getSymbolTags(const NamedDecl
+  // &ND).
+
+  // Iterate through SymbolTag enum values and collect any that are present in
+  // the bitmask. SymbolTag values are in the numeric range
+  // [FirstTag .. LastTag].
+  constexpr unsigned MinTag = static_cast<unsigned>(SymbolTag::FirstTag);
+  constexpr unsigned MaxTag = static_cast<unsigned>(SymbolTag::LastTag);
+  for (unsigned I = MinTag; I <= MaxTag; ++I) {
+    auto ST = static_cast<SymbolTag>(I);
+    if (STGS & toSymbolTagBitmask(ST))
+      Tags.push_back(ST);
+  }
+  return Tags;
+}
+
 std::vector<SymbolTag> getSymbolTags(const NamedDecl &ND) {
   const auto symbolTags = computeSymbolTags(ND);
   std::vector<SymbolTag> Tags;
 
   if (symbolTags == 0)
     return Tags;
 
+  // Apply specific filter to the symbol tags.
+  const auto filteredTags = filterSymbolTags(ND, symbolTags);
+
   // Iterate through SymbolTag enum values and collect any that are present in
   // the bitmask. SymbolTag values are in the numeric range
   // [FirstTag .. LastTag].
   constexpr unsigned MinTag = static_cast<unsigned>(SymbolTag::FirstTag);
   constexpr unsigned MaxTag = static_cast<unsigned>(SymbolTag::LastTag);
   for (unsigned I = MinTag; I <= MaxTag; ++I) {
     auto ST = static_cast<SymbolTag>(I);
-    if (symbolTags & toSymbolTagBitmask(ST))
+    if (filteredTags & toSymbolTagBitmask(ST))
       Tags.push_back(ST);
   }
   return Tags;
@@ -359,6 +497,7 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit,
     Info.score = Relevance.NameMatch > std::numeric_limits<float>::epsilon()
                      ? Score / Relevance.NameMatch
                      : QualScore;
+    Info.tags = expandTagBitmask(Sym.Tags);
     Top.push({Score, std::move(Info)});
   });
   for (auto &R : std::move(Top).items())

clang-tools-extra/clangd/FindSymbols.h

@@ -12,24 +12,18 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDSYMBOLS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDSYMBOLS_H
 
-#include "Protocol.h"
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
+class NamedDecl;
+
 namespace clangd {
 class ParsedAST;
 class SymbolIndex;
-
-/// 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<unsigned>(SymbolTag::LastTag) <= 32,
-              "Too many SymbolTags to fit in uint32_t. Change to uint64_t if "
-              "we ever have more than 32 tags.");
+struct Symbol;
+struct SymbolLocation;
 
 /// Helper function for deriving an LSP Location from an index SymbolLocation.
 llvm::Expected<Location> indexToLSPLocation(const SymbolLocation &Loc,
@@ -69,6 +63,9 @@ SymbolTags computeSymbolTags(const NamedDecl &ND);
 /// \p ND The declaration to get tags for.
 std::vector<SymbolTag> getSymbolTags(const NamedDecl &ND);
 
+/// Expands a SymbolTags bitmask into individual SymbolTag values.
+std::vector<SymbolTag> expandTagBitmask(SymbolTags STGS);
+
 } // namespace clangd
 } // namespace clang
 

clang-tools-extra/clangd/Protocol.cpp

@@ -867,6 +867,8 @@ llvm::json::Value toJSON(const SymbolInformation &P) {
   };
   if (P.score)
     O["score"] = *P.score;
+  if (!P.tags.empty())
+    O["tags"] = P.tags;
   return std::move(O);
 }
 
@@ -1445,6 +1447,8 @@ llvm::json::Value toJSON(const TypeHierarchyItem &I) {
 
   if (I.detail)
     Result["detail"] = I.detail;
+  if (!I.tags.empty())
+    Result["tags"] = I.tags;
   return std::move(Result);
 }
 

clang-tools-extra/clangd/Protocol.h

@@ -1115,22 +1115,33 @@ enum class SymbolTag {
   Internal = 6,
   File = 7,
   Static = 8,
-  Abstract = 9,
-  Final = 10,
+  Abstract = 9, // In context of a class and method - this symbol indicates a
+                // pure virtual class or method.
+  Final = 10, // In context of a method - this symbol indicates that the method
+              // cannot be overridden in subclasses.
+              // In context of a class - this symbol indicates that the class is
+              // final and thus cannot be extended.
   Sealed = 11,
   Transient = 12,
   Volatile = 13,
   Synchronized = 14,
-  Virtual = 15,
+  Virtual =
+      15, // In context of a method - this symbol indicates a virtual
+          // method declared and implemented in same class, and thereby it is
+          // not implementing or overriding a method from any base class.
   Nullable = 16,
   NonNull = 17,
   Declaration = 18,
   Definition = 19,
   ReadOnly = 20,
+  Overrides = 21,  // In context of a method - this symbol indicates a method
+                   // overriding a virtual method, implemented in base class.
+  Implements = 22, // In context of a method - this symbol indicates a method
+                   // implementing a pure virtual method from a base class.
 
   // Update as needed
   FirstTag = Deprecated,
-  LastTag = ReadOnly
+  LastTag = Implements
 };
 llvm::json::Value toJSON(SymbolTag);
 /// Represents programming constructs like variables, classes, interfaces etc.
@@ -1548,6 +1559,9 @@ struct TypeHierarchyItem {
   /// The kind of this item.
   SymbolKind kind;
 
+  /// The symbol tags for this item.
+  std::vector<SymbolTag> tags;
+
   /// More detail for this item, e.g. the signature of a function.
   std::optional<std::string> detail;
 

clang-tools-extra/clangd/XRefs.cpp

@@ -1818,8 +1818,9 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) {
 
   HierarchyItem HI;
   HI.name = printName(Ctx, ND);
-  // FIXME: Populate HI.detail the way we do in symbolToHierarchyItem?
+  HI.detail = printQualifiedName(ND);
   HI.kind = SK;
+  HI.tags = getSymbolTags(ND);
   HI.range = Range{sourceLocToPosition(SM, DeclRange->getBegin()),
                    sourceLocToPosition(SM, DeclRange->getEnd())};
   HI.selectionRange = Range{NameBegin, NameEnd};
@@ -1872,6 +1873,7 @@ static std::optional<HierarchyItem> symbolToHierarchyItem(const Symbol &S,
   HI.name = std::string(S.Name);
   HI.detail = (S.Scope + S.Name).str();
   HI.kind = indexSymbolKindToSymbolKind(S.SymInfo);
+  HI.tags = expandTagBitmask(S.Tags);
   HI.selectionRange = Loc->range;
   // FIXME: Populate 'range' correctly
   // (https://github.com/clangd/clangd/issues/59).
@@ -1897,8 +1899,6 @@ symbolToCallHierarchyItem(const Symbol &S, PathRef TUPath) {
   if (!Result)
     return Result;
   Result->data = S.ID.str();
-  if (S.Flags & Symbol::Deprecated)
-    Result->tags.push_back(SymbolTag::Deprecated);
   return Result;
 }
 
@@ -2144,7 +2144,7 @@ static void unwrapFindType(
     return;
 
   // If there's a specific type alias, point at that rather than unwrapping.
-  if (const auto* TDT = T->getAs<TypedefType>())
+  if (const auto *TDT = T->getAs<TypedefType>())
     return Out.push_back(QualType(TDT, 0));
 
   // Pointers etc => pointee type.
@@ -2307,24 +2307,23 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
 
 std::optional<std::vector<TypeHierarchyItem>>
 superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) {
-  std::vector<TypeHierarchyItem> Results;
-  if (!Item.data.parents)
+  if (!Index || !Item.data.parents)
     return std::nullopt;
-  if (Item.data.parents->empty())
-    return Results;
   LookupRequest Req;
   llvm::DenseMap<SymbolID, const TypeHierarchyItem::ResolveParams *> IDToData;
   for (const auto &Parent : *Item.data.parents) {
     Req.IDs.insert(Parent.symbolID);
     IDToData[Parent.symbolID] = &Parent;
   }
+  std::vector<TypeHierarchyItem> Results;
   Index->lookup(Req, [&Item, &Results, &IDToData](const Symbol &S) {
     if (auto THI = symbolToTypeHierarchyItem(S, Item.uri.file())) {
       THI->data = *IDToData.lookup(S.ID);
       Results.emplace_back(std::move(*THI));
     }
   });
-  return Results;
+  return Results.empty() ? std::nullopt
+                         : std::make_optional(std::move(Results));
 }
 
 std::vector<TypeHierarchyItem> subTypes(const TypeHierarchyItem &Item,

clang-tools-extra/clangd/XRefs.h

@@ -132,7 +132,8 @@ std::vector<TypeHierarchyItem> getTypeHierarchy(
     const SymbolIndex *Index = nullptr, PathRef TUPath = PathRef{});
 
 /// Returns direct parents of a TypeHierarchyItem using SymbolIDs stored inside
-/// the item.
+/// the item. Returns nullopt if the item does not have parents or if
+/// the index is not provided.
 std::optional<std::vector<TypeHierarchyItem>>
 superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index);
 /// Returns direct children of a TypeHierarchyItem.