[clangd] Add a new highlighting kind for typedefs

[ilya-biryukov]

Summary

We still attempt to highlight them as underlying types, but fallback to
the generic 'typedef' highlighting kind if the underlying type is too
complicated.

[ilya-biryukov]

clang-tools-extra/clangd/SemanticHighlighting.cpp

530 ↗

(On Diff #219138)

Not sure what the rules for the scope names are, happy to change if I'm doing it wrong

[hokein]

clang-tools-extra/clangd/SemanticHighlighting.cpp

530 ↗

(On Diff #219138)

I think the current name is fine, just checked the name for this type in vscode highlighting configs, but didn't find a particular name for this type (my guess is that regex-based highlighting is not able to catch this case).

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

501 ↗

(On Diff #219138)

nit: the comment is out-of-date with this patch.

[ilya-biryukov]

• Remove stale comment

clang-tools-extra/clangd/SemanticHighlighting.cpp

530 ↗

(On Diff #219138)

Ack. Keeping as is, thanks for taking a look.
Is there a way for us to provide a custom color for this in our extension? Or would we just revert to the default color for entity.name.type?

[hokein]

clang-tools-extra/clangd/SemanticHighlighting.cpp

530 ↗

(On Diff #219138)

Is there a way for us to provide a custom color for this in our extension?

we don't support it yet, this is a feature request in https://github.com/clangd/clangd/issues/143.

Or would we just revert to the default color for entity.name.type?

Yes, we will fallback to the color for entity.name.type.

clang-tools-extra/trunk/clangd/SemanticHighlighting.h

enum class HighlightingKind {
   Function,
   Method,
   StaticMethod,
   Field,
   StaticField,
   Class,
   Enum,
   EnumConstant,
+  Typedef,
   Namespace,
   TemplateParameter,
   Primitive,
   Macro,
 
   LastKind = Macro
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K);

clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp

 #include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
 #include <algorithm>
 
 namespace clang {
 namespace clangd {
 namespace {
 
 // Collects all semantic tokens in an ASTContext.
 class HighlightingTokenCollector
if (Ref->getNameInfo().getName().getNameKind() !=
       // Only want to highlight identifiers.
       return true;
 
     addToken(Ref->getLocation(), Ref->getDecl());
     return true;
   }
 
   bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-    if (const auto *TSI = TD->getTypeSourceInfo())
-      addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
+    addTokenForTypedef(TD->getLocation(), TD);
     return true;
   }
 
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
-    addType(TL.getBeginLoc(), TL.getTypePtr());
+    addTokenForTypedef(TL.getBeginLoc(), TL.getTypedefNameDecl());
     return true;
   }
 
   bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
     if (const TemplateDecl *TD =
             TL.getTypePtr()->getTemplateName().getAsTemplateDecl())
       addToken(TL.getBeginLoc(), TD);
     return true;
if (auto *AT = D->getType()->getContainedAutoType()) {
       const auto Deduced = AT->getDeducedType();
       if (!Deduced.isNull())
         addType(D->getTypeSpecStartLoc(), Deduced.getTypePtr());
     }
     return true;
   }
 
 private:
-  void addType(SourceLocation Loc, const Type *TP) {
-    if (!TP)
+  void addTokenForTypedef(SourceLocation Loc, const TypedefNameDecl *TD) {
+    auto *TSI = TD->getTypeSourceInfo();
+    if (!TSI)
+      return;
+    // Try to highlight as underlying type.
+    if (addType(Loc, TSI->getType().getTypePtrOrNull()))
       return;
-    if (TP->isBuiltinType())
+    // Fallback to the typedef highlighting kind.
+    addToken(Loc, HighlightingKind::Typedef);
+  }
+
+  bool addType(SourceLocation Loc, const Type *TP) {
+    if (!TP)
+      return false;
+    if (TP->isBuiltinType()) {
       // Builtins must be special cased as they do not have a TagDecl.
       addToken(Loc, HighlightingKind::Primitive);
-    if (auto *TD = dyn_cast<TemplateTypeParmType>(TP))
+      return true;
+    }
+    if (auto *TD = dyn_cast<TemplateTypeParmType>(TP)) {
       // TemplateTypeParmType also do not have a TagDecl.
       addToken(Loc, TD->getDecl());
-    if (auto *TD = TP->getAsTagDecl())
+      return true;
+    }
+    if (auto *TD = TP->getAsTagDecl()) {
       addToken(Loc, TD);
+      return true;
+    }
+    return false;
   }
 
   void addToken(SourceLocation Loc, const NamedDecl *D) {
     if (D->getDeclName().isIdentifier() && D->getName().empty())
       // Don't add symbols that don't have any length.
       return;
     // We highlight class decls, constructor decls and destructor decls as
     // `Class` type. The destructor decls are handled in `VisitTypeLoc` (we will
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K) {
   case HighlightingKind::StaticField:
     return OS << "StaticField";
   case HighlightingKind::Class:
     return OS << "Class";
   case HighlightingKind::Enum:
     return OS << "Enum";
   case HighlightingKind::EnumConstant:
     return OS << "EnumConstant";
+  case HighlightingKind::Typedef:
+    return OS << "Typedef";
   case HighlightingKind::Namespace:
     return OS << "Namespace";
   case HighlightingKind::TemplateParameter:
     return OS << "TemplateParameter";
   case HighlightingKind::Primitive:
     return OS << "Primitive";
   case HighlightingKind::Macro:
     return OS << "Macro";
llvm::StringRef toTextMateScope(HighlightingKind Kind) {
   case HighlightingKind::StaticField:
     return "variable.other.field.static.cpp";
   case HighlightingKind::Class:
     return "entity.name.type.class.cpp";
   case HighlightingKind::Enum:
     return "entity.name.type.enum.cpp";
   case HighlightingKind::EnumConstant:
     return "variable.other.enummember.cpp";
+  case HighlightingKind::Typedef:
+    return "entity.name.type.typedef.cpp";
   case HighlightingKind::Namespace:
     return "entity.name.namespace.cpp";
   case HighlightingKind::TemplateParameter:
     return "entity.name.type.template.cpp";
   case HighlightingKind::Primitive:
     return "storage.type.primitive.cpp";
   case HighlightingKind::Macro:
     return "entity.name.function.preprocessor.cpp";
   }
   llvm_unreachable("unhandled HighlightingKind");
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/test/semantic-highlighting.test

 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.type.enum.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "variable.other.enummember.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
+# CHECK-NEXT:            "entity.name.type.typedef.cpp"
+# CHECK-NEXT:          ],
+# CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.namespace.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.type.template.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "storage.type.primitive.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.function.preprocessor.cpp"
 # CHECK-NEXT:          ]
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"int x = 2;"}}}
 #      CHECK:  "method": "textDocument/semanticHighlighting",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "lines": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "line": 0,
-# CHECK-NEXT:        "tokens": "AAAAAAADAA0AAAAEAAEAAA=="
+# CHECK-NEXT:        "tokens": "AAAAAAADAA4AAAAEAAEAAA=="
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "textDocument": {
 # CHECK-NEXT:      "uri": "file:///clangd-test/foo.cpp"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo2.cpp","languageId":"cpp","version":1,"text":"int x = 2;\nint y = 2;"}}}
 #      CHECK:  "method": "textDocument/semanticHighlighting",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "lines": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "line": 0,
-# CHECK-NEXT:        "tokens": "AAAAAAADAA0AAAAEAAEAAA=="
+# CHECK-NEXT:        "tokens": "AAAAAAADAA4AAAAEAAEAAA=="
 # CHECK-NEXT:      }
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "line": 1,
-# CHECK-NEXT:        "tokens": "AAAAAAADAA0AAAAEAAEAAA=="
+# CHECK-NEXT:        "tokens": "AAAAAAADAA4AAAAEAAEAAA=="
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "textDocument": {
 # CHECK-NEXT:      "uri": "file:///clangd-test/foo2.cpp"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.cpp","version":2},"contentChanges": [{"range":{"start": {"line": 0,"character": 10},"end": {"line": 0,"character": 10}},"rangeLength": 0,"text": "\nint y = 2;"}]}}
 #      CHECK:  "method": "textDocument/semanticHighlighting",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "lines": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "line": 1,
-# CHECK-NEXT:        "tokens": "AAAAAAADAA0AAAAEAAEAAA=="
+# CHECK-NEXT:        "tokens": "AAAAAAADAA4AAAAEAAEAAA=="
 # CHECK-NEXT:      }
 # CHECK-NEXT:   ],
 # CHECK-NEXT:    "textDocument": {
 # CHECK-NEXT:      "uri": "file:///clangd-test/foo.cpp"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
 ---

clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

const char *TestCases[] = {
     )cpp",
       R"cpp(
       template<class $TemplateParameter[[T]]>
       class $Class[[A]] {
         using $TemplateParameter[[TemplateParam1]] = $TemplateParameter[[T]];
         typedef $TemplateParameter[[T]] $TemplateParameter[[TemplateParam2]];
         using $Primitive[[IntType]] = $Primitive[[int]];
 
-        // These typedefs are not yet highlighted, their types are complicated.
-        using Pointer = $TemplateParameter[[T]] *;
-        using LVReference = $TemplateParameter[[T]] &;
-        using RVReference = $TemplateParameter[[T]]&&;
-        using Array = $TemplateParameter[[T]]*[3];
-        using MemberPointer = $Primitive[[int]] (A::*)($Primitive[[int]]);
+        using $Typedef[[Pointer]] = $TemplateParameter[[T]] *;
+        using $Typedef[[LVReference]] = $TemplateParameter[[T]] &;
+        using $Typedef[[RVReference]] = $TemplateParameter[[T]]&&;
+        using $Typedef[[Array]] = $TemplateParameter[[T]]*[3];
+        using $Typedef[[MemberPointer]] = $Primitive[[int]] (A::*)($Primitive[[int]]);
+
+        // Use various previously defined typedefs in a function type.
+        $Primitive[[void]] $Method[[func]](
+          $Typedef[[Pointer]], $Typedef[[LVReference]], $Typedef[[RVReference]],
+          $Typedef[[Array]], $Typedef[[MemberPointer]]);
       };
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);
   }
 
   checkHighlightings(R"cpp(
     class $Class[[A]] {
EXPECT_THAT(ActualDiff,
               testing::UnorderedElementsAre(
                   testing::AllOf(LineNumber(3), EmptyHighlightings()),
                   testing::AllOf(LineNumber(4), EmptyHighlightings())));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang
 No newline at end of file