jvikstrom added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:38
+ bool VisitNamedDecl(NamedDecl *ND) {
+ if (ND->getDeclName().isEmpty())
+ // Don't add symbols that don't have any length.
----------------
sammccall wrote:
> I think you might want to bail out (both here and in VisitDeclRefExpr) if the
> name kind isn't identifier.
>
> Reason is you're only coloring the token at location, and most of the other
> name kinds can span multiple tokens or otherwise need special consideration.
I must have missed the Identifier NameKind because I was first-hand looking for
something like that.
Thanks.
Are you aware of any testcase I could add for this by the way?
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+ }
+ if(isa<FunctionDecl>(D)) {
+ addToken(Loc, HighlightingKind::Function);
----------------
sammccall wrote:
> note that methods, constructors, and destructors inherit from functiondecl,
> so if you want to exclude/distinguish those, order matters here
I'm aware of that, but thanks for the heads up. Although should I add it in a
comment somewhere in the method? Also added an additional testcase for classes
and FIXMEs to the skip if statement in VisitNamedDecl.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64199/new/
https://reviews.llvm.org/D64199
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
