[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

[Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 537928.
yronglin marked an inline comment as done.
yronglin added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153296/new/

https://reviews.llvm.org/D153296

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-function-recovery-crash.cpp


Index: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
===================================================================
--- clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
+++ clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
@@ -87,6 +87,7 @@
 // We're not checking specific recovery here so don't assert diagnostics.
 TEST_EVALUATE(Switch, switch (!!){});              // expected-error + {{}}
 TEST_EVALUATE(SwitchInit, switch (auto x = !!){}); // expected-error + {{}}
+TEST_EVALUATE(SwitchCondValDep, switch (invalid_value) { default: break; });   
 // expected-error + {{}}
 TEST_EVALUATE(For, for (!!){}); // expected-error + {{}}
                                 // FIXME: should bail out instead of looping.
                                 // expected-note@-2 + {{infinite loop}}
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5007,12 +5007,13 @@
         !EvaluateDecl(Info, SS->getConditionVariable()))
       return ESR_Failed;
     if (SS->getCond()->isValueDependent()) {
-      if (!EvaluateDependentExpr(SS->getCond(), Info))
-        return ESR_Failed;
-    } else {
-      if (!EvaluateInteger(SS->getCond(), Value, Info))
-        return ESR_Failed;
+      // We don't know what the value is, and which branch should jump to.
+      EvaluateDependentExpr(SS->getCond(), Info);
+      return ESR_Failed;
     }
+    if (!EvaluateInteger(SS->getCond(), Value, Info))
+        return ESR_Failed;
+   
     if (!CondScope.destroy())
       return ESR_Failed;
   }
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -568,6 +568,9 @@
 - Clang now correctly evaluates ``__has_extension (cxx_defaulted_functions)``
   and ``__has_extension (cxx_default_function_template_args)`` to 1.
   (`#61758 <https://github.com/llvm/llvm-project/issues/61758>`_)
+- Stop evaluating a constant expression if the condition expression which in
+  switch statement contains errors.
+  (`#63453 <https://github.com/llvm/llvm-project/issues/63453>_`)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^



Index: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
===================================================================
--- clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
+++ clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
@@ -87,6 +87,7 @@
 // We're not checking specific recovery here so don't assert diagnostics.
 TEST_EVALUATE(Switch, switch (!!){});              // expected-error + {{}}
 TEST_EVALUATE(SwitchInit, switch (auto x = !!){}); // expected-error + {{}}
+TEST_EVALUATE(SwitchCondValDep, switch (invalid_value) { default: break; });    // expected-error + {{}}
 TEST_EVALUATE(For, for (!!){}); // expected-error + {{}}
                                 // FIXME: should bail out instead of looping.
                                 // expected-note@-2 + {{infinite loop}}
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5007,12 +5007,13 @@
         !EvaluateDecl(Info, SS->getConditionVariable()))
       return ESR_Failed;
     if (SS->getCond()->isValueDependent()) {
-      if (!EvaluateDependentExpr(SS->getCond(), Info))
-        return ESR_Failed;
-    } else {
-      if (!EvaluateInteger(SS->getCond(), Value, Info))
-        return ESR_Failed;
+      // We don't know what the value is, and which branch should jump to.
+      EvaluateDependentExpr(SS->getCond(), Info);
+      return ESR_Failed;
     }
+    if (!EvaluateInteger(SS->getCond(), Value, Info))
+        return ESR_Failed;
+   
     if (!CondScope.destroy())
       return ESR_Failed;
   }
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -568,6 +568,9 @@
 - Clang now correctly evaluates ``__has_extension (cxx_defaulted_functions)``
   and ``__has_extension (cxx_default_function_template_args)`` to 1.
   (`#61758 <https://github.com/llvm/llvm-project/issues/61758>`_)
+- Stop evaluating a constant expression if the condition expression which in
+  switch statement contains errors.
+  (`#63453 <https://github.com/llvm/llvm-project/issues/63453>_`)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits