ilya-biryukov 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
----------------
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?


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

Reply via email to