[PATCH] D144157: Add 128-bit integer support to enum element

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

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.)



================
Comment at: clang/lib/Sema/SemaDecl.cpp:19571-19573
-  // 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.
----------------
h-vetinari wrote:
> That comment is still relevant AFAICT, at least partially (the ISO C restrictions are still there, GCC still extends it)
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.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:19698-19699
+      BestWidth = 128;
+      assert(NumPositiveBits <= BestWidth &&
+             "How could an initializer get larger than 128-bit?");
+      assert(Context.getTargetInfo().hasInt128Type() &&
----------------
Consider (in C2x):
```
enum E {
  Val = 0xFFFF'FFFF'FFFF'FFFF'FFFF'FFFF'FFFF'FFFF'1wb
};
```


================
Comment at: clang/lib/Sema/SemaDecl.cpp:19700-19702
+      assert(Context.getTargetInfo().hasInt128Type() &&
+             "How could an initializer get larger than ULL in target without "
+             "128-bit interger type support?");
----------------
Again, `_BitInt`.


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

https://reviews.llvm.org/D144157