Add 128-bit integer support to enum element

[zhouyizhou]

Summary

Add 128-bit integer support to enum element like GCC extension do.

Also remove comment: "TODO" from the code. This comment is added by 4ef40013d75ca ("Implement capturing of enum values and chaining of enums together"), while 6791a0d43b681 ("Improve handling of enumerator values for C and C++") has accomplished that "TODO".

I don't have write access to LLVM

Signed-off-by: Zhouyi Zhou <[e-mail]>

Thanks a lot
Zhouyi

[zhouyizhou]

add assertion for int128 support

[zhouyizhou]

debian build bot won't let me pass, so I clang-formatted the function ActOnEnumBody

[ChuanqiXu]

debian build bot won't let me pass, so I clang-formatted the function ActOnEnumBody

You can try git diff -U0 --no-color --relative HEAD^ | clang/tools/clang-format/clang-format-diff.py -p1 -i to format the changed part only. Currently there are many untouched part changed due to the format. This is not good.

[zhouyizhou]

In D144157#4131103, @ChuanqiXu wrote:

debian build bot won't let me pass, so I clang-formatted the function ActOnEnumBody

You can try git diff -U0 --no-color --relative HEAD^ | clang/tools/clang-format/clang-format-diff.py -p1 -i to format the changed part only. Currently there are many untouched part changed due to the format. This is not good.

Thank you for your valuable advice, the command is fantastic indeed. I will resend the patch immediately.
Thanks again
Cheers Zhouyi

[zhouyizhou]

reformat the patch according to Chuanqi's Advice

[MaskRay]

We also need a codegen test. clang/test/CodeGen/enum2.c may be a good one. It tests that debug information is correct as well.

[zhouyizhou]

Add CodeGen Test following MaskRay(Fangrui Song)'s Advice, I learned a lot during this process, to be honest ;-)

[MaskRay]

clang/test/CodeGen/enum3.c

1 ↗

(On Diff #498520)

We don't need a new test file. Reusing can improve test discoverability and readability for this test.

I think this test needs to show how the global variable v's value is changed by the __uint128_t enum, which enum2.c may lack (a problem of many old tests).

[zhouyizhou]

make test case even better according to MaskRay's guidance

[MaskRay]

clang/test/CodeGen/enum2.c

44

delete the trailing blank line

[zhouyizhou]

delete the trailing blank line

Thank you all ;-)

I have no access write to LLVM, could you commit it for me when you are not busy?

Thanks a lot
Cheers
Zhouyi

Zhouyi Zhou <[e-mail]>

[h-vetinari]

clang/lib/Sema/SemaDecl.cpp

19572–19574

That comment is still relevant AFAICT, at least partially (the ISO C restrictions are still there, GCC still extends it)

[aaron.ballman]

I think this needs additional test coverage around _BitInt as those can be arbitrarily large depending on the target. Also, C2x made changes in this area to how we calculate what type to represent the enumerations in; we don't have to implement that support as part of this patch, but we should make sure what we're doing doesn't conflict with that future work.

(Also, please add a release note for the changes.)

clang/lib/Sema/SemaDecl.cpp

19572–19574

Not only is it still relevant, there were changes for C2x in this area. See https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2997.htm for more details.

19717–19718

Consider (in C2x):

enum E {
  Val = 0xFFFF'FFFF'FFFF'FFFF'FFFF'FFFF'FFFF'FFFF'1wb
};

19719–19721

Again, _BitInt.

[zhouyizhou]

Thank you all for your guidance!

Now I am starting to learn C2X N2997, this is a fruitful process of learning for me ;-)

Cheers
Zhouyi

clang/lib/Sema/SemaDecl.cpp

19572–19574

That comment is still relevant AFAICT, at least partially (the ISO C restrictions are still there, GCC still extends it)

Thank for reviewing the patch ;-)

I think clang (with -pedantic) has emit the warning in function Sema::CheckEnumConstant, which behave very similar to what GCC (with -pedantic) do.

Cheers
Zhouyi

[zhouyizhou]

Add 128-bit integer support to enum element like GCC extension do.
Also test coverage around _BitInt which can be arbitrarily large depending on the target.
Also leave room for improvements to C2X because C2x made changes in this area to how we calculate what type to represent the enumerations in.

Signed-off-by: Zhouyi Zhou <[e-mail]>

clang/lib/Sema/SemaDecl.cpp

else {
             }
           }
 
           // Cast to the underlying type.
           Val = ImpCastExprToType(Val, EltTy,
                                   EltTy->isBooleanType() ? CK_IntegralToBoolean
                                                          : CK_IntegralCast)
                     .get();
