hokein added a subscriber: rsmith.
hokein added inline comments.
================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1843
+ if (const auto *TC = D->getTypeConstraint()) {
+ TRY_TO(TraverseStmt(TC->getImmediatelyDeclaredConstraint()));
TRY_TO(TraverseConceptReference(*TC));
----------------
nridge wrote:
> hokein wrote:
> > Looks like we may visit some nodes in `ConceptReference` twice:
> > - getImmediatelyDeclaredConstraint returns a `ConceptSpecializationExpr`
> > (most cases?) which is a subclass of `ConceptReference`;
> > - `TraverseStmt(ConceptSpecializationExpr*)` will dispatch to
> > `TraverseConceptSpecializationExpr` which invokes
> > `TraverseConceptReference` (see Line 2719);
> >
> >
> > It is sad that we don't have enough test coverage, could you write some
> > tests in `clang/unittests/Tooling/RecursiveASTVisitorTests/`?
> It is true that there will be two calls to `TraverseConceptReference()`.
> However, they are called on two different `ConceptReference` objects:
>
> * the call in `TraverseConceptSpecializationExpr` will visit the base
> subobject of the `ConceptSpecializationExpr` (which inherits from
> `ConceptReference`)
> * the call in `TraverseTemplateTypeParmDecl` will visit the base subobject
> of the `TypeConstraint` (which also inherits from `ConceptReference`).
>
> So, I think this is fine -- there are two distinct `ConceptReference` objects
> in the AST, and with this patch we visit both of them.
I understand that they are two different `ConceptReference` objects, but they
have members (`FoundDecl`, `ArgsAsWritten`) that may refer to the same AST
nodes.
```
template <typename T, typename U>
concept binary_concept = true;
struct Foo {};
template<binary_concept<Foo> T> // the template argument Foo will be visited
twice.
void k2();
```
I'm not sure what's is the right approach here, I can see two options:
- traverse TC + immediately-declared-constraint expr, this seem to cause some
ast nodes visited twice (maybe not a big deal?)
- just traverse immediately-declared-constraint expr, this seems not breaking
any tests, but the immediately-declared-constraint expr could be nullptr (e.g.
broken code, missing required template arguments); or the
immediately-declared-constraint expr could be a `CXXFoldExpr`, which will make
some members in `ConceptReference` not be visited;
@rsmith, do you have any idea about this?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84136/new/
https://reviews.llvm.org/D84136
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
