hokein created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix a crash when evaluating a constexpr function which contains
recovery-exprs. https://bugs.llvm.org/show_bug.cgi?id=46837

Would be nice to have constant expression evaluator support general template
value-dependent expressions, but it requires more work.

This patch is a good start I think, to handle the error-only
value-dependent expressions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84637

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
  clang/test/SemaCXX/enable_if.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp

Index: clang/test/SemaCXX/recovery-expr-type.cpp
===================================================================
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -53,14 +53,12 @@
     return 2;
   }
   static constexpr int foo2() {
-    return AA<T>::getB(); // expected-error{{no matching function for call to 'getB'}} \
-                          // expected-note {{subexpression not valid in a constant expression}}
+    return AA<T>::getB(); // expected-error{{no matching function for call to 'getB'}}
   }
 };
 // FIXME: should we suppress the "be initialized by a constant expression" diagnostic?
 constexpr auto x2 = AA<int>::foo2(); // expected-error {{be initialized by a constant expression}} \
-                                     // expected-note {{in instantiation of member function}} \
-                                     // expected-note {{in call to}}
+                                     // expected-note {{in instantiation of member function}}
 }
 
 // verify no assertion failure on violating value category.
Index: clang/test/SemaCXX/enable_if.cpp
===================================================================
--- clang/test/SemaCXX/enable_if.cpp
+++ clang/test/SemaCXX/enable_if.cpp
@@ -415,7 +415,7 @@
 template <int N> constexpr int callTemplated() { return templated<N>(); }
 
 constexpr int B = 10 + // expected-error {{initialized by a constant expression}}