-        } else if (getLangOpts().CPlusPlus) {
+        } else if (getLangOpts().CPlusPlus || getLangOpts().C2x) {
           // C++11 [dcl.enum]p5:
           //   If the underlying type is not fixed, the type of each enumerator
           //   is the type of its initializing value:
           //     - If an initializer is specified for an enumerator, the
           //       initializing value has the same type as the expression.
+
+          // C2x   6.7.2.2p2:
+          //   For all the integer constant expressions which make up the values
+          //   of the enumeration constant, there shall be an
+          //   implementation-defined signed or unsigned integer type (excluding
+          //   the bit-precise integer types) capable of representing all of the
+          //   values.
           EltTy = Val->getType();
         } else {
           // C99 6.7.2.2p2:
           //   The expression that defines the value of an enumeration constant
           //   shall be an integer constant expression that has a value
           //   representable as an int.
 
           // Complain if the value is not representable in an int.
for (unsigned i = 0, e = Elements.size(); i != e; ++i) {
 
       ECD->setType(EnumType);
     }
 
     Enum->completeDefinition(Context.DependentTy, Context.DependentTy, 0, 0);
     return;
   }
 
   // TODO: If the result value doesn't fit in an int, it must be a long or long
   // long value.  ISO C does not support this, but GCC does as an extension,
   // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
 
   // Verify that all the values are okay, compute the size of the values, and
   // reverse the list.
   unsigned NumNegativeBits = 0;
   unsigned NumPositiveBits = 0;
