jvikstrom added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:231
+ if (TP->isFunctionPointerType()) {
+ addToken(Loc, HighlightingKind::Function);
+ return;
----------------
ilya-biryukov wrote:
> jvikstrom wrote:
> > ilya-biryukov wrote:
> > > Why do we special-case template parameters, but not other kinds of
> > > variables?
> > > We definitely need a comment explaining why template parameters are
> > > handled in a special way, but variables, parameters, fields are not.
> > Not quite sure what you mean about variables/parameters/fields not being
> > handled in a special way.
> >
> > The reason for special casing non type templates is because it probably
> > gives more information/is more valuable to highlight a reference/pointer as
> > a variable rather than a normal template parameter (same for
> > methods/functions).
> >
> > But maybe they all should just be highlighted as with the TemplateParameter
> > kind instead?
> > Not quite sure what you mean about variables/parameters/fields not being
> > handled in a special way.
> Non-type template parameters are very similar to global and local variables,
> function parameters, class fields, etc.
> We could also match those on type and highlight differently based on the type.
>
> > The reason for special casing non type templates is because it probably
> > gives more information/is more valuable to highlight a reference/pointer as
> > a variable rather than a normal template parameter (same for
> > methods/functions).
> However, if a global variable has a function pointer type we do not highlight
> it as a function. Why would this be different?
>
> > But maybe they all should just be highlighted as with the TemplateParameter
> > kind instead?
> I would personally vouch for this option. The highlighting functionality lets
> me understand what the name resolves to; if it's a template parameter and it
> would be highlighted as a variable instead, this would create confusion on my
> end. If I need to know the type I'll look at completion results or hover.
>
> It is my personal preference and I'm ok with this going in a different
> direction if you feel the opposite is better. Just asking to put the
> rationale of this decision (why do this only for template parameters and not
> other things) in a comment somewhere in the source code.
> I would personally vouch for this option. The highlighting functionality lets
> me understand what the name resolves to;
Very true actually, was a bit unsure what was the correct way to go but this is
very true actually.
Changing to just highlight as TemplateParameter.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66221/new/
https://reviews.llvm.org/D66221
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
