faisalv updated this revision to Diff 305970.
faisalv added a comment.
Based on Richards Feedback, this update includes the following changes:
- avoids calling the fragile getZextValue() for comparing against bit-field
width, and uses APSInt's comparison operator overload
- suppresses/avoids the warning for unnamed bit-fields
- simplifies the test case and avoids preprocessor cleverness (and thus an
extra pass).
Thank you for taking the time to review this Richard!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91651/new/
https://reviews.llvm.org/D91651
Files:
clang/include/clang/AST/Decl.h
clang/lib/Sema/SemaDecl.cpp
clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
Index: clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
===================================================================
--- clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
+++ clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -57,3 +57,18 @@
f.two_bits = (unsigned)three_bits_signed;
f.two_bits = static_cast<unsigned>(three_bits_signed);
}
+
+enum class E_UNSIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long
long) * 8 - 2)) };
+enum class E_SIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long
long) * 8 - 2)), MINUS = -1 };
+
+struct E64 {
+ E_UNSIGNED_LONG_LONG b3 : (sizeof(long long) * 8) - 1; // OK.
+ E_SIGNED_LONG_LONG b4 : (sizeof(long long) * 8); // OK.
+
+ E_UNSIGNED_LONG_LONG b1 : 5; //expected-warning {{bit-field 'b1' is not wide
enough}} expected-note {{widen this field to 63}}
+ E_SIGNED_LONG_LONG b2 : 5; //expected-warning {{bit-field 'b2' is not wide
enough}} expected-note {{widen this field to 64}}
+
+ E_UNSIGNED_LONG_LONG : 5; // OK. no warning for unnamed bit fields.
+ E_SIGNED_LONG_LONG : 5; // OK. no warning for unnamed bit fields.
+
+};
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16430,6 +16430,22 @@
}
if (!FieldTy->isDependentType()) {
+ // Check if a named enum bit-field has enough bits to accomodate all its
+ // enumerators. We can ignore unnamed enum bit-fields since they do not
+ // represent values.
+ if (FieldName && FieldTy->isEnumeralType()) {
+ const EnumDecl *const ED = FieldTy->castAs<EnumType>()->getDecl();
+
+ const auto BitsNeeded = ED->getTotalBitsNeeded();
+ if (BitsNeeded > Value) {
+ Diag(FieldLoc, diag::warn_bitfield_too_small_for_enum)
+ << FieldName << ED;
+
+ Diag(BitWidth->getExprLoc(), diag::note_widen_bitfield)
+ << BitsNeeded << ED << BitWidth->getSourceRange();
+ }
+ }
+
uint64_t TypeStorageSize = Context.getTypeSize(FieldTy);
uint64_t TypeWidth = Context.getIntWidth(FieldTy);
bool BitfieldIsOverwide = Value.ugt(TypeWidth);
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3723,6 +3723,14 @@
/// -101 1001011 8
unsigned getNumNegativeBits() const { return EnumDeclBits.NumNegativeBits; }
+ /// Returns the least width in bits required to store all the enumerators.
+ unsigned getTotalBitsNeeded() const {
+ // If the enum has negative values, we need one more bit than the normal
+ // number of positive bits to represent the sign bit.
+ return getNumNegativeBits() > 0
+ ? std::max(getNumPositiveBits() + 1, getNumNegativeBits())
+ : getNumPositiveBits();
+ }
/// Returns true if this is a C++11 scoped enumeration.
bool isScoped() const { return EnumDeclBits.IsScoped; }
Index: clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
===================================================================
--- clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
+++ clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -57,3 +57,18 @@
f.two_bits = (unsigned)three_bits_signed;
f.two_bits = static_cast<unsigned>(three_bits_signed);
}
+
+enum class E_UNSIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long long) * 8 - 2)) };
+enum class E_SIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long long) * 8 - 2)), MINUS = -1 };
+
+struct E64 {
+ E_UNSIGNED_LONG_LONG b3 : (sizeof(long long) * 8) - 1; // OK.
+ E_SIGNED_LONG_LONG b4 : (sizeof(long long) * 8); // OK.
+
+ E_UNSIGNED_LONG_LONG b1 : 5; //expected-warning {{bit-field 'b1' is not wide enough}} expected-note {{widen this field to 63}}
+ E_SIGNED_LONG_LONG b2 : 5; //expected-warning {{bit-field 'b2' is not wide enough}} expected-note {{widen this field to 64}}
+
+ E_UNSIGNED_LONG_LONG : 5; // OK. no warning for unnamed bit fields.
+ E_SIGNED_LONG_LONG : 5; // OK. no warning for unnamed bit fields.
+
+};
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16430,6 +16430,22 @@
}
if (!FieldTy->isDependentType()) {
+ // Check if a named enum bit-field has enough bits to accomodate all its
+ // enumerators. We can ignore unnamed enum bit-fields since they do not
+ // represent values.
+ if (FieldName && FieldTy->isEnumeralType()) {
+ const EnumDecl *const ED = FieldTy->castAs<EnumType>()->getDecl();
+
+ const auto BitsNeeded = ED->getTotalBitsNeeded();
+ if (BitsNeeded > Value) {
+ Diag(FieldLoc, diag::warn_bitfield_too_small_for_enum)
+ << FieldName << ED;
+
+ Diag(BitWidth->getExprLoc(), diag::note_widen_bitfield)
+ << BitsNeeded << ED << BitWidth->getSourceRange();
+ }
+ }
+
uint64_t TypeStorageSize = Context.getTypeSize(FieldTy);
uint64_t TypeWidth = Context.getIntWidth(FieldTy);
bool BitfieldIsOverwide = Value.ugt(TypeWidth);
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3723,6 +3723,14 @@
/// -101 1001011 8
unsigned getNumNegativeBits() const { return EnumDeclBits.NumNegativeBits; }
+ /// Returns the least width in bits required to store all the enumerators.
+ unsigned getTotalBitsNeeded() const {
+ // If the enum has negative values, we need one more bit than the normal
+ // number of positive bits to represent the sign bit.
+ return getNumNegativeBits() > 0
+ ? std::max(getNumPositiveBits() + 1, getNumNegativeBits())
+ : getNumPositiveBits();
+ }
/// Returns true if this is a C++11 scoped enumeration.
bool isScoped() const { return EnumDeclBits.IsScoped; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