if (Packed && NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) {
       BestWidth = IntWidth;
     } else {
       BestWidth = Context.getTargetInfo().getLongWidth();
 
       if (NumNegativeBits <= BestWidth && NumPositiveBits < BestWidth) {
         BestType = Context.LongTy;
       } else {
         BestWidth = Context.getTargetInfo().getLongLongWidth();
-
-        if (NumNegativeBits > BestWidth || NumPositiveBits >= BestWidth)
-          Diag(Enum->getLocation(), diag::ext_enum_too_large);
+        if (NumNegativeBits <= BestWidth && NumPositiveBits < BestWidth) {
           BestType = Context.LongLongTy;
+        } else {
+          BestWidth = 128;
+          // TODO: C2x   6.7.2.2p2:
+          // For all the integer constant expressions which make up the values
+          // of the enumeration constant, there shall be an
+          // implementation-defined signed or unsigned integer type (excluding
+          // the bit-precise integer types) capable of representing all of the
+          // values.
+          if (NumNegativeBits > 128 || NumPositiveBits >= 128)
+            Diag(Enum->getLocation(), diag::ext_enum_too_large);
+          BestType = Context.Int128Ty;
+        }
       }
     }
     BestPromotionType = (BestWidth <= IntWidth ? Context.IntTy : BestType);
   } else {
     // If there is no negative value, figure out the smallest type that fits
     // all of the enumerator values.
     // If it's packed, check also if it fits a char or a short.
     if (Packed && NumPositiveBits <= CharWidth) {
if (Packed && NumPositiveBits <= CharWidth) {
         = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
                            ? Context.UnsignedIntTy : Context.IntTy;
     } else if (NumPositiveBits <=
                (BestWidth = Context.getTargetInfo().getLongWidth())) {
       BestType = Context.UnsignedLongTy;
       BestPromotionType
         = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
                            ? Context.UnsignedLongTy : Context.LongTy;
-    } else {
-      BestWidth = Context.getTargetInfo().getLongLongWidth();
-      assert(NumPositiveBits <= BestWidth &&
-             "How could an initializer get larger than ULL?");
+    } else if (NumPositiveBits <=
+               (BestWidth = Context.getTargetInfo().getLongLongWidth())) {
       BestType = Context.UnsignedLongLongTy;
-      BestPromotionType
-        = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
-                           ? Context.UnsignedLongLongTy : Context.LongLongTy;
+      BestPromotionType =
+          (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+              ? Context.UnsignedLongLongTy
+              : Context.LongLongTy;
+    } else {
+      BestWidth = 128;
+      BestType = Context.UnsignedInt128Ty;
+      BestPromotionType =
+          (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+              ? Context.UnsignedInt128Ty
+              : Context.Int128Ty;
+      // TODO: C2x   6.7.2.2p2:
+      // For all the integer constant expressions which make up the values
+      // of the enumeration constant, there shall be an implementation-defined
+      // signed or unsigned integer type (excluding the bit-precise integer
+      // types) capable of representing all of the values.
+      if (NumPositiveBits > 128)
+        Diag(Enum->getLocation(), diag::ext_enum_too_large);
     }
   }
 
   // Loop over all of the enumerator constants, changing their types to match
   // the type of the enum if needed.
   for (auto *D : Elements) {
     auto *ECD = cast_or_null<EnumConstantDecl>(D);
     if (!ECD) continue;  // Already issued a diagnostic.

clang/test/AST/Interp/enums-targets.cpp

 // RUN: %clang_cc1 -triple i686-pc-linux -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -triple i686-pc-linux -verify %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux -fexperimental-new-constant-interpreter -verify=warn %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux -verify=warn %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -verify %s
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -verify %s
 // RUN: %clang_cc1 -triple hexagon -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -triple hexagon -verify %s
 
 // expected-no-diagnostics
 
 /// This test is split out from the rest since the output is target dependent.
 
-enum E { // warn-warning {{enumeration values exceed range of largest integer}}
+enum E {
   E1 = -__LONG_MAX__ -1L,
   E2 = __LONG_MAX__ *2UL+1UL
 };
 

clang/test/CXX/drs/dr3xx.cpp

namespace dr374 { // dr374: yes
   template<> struct N::A<int> {};
 }
 
 // dr375: dup 345
 // dr376: na
 
 namespace dr377 { // dr377: yes
   enum E { // expected-error {{enumeration values exceed range of largest integer}}
-    a = -__LONG_LONG_MAX__ - 1, // expected-error 0-1{{extension}}
-    b = 2 * (unsigned long long)__LONG_LONG_MAX__ // expected-error 0-2{{extension}}
+    a = -((__int128_t)__LONG_LONG_MAX__ << 64|__LONG_LONG_MAX__) - 1, // expected-error 0-2{{extension}}
+    b = 2 * ((__uint128_t)__LONG_LONG_MAX__ << 64|__LONG_LONG_MAX__) // expected-error 0-2{{extension}}
   };
 }
 
 // dr378: dup 276
 // dr379: na
 
 namespace dr381 { // dr381: yes
   struct A {

clang/test/CodeGen/enum2.c

-// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c2x -triple x86_64-linux-gnu %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
 
 int v;
 enum e { MAX };
 
+__uint128_t v1;
+enum b {
+   b0 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL,
+};
+
+#if __STDC_VERSION__ >= 202000L
+_BitInt(256) v2;
+enum c {
+  c0 = 0xFFFF'FFFF'FFFF'FFFF'FFFF'FFFF'FFFF'FFFF'1wb
+  // TODO: check c0's debug inform under c2x
+};
+#endif
+
 void foo (void)
 {
   v = MAX;
+  // CHECK: store i32 0, ptr @v, align 4
+  v1 = b0;
+  // CHECK: store i128 24197857203266734864629346612071973665, ptr @v1, align 16
+#if __STDC_VERSION__ >= 202000L  
+  v2 = c0;
+  // TODO: check how v2's value is changed by the _BitInt enum under c2x
+#endif  
 }
+
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-SAME: baseType: ![[LONG:[0-9]+]]
 // CHECK-SAME: elements: ![[ELTS:[0-9]+]]
 // CHECK: ![[LONG]] = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
 // CHECK: ![[ELTS]] = !{![[MAX:[0-9]+]]}
 // CHECK: ![[MAX]] = !DIEnumerator(name: "MAX", value: 0)
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[BTYPE:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[BTYPE]] = !DIBasicType(name: "unsigned __int128", size: 128, encoding: DW_ATE_unsigned)
+// CHECK: ![[ELTS]] = !{![[E:[0-9]+]]}
+// CHECK: ![[E]] = !DIEnumerator(name: "b0", value: 24197857203266734864629346612071973665, isUnsigned: true)

clang/test/Sema/enum.c

 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
 enum e {A,
         B = 42LL << 32,        // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
       C = -4, D = 12456 };
 
 enum f { a = -2147483648, b = 2147483647 }; // ok.
 
 enum g {  // too negative
    c = -2147483649,         // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
    d = 2147483647 };
 enum h { e = -2147483648, // too pos
    f = 2147483648,           // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
   i = 0xFFFF0000 // expected-warning {{too large}}
 };
 
 // minll maxull
-enum x                      // expected-warning {{enumeration values exceed range of largest integer}}
+enum x                      
 { y = -9223372036854775807LL-1,  // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
 z = 9223372036854775808ULL };    // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
 
 int test(void) {
   return sizeof(enum e) ;
 }
 
 enum gccForwardEnumExtension ve; // expected-warning{{ISO C forbids forward references to 'enum' types}} \