[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 453389.
shafik marked an inline comment as done.
shafik added a comment.

- Added `consteval` test with default parameters


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

https://reviews.llvm.org/D131874

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===================================================================
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -865,3 +865,15 @@
 }
 
 } // namespace multiple_default_constructors
+
+namespace GH50055 {
+enum E {e1=0, e2=1};
+consteval int testDefaultArgForParam(E eParam = (E)-1) {
+// expected-error at -1 {{integer value -1 is outside the valid range of values [0, 1] for this enumeration type}}
+  return (int)eParam;
+}
+
+int test() {
+  return testDefaultArgForParam() + testDefaultArgForParam((E)1);
+}
+}
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===================================================================
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2419,6 +2419,13 @@
 
 enum EMaxInt {emaxint1=-1, emaxint2=__INT_MAX__};
 
+enum NumberType {};
+
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant expression context
+  E2 e2LocalInit = e2Param; // ok, not a constant expression context
+  return e2LocalInit;
+}
+
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast<E1>(-8);
   constexpr E1 x2 = static_cast<E1>(8);
@@ -2454,6 +2461,8 @@
   constexpr EMaxInt x19 = static_cast<EMaxInt>(__INT_MAX__-1);
   constexpr EMaxInt x20 = static_cast<EMaxInt>((long)__INT_MAX__+1);
   // expected-error at -1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
+
+  const NumberType neg_one = (NumberType) ((NumberType) 0 - (NumberType) 1); // ok, not a constant expression context
 }
 
 enum SortOrder {
@@ -2470,3 +2479,8 @@
     return;
 }
 }
+
+GH50055::E2 GlobalInitNotCE1 = (GH50055::E2)-1; // ok, not a constant expression context
+GH50055::E2 GlobalInitNotCE2 = GH50055::testDefaultArgForParam(); // ok, not a constant expression context
+constexpr GH50055::E2 GlobalInitCE = (GH50055::E2)-1;
+// expected-error at -1 {{integer value -1 is outside the valid range of values [0, 7] for this enumeration type}}
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13536,6 +13536,18 @@
     if (Info.Ctx.getLangOpts().CPlusPlus && Info.InConstantContext &&
         Info.EvalMode == EvalInfo::EM_ConstantExpression &&
         DestType->isEnumeralType()) {
+
+      bool ConstexprVar = true;
+
+      // We know if we are here that we are in a context that we might require
+      // a constant expression or a context that requires a constant
+      // value. But if we are initializing a value we don't know if it is a
+      // constexpr variable or not. We can check the EvaluatingDecl to determine
+      // if it constexpr or not. If not then we don't want to emit a diagnostic.
+      if (const auto *VD = dyn_cast_or_null<VarDecl>(
+              Info.EvaluatingDecl.dyn_cast<const ValueDecl *>()))
+        ConstexprVar = VD->isConstexpr();
+
       const EnumType *ET = dyn_cast<EnumType>(DestType.getCanonicalType());
       const EnumDecl *ED = ET->getDecl();
       // Check that the value is within the range of the enumeration values.
@@ -13555,13 +13567,14 @@
         ED->getValueRange(Max, Min);
         --Max;
 
-        if (ED->getNumNegativeBits() &&
+        if (ED->getNumNegativeBits() && ConstexprVar &&
             (Max.slt(Result.getInt().getSExtValue()) ||
              Min.sgt(Result.getInt().getSExtValue())))
-          Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
-                                       diag::warn_constexpr_unscoped_enum_out_of_range)
-	       << llvm::toString(Result.getInt(),10) << Min.getSExtValue() << Max.getSExtValue();
-        else if (!ED->getNumNegativeBits() &&
+          Info.Ctx.getDiagnostics().Report(
+              E->getExprLoc(), diag::warn_constexpr_unscoped_enum_out_of_range)
+              << llvm::toString(Result.getInt(), 10) << Min.getSExtValue()
+              << Max.getSExtValue();
+        else if (!ED->getNumNegativeBits() && ConstexprVar &&
                  Max.ult(Result.getInt().getZExtValue()))
           Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
                                        diag::warn_constexpr_unscoped_enum_out_of_range)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131874.453389.patch
Type: text/x-patch
Size: 4362 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220817/54b737c0/attachment-0001.bin>