-    callTemplated<0>(); // expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}} expected-note {{in call to 'callTemplated()'}} expected-note@-3 {{subexpression not valid in a constant expression}}
+    callTemplated<0>(); // expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 
Index: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+
+// verify no value-dependent-assertion crash in constexpr function body.
+class Foo {
+  constexpr Foo() {
+    while (invalid()) {} // expected-error {{use of undeclared identifier}}
+    if (invalid()) {} // expected-error {{use of undeclared identifier}}
+  }
+};
+
+constexpr void test1() {
+  while (invalid()) {} // expected-error {{use of undeclared identifier}}
+  if (invalid()) {} // expected-error {{use of undeclared identifier}}
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -1868,6 +1868,7 @@
 /// result.
 /// \return \c true if the caller should keep evaluating.
 static bool EvaluateIgnoredValue(EvalInfo &Info, const Expr *E) {
+  assert(!E->isValueDependent());
   APValue Scratch;
   if (!Evaluate(Scratch, Info, E))
     // We don't need the value, but we might have skipped a side effect here.
@@ -2369,6 +2370,7 @@
 
 static bool EvaluateAsBooleanCondition(const Expr *E, bool &Result,
                                        EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && "missing lvalue-to-rvalue conv in bool condition");
   APValue Val;
   if (!Evaluate(Val, Info, E))
@@ -4611,6 +4613,7 @@
 /// Evaluate a condition (either a variable declaration or an expression).
 static bool EvaluateCond(EvalInfo &Info, const VarDecl *CondDecl,
                          const Expr *Cond, bool &Result) {
+  assert(!Cond->isValueDependent());
   FullExpressionRAII Scope(Info);
   if (CondDecl && !EvaluateDecl(Info, CondDecl))
     return false;
@@ -4868,6 +4871,10 @@
   switch (S->getStmtClass()) {
   default:
     if (const Expr *E = dyn_cast<Expr>(S)) {
+      if (E->isValueDependent()) {
+        assert(E->containsErrors());
+        return ESR_Failed;
+      }
       // Don't bother evaluating beyond an expression-statement which couldn't
       // be evaluated.
       // FIXME: Do we need the FullExpressionRAII object here?
@@ -4900,6 +4907,10 @@
   case Stmt::ReturnStmtClass: {
     const Expr *RetExpr = cast<ReturnStmt>(S)->getRetValue();
     FullExpressionRAII Scope(Info);
+    if (RetExpr && RetExpr->isValueDependent()) {
+      assert(RetExpr->containsErrors());
+      return ESR_Failed;
+    }
     if (RetExpr &&
         !(Result.Slot
               ? EvaluateInPlace(Result.Value, Info, *Result.Slot, RetExpr)
@@ -4940,6 +4951,10 @@
         return ESR;
       }
     }
+    if (IS->getCond()->isValueDependent()) {
+      assert(IS->getCond()->containsErrors());
+      return ESR_Failed;
+    }
     bool Cond;
     if (!EvaluateCond(Info, IS->getConditionVariable(), IS->getCond(), Cond))
       return ESR_Failed;
@@ -4958,6 +4973,10 @@
   case Stmt::WhileStmtClass: {
     const WhileStmt *WS = cast<WhileStmt>(S);
     while (true) {
+      if (WS->getCond()->isValueDependent()) {
+        assert(WS->getCond()->containsErrors());
+        return ESR_Failed;
+      }
       BlockScopeRAII Scope(Info);
       bool Continue;
       if (!EvaluateCond(Info, WS->getConditionVariable(), WS->getCond(),
@@ -4987,6 +5006,10 @@
         return ESR;
       Case = nullptr;
 
+      if (DS->getCond()->isValueDependent()) {
+        assert(DS->getCond()->containsErrors());
+        return ESR_Failed;
+      }
       FullExpressionRAII CondScope(Info);
       if (!EvaluateAsBooleanCondition(DS->getCond(), Continue, Info) ||
           !CondScope.destroy())
@@ -5007,6 +5030,10 @@
       }
     }
     while (true) {
+      if (FS->getCond() && FS->getCond()->isValueDependent()) {
+        assert(FS->getCond()->containsErrors());
+        return ESR_Failed;
+      }
       BlockScopeRAII IterScope(Info);
       bool Continue = true;
       if (FS->getCond() && !EvaluateCond(Info, FS->getConditionVariable(),
@@ -5073,6 +5100,10 @@
     while (true) {
       // Condition: __begin != __end.
       {
+        if (FS->getCond()->isValueDependent()) {
+          assert(FS->getCond()->containsErrors());
+          return ESR_Failed;
+        }
         bool Continue = true;
         FullExpressionRAII CondExpr(Info);
         if (!EvaluateAsBooleanCondition(FS->getCond(), Continue, Info))
@@ -5097,6 +5128,10 @@
           return ESR_Failed;
         return ESR;
       }
+      if (FS->getInc()->isValueDependent()) {
+        assert(FS->getInc()->containsErrors());
+        return ESR_Failed;
+      }
 
       // Increment: ++__begin
       if (!EvaluateIgnoredValue(Info, FS->getInc()))
@@ -7750,6 +7785,7 @@
 ///  * @selector() expressions in Objective-C
 static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
                            bool InvalidBaseOK) {
+  assert(!E->isValueDependent());
   assert(E->isGLValue() || E->getType()->isFunctionType() ||
          E->getType()->isVoidType() || isa<ObjCSelectorExpr>(E));
   return LValueExprEvaluator(Info, Result, InvalidBaseOK).Visit(E);
@@ -8280,6 +8316,7 @@
 
 static bool EvaluatePointer(const Expr* E, LValue& Result, EvalInfo &Info,
                             bool InvalidBaseOK) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->hasPointerRepresentation());
   return PointerExprEvaluator(Info, Result, InvalidBaseOK).Visit(E);
 }
@@ -9157,6 +9194,7 @@
 
 static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,
                                   EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isMemberPointerType());
   return MemberPointerExprEvaluator(Info, Result).Visit(E);
 }
@@ -9631,6 +9669,7 @@
 
 static bool EvaluateRecord(const Expr *E, const LValue &This,
                            APValue &Result, EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isRecordType() &&
          "can't evaluate expression as a record rvalue");
   return RecordExprEvaluator(Info, This, Result).Visit(E);
@@ -9686,6 +9725,7 @@
 
 /// Evaluate an expression of record type as a temporary.
 static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isRecordType());
   return TemporaryExprEvaluator(Info, Result).Visit(E);
 }
@@ -9972,6 +10012,7 @@
 
 static bool EvaluateArray(const Expr *E, const LValue &This,
                           APValue &Result, EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isArrayType() && "not an array rvalue");
   return ArrayExprEvaluator(Info, This, Result).Visit(E);
 }
@@ -9979,6 +10020,7 @@
 static bool EvaluateArrayNewInitList(EvalInfo &Info, LValue &This,
                                      APValue &Result, const InitListExpr *ILE,
                                      QualType AllocType) {
+  assert(!ILE->isValueDependent());
   assert(ILE->isRValue() && ILE->getType()->isArrayType() &&
          "not an array rvalue");
   return ArrayExprEvaluator(Info, This, Result)
@@ -9989,6 +10031,7 @@
                                           APValue &Result,
                                           const CXXConstructExpr *CCE,
                                           QualType AllocType) {
+  assert(!CCE->isValueDependent());
   assert(CCE->isRValue() && CCE->getType()->isArrayType() &&
          "not an array rvalue");
   return ArrayExprEvaluator(Info, This, Result)
@@ -10366,11 +10409,13 @@
 /// like char*).
 static bool EvaluateIntegerOrLValue(const Expr *E, APValue &Result,
                                     EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isIntegralOrEnumerationType());
   return IntExprEvaluator(Info, Result).Visit(E);
 }
 
 static bool EvaluateInteger(const Expr *E, APSInt &Result, EvalInfo &Info) {
+  assert(!E->isValueDependent());
   APValue Val;
   if (!EvaluateIntegerOrLValue(E, Val, Info))
     return false;
@@ -10392,6 +10437,7 @@
 
 static bool EvaluateFixedPoint(const Expr *E, APFixedPoint &Result,
                                EvalInfo &Info) {
+  assert(!E->isValueDependent());
   if (E->getType()->isFixedPointType()) {
     APValue Val;
     if (!FixedPointExprEvaluator(Info, Val).Visit(E))
@@ -10407,6 +10453,7 @@
 
 static bool EvaluateFixedPointOrInteger(const Expr *E, APFixedPoint &Result,
                                         EvalInfo &Info) {
+  assert(!E->isValueDependent());
   if (E->getType()->isIntegerType()) {
     auto FXSema = Info.Ctx.getFixedPointSemantics(E->getType());
     APSInt Val;
@@ -12026,6 +12073,7 @@
 static bool
 EvaluateComparisonBinaryOperator(EvalInfo &Info, const BinaryOperator *E,
                                  SuccessCB &&Success, AfterCB &&DoAfter) {
+  assert(!E->isValueDependent());
   assert(E->isComparisonOp() && "expected comparison operator");
   assert((E->getOpcode() == BO_Cmp ||
           E->getType()->isIntegralOrEnumerationType()) &&
@@ -13054,6 +13102,7 @@
 } // end anonymous namespace
 
 static bool EvaluateFloat(const Expr* E, APFloat& Result, EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isRealFloatingType());
   return FloatExprEvaluator(Info, Result).Visit(E);
 }
@@ -13287,6 +13336,7 @@
 
 static bool EvaluateComplex(const Expr *E, ComplexValue &Result,
                             EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isAnyComplexType());
   return ComplexExprEvaluator(Info, Result).Visit(E);
 }
@@ -13809,6 +13859,7 @@
 
 static bool EvaluateAtomic(const Expr *E, const LValue *This, APValue &Result,
                            EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isAtomicType());
   return AtomicExprEvaluator(Info, This, Result).Visit(E);
 }
@@ -13933,6 +13984,7 @@
 }
 
 static bool EvaluateVoid(const Expr *E, EvalInfo &Info) {
+  assert(!E->isValueDependent());
   assert(E->isRValue() && E->getType()->isVoidType());
   return VoidExprEvaluator(Info).Visit(E);
 }
@@ -13942,6 +13994,7 @@
 //===----------------------------------------------------------------------===//
 
 static bool Evaluate(APValue &Result, EvalInfo &Info, const Expr *E) {
+  assert(!E->isValueDependent());
   // In C, function designators are not lvalues, but we evaluate them as if they
   // are.
   QualType T = E->getType();
@@ -14052,6 +14105,7 @@
 /// EvaluateAsRValue - Try to evaluate this expression, performing an implicit
 /// lvalue-to-rvalue cast if it is an lvalue.
 static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {
+  assert(!E->isValueDependent());
   if (Info.EnableNewConstInterp) {
     if (!Info.Ctx.getInterpContext().evaluateAsRValue(Info, E, Result))
       return false;
@@ -14115,6 +14169,7 @@
 
 static bool EvaluateAsRValue(const Expr *E, Expr::EvalResult &Result,
                              const ASTContext &Ctx, EvalInfo &Info) {
+  assert(!E->isValueDependent());
   bool IsConst;
   if (FastEvaluateAsRValue(E, Result, Ctx, IsConst))
     return IsConst;
@@ -14126,6 +14181,7 @@
                           const ASTContext &Ctx,
                           Expr::SideEffectsKind AllowSideEffects,
                           EvalInfo &Info) {
+  assert(!E->isValueDependent());
   if (!E->getType()->isIntegralOrEnumerationType())
     return false;
 
@@ -14141,6 +14197,7 @@
                                  const ASTContext &Ctx,
                                  Expr::SideEffectsKind AllowSideEffects,
                                  EvalInfo &Info) {
+  assert(!E->isValueDependent());
   if (!E->getType()->isFixedPointType())
     return false;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to