[PATCH] D142871: [clangd] Semantic highlighting for constrained-parameter

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a subscriber: usaxena95.
kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:720
+      // Pick up the name via VisitNamedDecl
+      Base::VisitTemplateTypeParmDecl(D);
+    }
----------------
nridge wrote:
> Am I using the API correctly here? It's a bit weird that `ConstDeclVisitor` would work differently from `RecursiveASTVisitor` in that VisitXXX methods for super-classes are not automatically called
right, non-recursive ast visitors are mere helpers for visiting most specific type of an ast node which is interesting to the visitor (i.e. visit method is overridden), and only that. they also don't traverse children of an entitie's entries.

which usually hints towards this being the wrong place to handle such constructs. we actually have an outer recursiveastvisitor, that tries to visit each children. but concept references seem to be missing from there.
taking a look at the RecursiveASTVisitor pieces around this piece, we actually do visit the children appropriately:
- https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/RecursiveASTVisitor.h#L1931 calls traverse on the constraint
- https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/RecursiveASTVisitor.h#L1923 jumps into type constraint
- https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/RecursiveASTVisitor.h#L510 some more indirection
- https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/RecursiveASTVisitor.h#L547 calls visit for the decl name
- https://github.com/llvm/llvm-project/blob/clang/include/clang/AST/RecursiveASTVisitor.h#L830 unfortunately we stop here, because all we store is an identifier.

We should probably figure out how to properly visit type constraints inside the RecursiveASTVisitor (i guess we should actually be visiting TypeConstraints). @usaxena95 who's looking at C++20 features and their interactions with source tooling.

In the meanwhile, i think we should have this specialization at the outer layer, in `ExplicitReferenceCollector`, with something like:
```
bool TraverseTemplateTypeParamDeclConstraints(TemplateTypeParmDecl *TTPD) {
  if (auto *TC = TTPD->getTypeConstraint()) {
    reportReference(ReferenceLoc{TC->getNestedNameSpecifierLoc(),
                                 TC->getConceptNameLoc(),
                                 /*IsDecl=*/false,
                                 {TC->getNamedConcept()}},
                    DynTypedNode::create(*TTPD));
    return RecursiveASTVisitor::TraverseTypeConstraint(TC);
  }
  return true;
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142871/new/

https://reviews.llvm.org/D142871