[clang] 87aa318 - [clang] fix concepts crash on substitution failure during normalization

[Matheus Izvekov via cfe-commits]

Author: Matheus Izvekov
Date: 2021-07-28T23:28:45+02:00
New Revision: 87aa31827b293127619e2ef96e80baf709eae338

URL: https://github.com/llvm/llvm-project/commit/87aa31827b293127619e2ef96e80baf709eae338
DIFF: https://github.com/llvm/llvm-project/commit/87aa31827b293127619e2ef96e80baf709eae338.diff

LOG: [clang] fix concepts crash on substitution failure during normalization

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 <mizvekov at gmail.com>

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D106907

Added: 
    

Modified: 
    clang/lib/Sema/SemaConcept.cpp
    clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 30f0730671c0a..82443be09c062 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -742,22 +742,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),
+      return None;
+    *Conjunction = NormalizedConstraint(S.Context, std::move(*Conjunction),
                                         std::move(*Next), CCK_Conjunction);
-    *Conjunction = std::move(NewConjunction);
   }
   return Conjunction;
 }

diff  --git a/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp b/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
index 153d4a56bea31..2134968101470 100644
--- a/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
+++ b/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
@@ -67,3 +67,18 @@ namespace non_type_pack {
 
   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