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