[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 4 08:03:17 PDT 2019
sammccall 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.
----------------
jvikstrom wrote:
> 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?
Such a testcase would ensure you're not coloring any part of `struct F { ~F(); }` as a method, or `operator <<` etc.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+ }
+ if(isa<FunctionDecl>(D)) {
+ addToken(Loc, HighlightingKind::Function);
----------------
jvikstrom wrote:
> 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.
I don't think it needs a comment, especially if you're not actually highlighting them (because they have weird DeclarationNames)
> FIXMEs to the skip if statement in VisitNamedDecl
I'm not actually sure there's anything to fix here - it's a bit hard to talk about constructor/destructor highlighting as distinct from type name highlighting in C++. If you want them highlighted as classes, then that should just start working when you start handling TypeLocs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64199/new/
https://reviews.llvm.org/D64199
More information about the cfe-commits
mailing list