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;
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
