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"
@@ -118,6 +121,16 @@ bool isAbstract(const Decl *D) {
 
 // Indicates whether declaration D is virtual in cases where D is a method.
 bool isVirtual(const Decl *D) {
+  // 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.
   if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D))
     return CMD->isVirtual();
   return false;
@@ -159,6 +172,45 @@ SymbolTags toSymbolTagBitmask(const SymbolTag ST) {
   return (1 << static_cast<unsigned>(ST));
 }
 
+bool isOverrides(const NamedDecl *ND) {
+  if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(ND)) {
+    // 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)
+
+    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;
+  }
+  return false;
+}
+
+bool isImplements(const NamedDecl *ND) {
+  if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(ND)) {
+    // 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
+
+    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;
+}
+
 SymbolTags computeSymbolTags(const NamedDecl &ND) {
   SymbolTags Result = 0;
   const auto IsDef = isUniqueDefinition(&ND);
@@ -181,6 +233,12 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) {
   if (isFinal(&ND))
     Result |= toSymbolTagBitmask(SymbolTag::Final);
 
+  if (isOverrides(&ND))
+    Result |= toSymbolTagBitmask(SymbolTag::Overrides);
+
+  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 +266,103 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) {
   return Result;
 }
 
+// Filter symbol tags based on the presence of other tags and the kind of
+// symbol. This is needed to avoid redundant tags.
+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;
+}
+
+std::vector<SymbolTag> expandTagBitmask(const SymbolTags symbolTags) {
+  std::vector<SymbolTag> Tags;
+
+  if (symbolTags == 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 (symbolTags & 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 +499,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);
 
+/// Returns the symbol tags for the given declaration as a bitmask.
+std::vector<SymbolTag> expandTagBitmask(SymbolTags symbolTags);
+
 } // 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,11 +1899,39 @@ 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;
 }
 
+// Tries to find a NamedDecl in the AST that matches the given Symbol.
+// Returns nullptr if the symbol is not found in the current AST.
+const NamedDecl *getNamedDeclFromSymbol(const Symbol &Sym,
+                                        const ParsedAST &AST) {
+  // Try to convert the symbol to a location and find the decl at that location
+  auto SymLoc = symbolToLocation(Sym, AST.tuPath());
+  if (!SymLoc)
+    return nullptr;
+
+  // Check if the symbol location is in the main file
+  if (SymLoc->uri.file() != AST.tuPath())
+    return nullptr;
+
+  // Convert LSP position to source location
+  const auto &SM = AST.getSourceManager();
+  auto CurLoc = sourceLocationInMainFile(SM, SymLoc->range.start);
+  if (!CurLoc) {
+    llvm::consumeError(CurLoc.takeError());
+    return nullptr;
+  }
+
+  // Get all decls at this location
+  auto Decls = getDeclAtPosition(const_cast<ParsedAST &>(AST), *CurLoc, {});
+  if (Decls.empty())
+    return nullptr;
+
+  // Return the first decl (usually the most specific one)
+  return Decls[0];
+}
+
 static void fillSubTypes(const SymbolID &ID,
                          std::vector<TypeHierarchyItem> &SubTypes,
                          const SymbolIndex *Index, int Levels, PathRef TUPath) {
@@ -2144,7 +2174,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.
@@ -2308,6 +2338,8 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
 std::optional<std::vector<TypeHierarchyItem>>
 superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) {
   std::vector<TypeHierarchyItem> Results;
+  if (!Index)
+    return Results;
   if (!Item.data.parents)
     return std::nullopt;
   if (Item.data.parents->empty())