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

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 20 07:19:20 PDT 2023


erichkeane wrote:

>As I mentioned in https://github.com/llvm/llvm-project/pull/69104#discussion_r1365269451, I'm not putting any restrictions on type parameter of the attribute, which makes even more sense for more generic preferred_type.

>But I'm confused by the fact you are raising this question. In https://github.com/llvm/llvm-project/pull/69104#discussion_r1364432624, you wrote:

It was just a question, based on the motivation I was misguided and wanted to make sure I had the complete list.  I can see value to leaving it unrestricted.

>I'd like to state upfront that I'm diagnosing only booleans, because I see how it can help maintaining Clang headers. Now, on CHAR_BIT == 8 platform, your example results in 8 bits for value, and 1 bit of padding (http://eel.is/c++draft/class.bit#1.sentence-6). I find such bit-field somewhat strange (why can't padding be declared explicitly via unnamed bit-field?), but maybe I haven't seen enough bit-fields in my life to appreciate it.

No, I'm saying that there ARE cases where the user would want to do something like:
` unsigned SomeChar : 9;`

That is because in some interfaces in the C standard library will return an integer that is EITHER a CHAR or an error-code.  So if the user is intending to capture that, this would be a false positive.  Definitively motivated IMO more than a 2 bit `bool`.  But since you're limiting this to `bool`, I'm less concerned.

>I wonder if we should treat one-bit bit-fields as if they were bool automatically (e.g., create this attribute implicitly in that case). How often do we expect to see one-bit bit-fields that are arithmetic? I'm sure it happens (to multiply against -1, 0, or 1 depending on the sign of the bit-field, for example), but I expect it to be quite rare compared to bool-like use.

I'd be tentative to do that initially, I think I'd prefer that be a future 'enhancement' after we evaluate the fall-out of this.  The rest of this patch has some significant promise, and I'd like to not hold it up trying to evaluate that.


>However, will this actually work in practice in the debugger? If not, perhaps we should limit to just integer and enumeration types for now, leaving the extension for the future.

OOof, yeah, I was initially thinking about something like `Fancy`, but the tests I'd need to feel comfortable doing something like that would be pretty extensive.  Additionally, anything 'floating point' would be REALLY messy as a bitfield anyway.  I like the idea of limiting this to `isIntegralOrEnumerationType` (or, perhaps that and `isa<EnumType>` so that we support incomplete enum types, just with no diagnostics).

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


More information about the cfe-commits mailing list