hokein added a comment.
as you mentioned in the description, there are two problems, I'd prefer address
them in two different patches.
================
Comment at: clang/include/clang/AST/ExprConcepts.h:132
+ // there may not be a template argument list.
+ return ArgsAsWritten->RAngleLoc.isValid() ? ArgsAsWritten->RAngleLoc
+ : ConceptName.getEndLoc();
----------------
kadircet wrote:
> i think we should have some tests in clang, at least an ast-dump test in
> `clang/test/AST/` (for both cases) and possibly also in
> `clang/unittests/AST/SourceLocationTest.cpp`
+1, ast-dump should be enough to test the invalid end loc, I have a D84461 to
slightly improve the dump.
================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1843
+ if (const auto *TC = D->getTypeConstraint()) {
+ TRY_TO(TraverseStmt(TC->getImmediatelyDeclaredConstraint()));
TRY_TO(TraverseConceptReference(*TC));
----------------
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/`?
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
