hokein added a comment.
could you please also update the patch description? "non-builtin" types are not
precise, this patch only highlights the class and enum types.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:71
+
+ TagDecl *D = TL.getTypePtr()->getAsTagDecl();
+ if (!D)
----------------
nit: you could simplify the code like
```
if (const auto* D = TL.getXXX)
addToken(D->getLocation(), D);
return true;
```
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:81
void addToken(SourceLocation Loc, const Decl *D) {
+ if (isa<CXXConstructorDecl>(D)) {
+ addToken(Loc, HighlightingKind::Class);
----------------
nit: move this around `if (isa<RecordDecl>(D)) {` since they are related to
`Class`, and we should have a comment describing the highlighting behavior of
`class, constructor, and destructor`.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:87
+ typedef abc::$Class[[A]]<int> AAA;
+ enum class $Enum[[E]] {};
+ enum $Enum[[EE]] {};
----------------
could we split the enum case into a separate testcase?
Thinking it further, we may want to highlight the enumerator as well.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:101
+ R"cpp(
+ struct $Class[[A]] {
+ $Class[[A]]();
----------------
this test case can be merged into the above case (we could add
constructor/destructor to class B there)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64257/new/
https://reviews.llvm.org/D64257
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
