shafik updated this revision to Diff 447500.
shafik marked 5 inline comments as done.
shafik added a comment.
- Adding release note
- Fixing comments
- Removing top level const on local variable
- Adjusting test so checks go after the line they are checking
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130301/new/
https://reviews.llvm.org/D130301
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDecl.cpp
clang/test/CodeGenCXX/pr12251.cpp
compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
Index: compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
===================================================================
--- compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
+++ compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
@@ -1,21 +1,33 @@
-// RUN: %clangxx -fsanitize=enum %s -O3 -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-PLAIN
-// RUN: %clangxx -fsanitize=enum -std=c++11 -DE="class E" %s -O3 -o %t && %run %t
-// RUN: %clangxx -fsanitize=enum -std=c++11 -DE="class E : bool" %s -O3 -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-BOOL
+// RUN: %clangxx -fsanitize=enum %s -O3 -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
// FIXME: UBSan fails to add the correct instrumentation code for some reason on
// Windows.
// XFAIL: windows-msvc
-enum E { a = 1 } e;
-#undef E
+enum E { a = 1 };
+enum class EClass { a = 1 };
+enum class EBool : bool { a = 1 } e3;
+enum EEmpty {};
+enum EMinus { em = -1 };
int main(int argc, char **argv) {
- // memset(&e, 0xff, sizeof(e));
- for (unsigned char *p = (unsigned char*)&e; p != (unsigned char*)(&e + 1); ++p)
+ E e1 = static_cast<E>(0xFFFFFFFF);
+ EClass e2 = static_cast<EClass>(0xFFFFFFFF);
+ EEmpty e4 = static_cast<EEmpty>(1);
+ EEmpty e5 = static_cast<EEmpty>(2);
+ EMinus e6 = static_cast<EMinus>(1);
+ EMinus e7 = static_cast<EMinus>(2);
+
+ for (unsigned char *p = (unsigned char *)&e3; p != (unsigned char *)(&e3 + 1);
+ ++p)
*p = 0xff;
- // CHECK-PLAIN: error: load of value 4294967295, which is not a valid value for type 'enum E'
- // FIXME: Support marshalling and display of enum class values.
- // CHECK-BOOL: error: load of value <unknown>, which is not a valid value for type 'enum E'
- return (int)e != -1;
+ return ((int)e1 != -1) & ((int)e2 != -1) &
+ // CHECK: error: load of value 4294967295, which is not a valid value for type 'E'
+ ((int)e3 != -1) & ((int)e4 == 1) &
+ // CHECK: error: load of value <unknown>, which is not a valid value for type 'enum EBool'
+ ((int)e5 == 2) & ((int)e6 == 1) &
+ // CHECK: error: load of value 2, which is not a valid value for type 'EEmpty'
+ ((int)e7 == 2);
+ // CHECK: error: load of value 2, which is not a valid value for type 'EMinus'
}
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
@@ -18899,14 +18899,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.
+ unsigned ActiveBits = InitVal.getActiveBits();
+ NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+ } else {
NumNegativeBits = std::max(NumNegativeBits,
(unsigned)InitVal.getMinSignedBits());
+ }
}
+ // If we have have an 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/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -198,6 +198,10 @@
constant folded. Fixes `Issue 55638 <https://github.com/llvm/llvm-project/issues/55638>`_.
- Fixed incompatibility of Clang's ``<stdatomic.h>`` with MSVC ``<atomic>``.
Fixes `MSVC STL Issue 2862 <https://github.com/microsoft/STL/issues/2862>`_.
+- Empty enums and enums with a single enumerator with value zero will be
+ considered to have one positive bit in order to represent the underlying
+ value. This effects whether we consider the store of the value one to be well
+ defined.
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
