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

[Matheus Izvekov via Phabricator via cfe-commits]

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 <mizvekov at 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;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D106907.362137.patch
Type: text/x-patch
Size: 2519 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210727/fd59141a/attachment.bin>