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

Vlad Serebrennikov via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 20 00:48:39 PDT 2023


Endilll wrote:

> While I think that warning is accurate, I somewhat question the value of the 'bool' as working on this type

I'm not sure what you mean by "working" here, but I'd like to highlight that we have hundreds of single-bit bit-fields across Clang that would benefit from `[[clang::preferred_type(bool)]]`.

> as, I'm not sure what it really means to put a non-enum here?

When it was `clang::debug_info_type`, the meaning was as simple as "I want the value of this bit-field to be interpreted as T". Now that we moved to `preferred_type`, there might be more interpretations.

> What types ARE we allowing in this now?

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:
> I can see potential value of a "OpaqueValueOfStruct" storage type thing, that perhaps we should just 'trust' that the user is doing something sensible with it.

I read that as you being in favor of trusting user to pass whatever type they want to `preferred_type`. I feel like I'm getting mixed signals from you on this topic, so it'd be nice if you can make yourself clear.

> I think the wording of the diagnostic is perhaps awkward, so we should bikeshed that as well

Sure.

> though I'm on the fence about the diagnostic existing at all. On one hand, this would be such a special case, as typically 'too large' for the type isn't an issue for any reason. On the other, the 'bool' case is perhaps uniquely an issue.

As we are going to attach semantic type information to our bit-fields, I see an opportunity to raise valid questions like "Why do you need 2 bits to hold a boolean value?". If intention is to reserve bits for future extensions, I believe it should be stated more explicitly via something along the lines of `enum E { False, True, Reserved = 3}`.

> What about if the person prefers the type be 'char', but has it be a size of 9? THIS is a case where it is reasonable (consider cases where you need to store the result of one of those calls to the C library that returns 'an int that is either a char or -1').

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.

> So I guess I'm really questioning how accurate/error-free we could make that diagnostic.

In it's current form (only `bool` is checked), I consider it rather accurate. I'm yet to see a use case where `[[clang::preferred_type(bool)]] unsigned Predicate : 2` is the best solution available.

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


More information about the cfe-commits mailing list