adamcz created this revision.
adamcz added a reviewer: ilya-biryukov.
Herald added a project: All.
adamcz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The code would assume that SubstExpr() cannot fail on concept
specialization. This is incorret - we give up on some things after fatal
error occurred, since there's no value in doing futher work that the
user will not see anyway. In this case, this lead to crash.

The fatal error is simulated in tests with -ferror-limit=1, but this
could happen in other cases too.

Fixes https://github.com/llvm/llvm-project/issues/55401


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129499

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/concept-fatal-error.cpp


Index: clang/test/SemaCXX/concept-fatal-error.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/concept-fatal-error.cpp
@@ -0,0 +1,11 @@
+// RUN: not %clang_cc1 -fsyntax-only -std=c++20 -ferror-limit 1 %s 2>&1 | 
FileCheck %s
+
+template <class>
+concept f = requires { 42; };
+struct h {
+  // The missing semicolon will trigger an error and -ferror-limit=1 will make 
it fatal
+  // We test that we do not crash in such cases (#55401)
+  int i = requires { { i } f } // expected-error {{expected ';' at end of 
declaration list}}
+
+  // CHECK: fatal error: too many errors emitted, stopping now
+};
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8971,14 +8971,14 @@
         cast<TemplateTypeParmDecl>(TPL->getParam(0))->getTypeConstraint()
             ->getImmediatelyDeclaredConstraint();
     ExprResult Constraint = SubstExpr(IDC, MLTAL);
-    assert(!Constraint.isInvalid() &&
-           "Substitution cannot fail as it is simply putting a type template "
-           "argument into a concept specialization expression's parameter.");
-
-    SubstitutedConstraintExpr =
-        cast<ConceptSpecializationExpr>(Constraint.get());
-    if (!SubstitutedConstraintExpr->isSatisfied())
-      Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
+    if (Constraint.isInvalid()) {
+      Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
+    } else {
+      SubstitutedConstraintExpr =
+          cast<ConceptSpecializationExpr>(Constraint.get());
+      if (!SubstitutedConstraintExpr->isSatisfied())
+        Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
+    }
   }
   return new (Context) concepts::ExprRequirement(E, IsSimple, NoexceptLoc,
                                                  ReturnTypeRequirement, Status,


Index: clang/test/SemaCXX/concept-fatal-error.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/concept-fatal-error.cpp
@@ -0,0 +1,11 @@
+// RUN: not %clang_cc1 -fsyntax-only -std=c++20 -ferror-limit 1 %s 2>&1 | FileCheck %s
+
+template <class>
+concept f = requires { 42; };
+struct h {
+  // The missing semicolon will trigger an error and -ferror-limit=1 will make it fatal
+  // We test that we do not crash in such cases (#55401)
+  int i = requires { { i } f } // expected-error {{expected ';' at end of declaration list}}
+
+  // CHECK: fatal error: too many errors emitted, stopping now
+};
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8971,14 +8971,14 @@
         cast<TemplateTypeParmDecl>(TPL->getParam(0))->getTypeConstraint()
             ->getImmediatelyDeclaredConstraint();
     ExprResult Constraint = SubstExpr(IDC, MLTAL);
-    assert(!Constraint.isInvalid() &&
-           "Substitution cannot fail as it is simply putting a type template "
-           "argument into a concept specialization expression's parameter.");
-
-    SubstitutedConstraintExpr =
-        cast<ConceptSpecializationExpr>(Constraint.get());
-    if (!SubstitutedConstraintExpr->isSatisfied())
-      Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
+    if (Constraint.isInvalid()) {
+      Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
+    } else {
+      SubstitutedConstraintExpr =
+          cast<ConceptSpecializationExpr>(Constraint.get());
+      if (!SubstitutedConstraintExpr->isSatisfied())
+        Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
+    }
   }
   return new (Context) concepts::ExprRequirement(E, IsSimple, NoexceptLoc,
                                                  ReturnTypeRequirement, Status,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to