This revision was automatically updated to reflect the committed changes.
Closed by commit rL297761: Warn on enum assignment to bitfields that can't fit
all values (authored by rnk).
Changed prior to commit:
https://reviews.llvm.org/D30923?vs=91749&id=91752#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30923
Files:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-bitfield-enum-conversion.cpp
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -8729,13 +8729,66 @@
return false;
Expr *OriginalInit = Init->IgnoreParenImpCasts();
+ unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
llvm::APSInt Value;
- if (!OriginalInit->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects))
+ if (!OriginalInit->EvaluateAsInt(Value, S.Context,
+ Expr::SE_AllowSideEffects)) {
+ // The RHS is not constant. If the RHS has an enum type, make sure the
+ // bitfield is wide enough to hold all the values of the enum without
+ // truncation.
+ if (const auto *EnumTy = OriginalInit->getType()->getAs<EnumType>()) {
+ EnumDecl *ED = EnumTy->getDecl();
+ bool SignedBitfield = BitfieldType->isSignedIntegerType();
+
+ // Enum types are implicitly signed on Windows, so check if there are any
+ // negative enumerators to see if the enum was intended to be signed or
+ // not.
+ bool SignedEnum = ED->getNumNegativeBits() > 0;
+
+ // Check for surprising sign changes when assigning enum values to a
+ // bitfield of different signedness. If the bitfield is signed and we
+ // have exactly the right number of bits to store this unsigned enum,
+ // suggest changing the enum to an unsigned type. This typically happens
+ // on Windows where unfixed enums always use an underlying type of 'int'.
+ unsigned DiagID = 0;
+ if (SignedEnum && !SignedBitfield) {
+ DiagID = diag::warn_unsigned_bitfield_assigned_signed_enum;
+ } else if (SignedBitfield && !SignedEnum &&
+ ED->getNumPositiveBits() == FieldWidth) {
+ DiagID = diag::warn_signed_bitfield_enum_conversion;
+ }
+
+ if (DiagID) {
+ S.Diag(InitLoc, DiagID) << Bitfield << ED;
+ TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
+ SourceRange TypeRange =
+ TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
+ S.Diag(Bitfield->getTypeSpecStartLoc(), diag::note_change_bitfield_sign)
+ << SignedEnum << TypeRange;
+ }
+
+ // Compute the required bitwidth. If the enum has negative values, we need
+ // one more bit than the normal number of positive bits to represent the
+ // sign bit.
+ unsigned BitsNeeded = SignedEnum ? std::max(ED->getNumPositiveBits() + 1,
+ ED->getNumNegativeBits())
+ : ED->getNumPositiveBits();
+
+ // Check the bitwidth.
+ if (BitsNeeded > FieldWidth) {
+ Expr *WidthExpr = Bitfield->getBitWidth();
+ S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
+ << Bitfield << ED;
+ S.Diag(WidthExpr->getExprLoc(), diag::note_widen_bitfield)
+ << BitsNeeded << ED << WidthExpr->getSourceRange();
+ }
+ }
+
return false;
+ }
unsigned OriginalWidth = Value.getBitWidth();
- unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
if (!Value.isSigned() || Value.isNegative())
if (UnaryOperator *UO = dyn_cast<UnaryOperator>(OriginalInit))
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4908,6 +4908,21 @@
def warn_anon_bitfield_width_exceeds_type_width : Warning<
"width of anonymous bit-field (%0 bits) exceeds width of its type; value "
"will be truncated to %1 bit%s1">, InGroup<BitFieldWidth>;
+def warn_bitfield_too_small_for_enum : Warning<
+ "bit-field %0 is not wide enough to store all enumerators of %1">,
+ InGroup<BitFieldEnumConversion>, DefaultIgnore;
+def note_widen_bitfield : Note<
+ "widen this field to %0 bits to store all values of %1">;
+def warn_unsigned_bitfield_assigned_signed_enum : Warning<
+ "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "negative enumerators of enum %1 will be converted to positive values">,
+ InGroup<BitFieldEnumConversion>, DefaultIgnore;
+def warn_signed_bitfield_enum_conversion : Warning<
+ "signed bit-field %0 needs an extra bit to represent the largest positive "
+ "enumerators of %1">,
+ InGroup<BitFieldEnumConversion>, DefaultIgnore;
+def note_change_bitfield_sign : Note<
+ "consider making the bitfield type %select{unsigned|signed}0">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td
@@ -34,6 +34,7 @@
def GNUBinaryLiteral : DiagGroup<"gnu-binary-literal">;
def GNUCompoundLiteralInitializer : DiagGroup<"gnu-compound-literal-initializer">;
def BitFieldConstantConversion : DiagGroup<"bitfield-constant-conversion">;
+def BitFieldEnumConversion : DiagGroup<"bitfield-enum-conversion">;
def BitFieldWidth : DiagGroup<"bitfield-width">;
def ConstantConversion :
DiagGroup<"constant-conversion", [ BitFieldConstantConversion ] >;
@@ -606,6 +607,7 @@
[BoolConversion,
ConstantConversion,
EnumConversion,
+ BitFieldEnumConversion,
FloatConversion,
Shorten64To32,
IntConversion,
Index: cfe/trunk/test/SemaCXX/warn-bitfield-enum-conversion.cpp
===================================================================
--- cfe/trunk/test/SemaCXX/warn-bitfield-enum-conversion.cpp
+++ cfe/trunk/test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -verify %s -Wbitfield-enum-conversion
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-linux -verify %s -Wbitfield-enum-conversion
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;
+enum ThreeBits { Hi3 = 7 } three_bits;
+enum ThreeBitsSigned { Lo4 = -4, Hi4 = 3 } three_bits_signed;
+enum TwoBitsFixed : unsigned { Hi5 = 3 } two_bits_fixed;
+
+struct Foo {
+ unsigned two_bits : 2; // expected-note 2 {{widen this field to 3 bits}} expected-note 2 {{type signed}}
+ int two_bits_signed : 2; // expected-note 2 {{widen this field to 3 bits}} expected-note 1 {{type unsigned}}
+ unsigned three_bits : 3; // expected-note 2 {{type signed}}
+ int three_bits_signed : 3; // expected-note 1 {{type unsigned}}
+
+#ifdef _WIN32
+ // expected-note@+2 {{type unsigned}}
+#endif
+ ThreeBits three_bits_enum : 3;
+ ThreeBits four_bits_enum : 4;
+};
+
+void f() {
+ Foo f;
+
+ f.two_bits = two_bits;
+ f.two_bits = two_bits_signed; // expected-warning {{negative enumerators}}
+ f.two_bits = three_bits; // expected-warning {{not wide enough}}
+ f.two_bits = three_bits_signed; // expected-warning {{negative enumerators}} expected-warning {{not wide enough}}
+ f.two_bits = two_bits_fixed;
+
+ f.two_bits_signed = two_bits; // expected-warning {{needs an extra bit}}
+ f.two_bits_signed = two_bits_signed;
+ f.two_bits_signed = three_bits; // expected-warning {{not wide enough}}
+ f.two_bits_signed = three_bits_signed; // expected-warning {{not wide enough}}
+
+ f.three_bits = two_bits;
+ f.three_bits = two_bits_signed; // expected-warning {{negative enumerators}}
+ f.three_bits = three_bits;
+ f.three_bits = three_bits_signed; // expected-warning {{negative enumerators}}
+
+ f.three_bits_signed = two_bits;
+ f.three_bits_signed = two_bits_signed;
+ f.three_bits_signed = three_bits; // expected-warning {{needs an extra bit}}
+ f.three_bits_signed = three_bits_signed;
+
+#ifdef _WIN32
+ // Enums on Windows are always implicitly 'int', which is signed, so you need
+ // an extra bit to store values that set the MSB. This is not true on SysV
+ // platforms like Linux.
+ // expected-warning@+2 {{needs an extra bit}}
+#endif
+ f.three_bits_enum = three_bits;
+ f.four_bits_enum = three_bits;
+
+ // Explicit casts suppress the warning.
+ f.two_bits = (unsigned)three_bits_signed;
+ f.two_bits = static_cast<unsigned>(three_bits_signed);
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
