jvikstrom marked an inline comment as done.
jvikstrom added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177
return;
+ if (TP->isPointerType() || TP->isLValueReferenceType())
+ // When highlighting dependant template types the type can be a pointer
or
----------------
ilya-biryukov wrote:
> jvikstrom wrote:
> > ilya-biryukov wrote:
> > > jvikstrom wrote:
> > > > ilya-biryukov wrote:
> > > > > `RecursiveASTVisitor` also traverses the pointer and reference types,
> > > > > why does it not reach the inner `TemplateTypeParmType` in the cases
> > > > > you describe?
> > > > The D in `using D = ...` `typedef ... D` does not have a TypeLoc (at
> > > > least not one that is visited). Therefore we use the
> > > > VisitTypedefNameDecl (line 121) to get the location of `D` to be able
> > > > to highlight it. And we just send the typeLocs typeptr to addType
> > > > (which is a Pointer for `using D = T*;`)...
> > > >
> > > > But maybe we should get the underlying type before we call addType with
> > > > TypePtr? Just a while loop on line 123 basically (can we have multiple
> > > > PointerTypes nested in each other actually?)
> > > >
> > > > Even if we keep it in addType the comment is actually wrong, because it
> > > > obviously works when for the actual "type occurrences" for `D` (so will
> > > > fix that no matter what). This recursion will just make us add more
> > > > duplicate tokens...
> > > Could we investigate why `RecursiveASTVisitor` does not visit the
> > > `TypeLoc` of a corresponding decl?
> > > Here's the code from `RecursiveASTVisitor.h` that should do the trick:
> > > ```
> > > DEF_TRAVERSE_DECL(TypeAliasDecl, {
> > > TRY_TO(TraverseTypeLoc(D->getTypeSourceInfo()->getTypeLoc()));
> > > // We shouldn't traverse D->getTypeForDecl(); it's a result of
> > > // declaring the type alias, not something that was written in the
> > > // source.
> > > })
> > > ```
> > >
> > > If it doesn't, we are probably holding it wrong.
> > There just doesn't seem to be a TypeLoc for the typedef'ed Decl. We can
> > get the `T*` TypeLoc (with `D->getTypeSourceInfo()->getTypeLoc()`). But
> > there isn't one for `D`. Even the `D->getTypeForDecl` returns null.
> >
> > And I have no idea where I'd even start debugging that. Or if it's even a
> > bug
> >
> I may have misinterpreted the patch. Are we trying to add highlightings for
> the names of using aliases here? E.g. for the following range:
> ```
> template <class T>
> struct Foo {
> using [[D]] = T**;
> };
> ```
>
> Why isn't this handled in `VisitNamedDecl`?
> We don't seem to call this function for `TypedefNameDecl` at all and it
> actually weird. Is this because we attempt to highlight typedefs as their
> underlying types?
So currently using aliases and typedefs are highlighted the same as the
underlying type (in most cases). One case where they aren't is when the
underlying type is a template parameter (which is what this patch is trying to
solve).
> Why isn't this handled in VisitNamedDecl?
The Decl is actually visited in `VisitNamedDecl`, however as it is a
`TypeAliasDecl` which we do not have a check for in the addToken function it
will not get highlighted in that visit.
Actually, could add a check for `TypeAliasDecl` in `addToken` (should probably
be a check for `TypedefNameDecl` to cover both `using ...` and `typedef ...`)
and move the code from the `VisitTypedefNameDecl` to the `addToken` function
inside that check instead.
> We don't seem to call this function for TypedefNameDecl at all and it
> actually weird. Is this because we attempt to highlight typedefs as their
> underlying types?
Don't understand what you mean. What function?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66516/new/
https://reviews.llvm.org/D66516
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
