diff options
Diffstat (limited to 'clang')
| -rw-r--r-- | clang/include/clang/Basic/DiagnosticGroups.td | 2 | ||||
| -rw-r--r-- | clang/include/clang/Basic/DiagnosticSemaKinds.td | 15 | ||||
| -rw-r--r-- | clang/lib/Sema/SemaChecking.cpp | 57 | ||||
| -rw-r--r-- | clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp | 59 |
4 files changed, 131 insertions, 2 deletions
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 6153b8e7edb..819065f2e97 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -34,6 +34,7 @@ def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">; 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 @@ def Conversion : DiagGroup<"conversion", [BoolConversion, ConstantConversion, EnumConversion, + BitFieldEnumConversion, FloatConversion, Shorten64To32, IntConversion, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 9764c9ee347..74dabe7f3fa 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -4908,6 +4908,21 @@ def warn_bitfield_width_exceeds_type_width: Warning< 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">, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 9858244d569..0e5d519c8f8 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -8729,13 +8729,66 @@ bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init, 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)) diff --git a/clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp b/clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp new file mode 100644 index 00000000000..7e7a202f8d2 --- /dev/null +++ b/clang/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); +} |
