[PATCH] D130301: [Clang] Fix how we set the NumPositiveBits on an E numDecl to cover the case of single enumerator with value zero or an empty enum

[Shafik Yaghmour via Phabricator via cfe-commits]

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

Currently in `Sema::ActOnEnumBody(...)` when calculating `NumPositiveBits` we miss the case where there is only a single enumerator with value zero and the case of an empty enum. In both cases we end up with zero positive bits when in fact we need one bit to store the value zero.

This PR updates the calculation to account for these cases.

Relevant C++ standard quotes are `[dcl.enum]p7`:

> .., If the enumerator-list is empty, the underlying type is as if the enumeration had a single enumerator with value 0.

and `[dcl.enum]p8`:

> ... Otherwise, the values of the enumeration are the values representable by a hypothetical
> integer type with minimal width M such that all enumerators can be represented. The width of the smallest
> bit-field large enough to hold all the values of the enumeration type is M. ...


https://reviews.llvm.org/D130301

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/pr12251.cpp


Index: clang/test/CodeGenCXX/pr12251.cpp
===================================================================
--- clang/test/CodeGenCXX/pr12251.cpp
+++ clang/test/CodeGenCXX/pr12251.cpp
@@ -18,14 +18,14 @@
   return *x;
 }
 // CHECK-LABEL: define{{.*}} i32 @_Z2g1P2e1
-// CHECK: ret i32 0
+// CHECK: ret i32 %0
 
 enum e2 { e2_a = 0 };
 e2 g2(e2 *x) {
   return *x;
 }
 // CHECK-LABEL: define{{.*}} i32 @_Z2g2P2e2
-// CHECK: ret i32 0
+// CHECK: ret i32 %0
 
 enum e3 { e3_a = 16 };
 e3 g3(e3 *x) {
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -18878,14 +18878,24 @@
     const llvm::APSInt &InitVal = ECD->getInitVal();
 
     // Keep track of the size of positive and negative values.
-    if (InitVal.isUnsigned() || InitVal.isNonNegative())
-      NumPositiveBits = std::max(NumPositiveBits,
-                                 (unsigned)InitVal.getActiveBits());
-    else
+    if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+      // If the enumerator is zero that should still be counted as a positive
+      // bit since we need a bit to store the value zero
+      const unsigned ActiveBits =
+          InitVal.getActiveBits() ? InitVal.getActiveBits() : 1;
+      NumPositiveBits = std::max(NumPositiveBits, ActiveBits);
+    } else
       NumNegativeBits = std::max(NumNegativeBits,
                                  (unsigned)InitVal.getMinSignedBits());
   }
 
+  // If we have have a empty set of enumerators we still need one bit.
+  // From [dcl.enum]p8
+  // If the enumerator-list is empty, the values of the enumeration are as if
+  // the enumeration had a single enumerator with value 0
+  if (!NumPositiveBits && !NumNegativeBits)
+    NumPositiveBits = 1;
+
   // Figure out the type that should be used for this enum.
   QualType BestType;
   unsigned BestWidth;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130301.446617.patch
Type: text/x-patch
Size: 1918 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220721/aa064526/attachment.bin>