mizvekov created this revision.
mizvekov edited the summary of this revision.
mizvekov added a reviewer: rsmith.
mizvekov added a subscriber: Quuxplusone.
mizvekov published this revision for review.
mizvekov added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This partially fixes https://bugs.llvm.org/show_bug.cgi?id=47174
It fixes the crash, but there is however a remaining issue, the bogus template
lookup (`no template named 'Y'`) diagnostic from @Quuxplusone's reduced test
case is still there.
So far as I have seen though, it is only a diagnostic issue.
I can't find an alternate test case with valid code (non ambiguous overload
resolution or otherwise) that triggers that kind of error.
When substitution failed on the first constrained template argument (but
only the first), we would assert / crash. Checking for failure was only
being performed from the second constraint on.
This changes it so the checking is performed in that case,
and the code is also now simplified a little bit to hopefully
avoid this confusion.
Signed-off-by: Matheus Izvekov <mizve...@gmail.com>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D106907
Files:
clang/lib/Sema/SemaConcept.cpp
clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
Index: clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
===================================================================
--- clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
+++ clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
@@ -67,3 +67,14 @@
static_assert((foo<1>(), true));
}
+
+namespace PR47174 {
+ // This checks that we don't crash with a failed substitution on the first
constrained argument when
+ // performing normalization.
+ template<Bar2 T, True U> requires true struct S3; // expected-note
{{template is declared here}}
+ template<True T, True U> requires true struct S3<T, U>; // expected-error
{{class template partial specialization is not more specialized than the
primary template}}
+
+ // Same as above, for the second position (but this was already working).
+ template<True T, Bar2 U> requires true struct S4; // expected-note
{{template is declared here}}
+ template<True T, True U> requires true struct S4<T, U>; // expected-error
{{class template partial specialization is not more specialized than the
primary template}}
+} // namespace PR47174
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -741,23 +741,15 @@
Optional<NormalizedConstraint>
NormalizedConstraint::fromConstraintExprs(Sema &S, NamedDecl *D,
ArrayRef<const Expr *> E) {
- assert(E.size() != 0);
- auto First = fromConstraintExpr(S, D, E[0]);
- if (E.size() == 1)
- return First;
- auto Second = fromConstraintExpr(S, D, E[1]);
- if (!Second)
+ auto Conjunction = fromConstraintExpr(S, D, E[0]);
+ if (!Conjunction)
return None;
- llvm::Optional<NormalizedConstraint> Conjunction;
- Conjunction.emplace(S.Context, std::move(*First), std::move(*Second),
- CCK_Conjunction);
- for (unsigned I = 2; I < E.size(); ++I) {
+ for (unsigned I = 1; I < E.size(); ++I) {
auto Next = fromConstraintExpr(S, D, E[I]);
if (!Next)
- return llvm::Optional<NormalizedConstraint>{};
- NormalizedConstraint NewConjunction(S.Context, std::move(*Conjunction),
- std::move(*Next), CCK_Conjunction);
- *Conjunction = std::move(NewConjunction);
+ return None;
+ Conjunction.emplace(S.Context, std::move(*Conjunction), std::move(*Next),
+ CCK_Conjunction);
}
return Conjunction;
}
Index: clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
===================================================================
--- clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
+++ clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
@@ -67,3 +67,14 @@
static_assert((foo<1>(), true));
}
+
+namespace PR47174 {
+ // This checks that we don't crash with a failed substitution on the first constrained argument when
+ // performing normalization.
+ template<Bar2 T, True U> requires true struct S3; // expected-note {{template is declared here}}
+ template<True T, True U> requires true struct S3<T, U>; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+ // Same as above, for the second position (but this was already working).
+ template<True T, Bar2 U> requires true struct S4; // expected-note {{template is declared here}}
+ template<True T, True U> requires true struct S4<T, U>; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+} // namespace PR47174
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -741,23 +741,15 @@
Optional<NormalizedConstraint>
NormalizedConstraint::fromConstraintExprs(Sema &S, NamedDecl *D,
ArrayRef<const Expr *> E) {
- assert(E.size() != 0);
- auto First = fromConstraintExpr(S, D, E[0]);
- if (E.size() == 1)
- return First;
- auto Second = fromConstraintExpr(S, D, E[1]);
- if (!Second)
+ auto Conjunction = fromConstraintExpr(S, D, E[0]);
+ if (!Conjunction)
return None;
- llvm::Optional<NormalizedConstraint> Conjunction;
- Conjunction.emplace(S.Context, std::move(*First), std::move(*Second),
- CCK_Conjunction);
- for (unsigned I = 2; I < E.size(); ++I) {
+ for (unsigned I = 1; I < E.size(); ++I) {
auto Next = fromConstraintExpr(S, D, E[I]);
if (!Next)
- return llvm::Optional<NormalizedConstraint>{};
- NormalizedConstraint NewConjunction(S.Context, std::move(*Conjunction),
- std::move(*Next), CCK_Conjunction);
- *Conjunction = std::move(NewConjunction);
+ return None;
+ Conjunction.emplace(S.Context, std::move(*Conjunction), std::move(*Next),
+ CCK_Conjunction);
}
return Conjunction;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
