[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 20 12:13:48 PDT 2023


================
@@ -3153,6 +3153,12 @@ def err_invalid_branch_protection_spec : Error<
   "invalid or misplaced branch protection specification '%0'">;
 def warn_unsupported_branch_protection_spec : Warning<
   "unsupported branch protection specification '%0'">, InGroup<BranchProtection>;
+def warn_attribute_underlying_type_mismatch : Warning<
+  "underlying type %0 of enumeration %1 doesn't match bit-field type %2">,
+  InGroup<BitFieldType>;
+def warn_attribute_bool_bitfield_width : Warning<
+  "bit-field that holds a boolean value should have width of 1 instead of %0">,
----------------
AaronBallman wrote:

Stepping back a bit, I think there are two orthogonal goals here: 1) warn the user "hey, this bit-field is too small to fit everything of that type you said you'll be storing in here" and 2) warn the user "hey, this bit-field is larger than it needs to be in order to fit everything of that type you said you'll be storing in here."

I think #1 is better-served by the existing warning that happens at a problematic expression rather than trying to do it at the point of declaration: https://godbolt.org/z/MG8sPEq6o  This won't help in this situation: https://godbolt.org/z/8sdEY559K but that situation is one the user is explicitly asking to lose type information and so it seems reasonable that we don't diagnose (IMO).

I think #2 is really more of a clang-tidy concern because it's not so much about correctness but space efficiency. We've got things like `-Wpadded` to help point out those issues and maybe this attribute could feed into a similar diagnostic in Clang, but I'm not convinced it will find enough issues in practice to warrant adding it.

So I currently lean towards not diagnosing this situation. (I also wonder if we're accidentally scope-creeping this PR somewhat -- what kicked all this off was me suggesting to rename the attribute so it could be extensible to situations like this, but I didn't intend for that to mean "right now" FWIW.)

https://github.com/llvm/llvm-project/pull/69104


More information about the cfe-commits mailing list