[PATCH] D130811: [Clang] Fix handling of Max from getValueRange(...) in IntExprEvaluator::VisitCastExpr(...)

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: erichkeane, aaron.ballman, tahonermann, thakis.
Herald added a project: All.
shafik requested review of this revision.

This is a follow-up to D130058 <https://reviews.llvm.org/D130058> to fix how we handle the `Max` value we obtain from `getValueRange(...)` in `IntExprEvaluator::VisitCastExpr(...)` which in the case of an enum that contains an enumerator with the max integer value will overflow by one.

The fix is to decrement the value of `Max` and use `slt` and `ult` for comparison Vs `sle` and `ule`.

`getValueRange(...)` is also used by `CodeGenFunction::EmitScalarRangeCheck(...)` through `getRangeForType(...)` and this code does generate the correct IR for these values even though it appears to be logically doing the same operations that does not work in `IntExprEvaluator::VisitCastExpr(...)`.


https://reviews.llvm.org/D130811

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===================================================================
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2414,35 +2414,41 @@
 // Enum with fixed underlying type because it is scoped
 enum class EScoped {escoped1=-4, escoped2=4};
 
+enum EMaxInt {emaxint1=-1, emaxint2=__INT_MAX__};
+
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast<E1>(-8);
   constexpr E1 x2 = static_cast<E1>(8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note at -1 {{integer value 8 is outside the valid range of values [-8, 8) for this enumeration type}}
+  // expected-note at -1 {{integer value 8 is outside the valid range of values [-8, 7] for this enumeration type}}
 
   constexpr E2 x3 = static_cast<E2>(-8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note at -1 {{integer value -8 is outside the valid range of values [0, 8) for this enumeration type}}
+  // expected-note at -1 {{integer value -8 is outside the valid range of values [0, 7] for this enumeration type}}
   constexpr E2 x4 = static_cast<E2>(0);
   constexpr E2 x5 = static_cast<E2>(8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note at -1 {{integer value 8 is outside the valid range of values [0, 8) for this enumeration type}}
+  // expected-note at -1 {{integer value 8 is outside the valid range of values [0, 7] for this enumeration type}}
 
   constexpr E3 x6 = static_cast<E3>(-2048);
   constexpr E3 x7 = static_cast<E3>(-8);
   constexpr E3 x8 = static_cast<E3>(0);
   constexpr E3 x9 = static_cast<E3>(8);
   constexpr E3 x10 = static_cast<E3>(2048); // expected-error {{must be initialized by a constant expression}}
-  // expected-note at -1 {{integer value 2048 is outside the valid range of values [-2048, 2048) for this enumeration type}}
+  // expected-note at -1 {{integer value 2048 is outside the valid range of values [-2048, 2047] for this enumeration type}}
 
   constexpr E4 x11 = static_cast<E4>(0);
   constexpr E4 x12 = static_cast<E4>(1);
   constexpr E4 x13 = static_cast<E4>(2); // expected-error {{must be initialized by a constant expression}}
-  // expected-note at -1 {{integer value 2 is outside the valid range of values [0, 2) for this enumeration type}}
+  // expected-note at -1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
 
   constexpr EEmpty x14 = static_cast<EEmpty>(0);
   constexpr EEmpty x15 = static_cast<EEmpty>(1);
   constexpr EEmpty x16 = static_cast<EEmpty>(2); // expected-error {{must be initialized by a constant expression}}
-  // expected-note at -1 {{integer value 2 is outside the valid range of values [0, 2) for this enumeration type}}
+  // expected-note at -1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
 
   constexpr EFixed x17 = static_cast<EFixed>(100);
   constexpr EScoped x18 = static_cast<EScoped>(100);
+
+  constexpr EMaxInt x19 = static_cast<EMaxInt>(__INT_MAX__-1);
+  constexpr EMaxInt x20 = static_cast<EMaxInt>((long)__INT_MAX__+1); // expected-error {{must be initialized by a constant expression}}
+  // expected-note at -1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
 }
 }
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13537,14 +13537,15 @@
         llvm::APInt Max;
 
         ED->getValueRange(Max, Min);
+        --Max;
 
         if (ED->getNumNegativeBits() &&
-            (Max.sle(Result.getInt().getSExtValue()) ||
+            (Max.slt(Result.getInt().getSExtValue()) ||
              Min.sgt(Result.getInt().getSExtValue())))
           CCEDiag(E, diag::note_constexpr_unscoped_enum_out_of_range)
               << Result.getInt() << Min.getSExtValue() << Max.getSExtValue();
         else if (!ED->getNumNegativeBits() &&
-                 Max.ule(Result.getInt().getZExtValue()))
+                 Max.ult(Result.getInt().getZExtValue()))
           CCEDiag(E, diag::note_constexpr_unscoped_enum_out_of_range)
               << Result.getInt() << Min.getZExtValue() << Max.getZExtValue();
       }
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -367,7 +367,7 @@
 def note_constexpr_unsupported_flexible_array : Note<
   "flexible array initialization is not yet supported">;
 def note_constexpr_unscoped_enum_out_of_range : Note<
-  "integer value %0 is outside the valid range of values [%1, %2) for this "
+  "integer value %0 is outside the valid range of values [%1, %2] for this "
   "enumeration type">;
 def err_experimental_clang_interp_failed : Error<
   "the experimental clang interpreter failed to evaluate an expression">;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,7 +60,7 @@
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Clang will now correctly diagnose as ill-formed a constant expression where an
   enum without a fixed underlying type is set to a value outside the range of
-  of the enumerations values. Fixes
+  the enumeration's values. Fixes
   `Issue 50055: <https://github.com/llvm/llvm-project/issues/50055>`_.
 
 Non-comprehensive list of changes in this release


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130811.448748.patch
Type: text/x-patch
Size: 5676 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220730/3c94c829/attachment-0001.bin>