vsapsai added a comment.
After looking at this change more, I was thinking about changing the title from
**how** to **what** you are doing. For example, something like "[AST] Accept
identical TypeConstraint referring to other template parameters." You can tweak
it as you know better what's going on but that is my general understanding of
the change.
Instead of my previous ideas with `Profile` I'm looking into how we are
handling constraints from different modules when they are specified in
`requires` clause. Because I wasn't able to cause errors with
template <__integer_like _Tp, typename Sentinel>
constexpr _Tp funcA(_Tp &&__t, Sentinel &&last) requires C<Sentinel, _Tp> {
return __t;
}
And it seems beneficial to handle constraints across modules in the same way.
Similar story with template parameters referring to other template parameters.
In D129068#3629217 <https://reviews.llvm.org/D129068#3629217>, @ChuanqiXu wrote:
>> And I have some ideas about the tests. It might be covered elsewhere but I'm
>> curious if there are any not-isSameTemplateParameter test cases?
>
> I am not sure what you mean about "not-isSameTemplateParameter test case".
For example, when I make the change
+#if 0
if (XID != YID)
return false;
+#endif
in `ASTContext::isSameTemplateParameter`, only "PCH/cxx2a-constraints.cpp" is
failing. So it looks like there aren't many tests verifying various
similar-but-slightly-different constrained templates are rejected. I agree that
some of such tests should have existed before your changes and I'm not asking
to test all possible kinds of mismatches. But we can still bolster the test
suite, I hope. Though specific tests might depend on the implementation, so you
don't have to rush with extra tests (unless you want to).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129068/new/
https://reviews.llvm.org/D129068
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
