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

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 18 12:23:37 PDT 2023


erichkeane wrote:

> > > > Wouldn't it be better to go the other way around? i.e. have a `[[clang::compressed_bitfield]]` (or whatever) which influences the ABI, so it's possible to do stuff like
> > > > ```
> > > > [[clang::compressed_bitfield]] bool IsSomething : 1;
> > > > [[clang::compressed_bitfield]] MyEnum Whatever : 3;
> > > > [[clang::compressed_bitfield]] int MoreStuff : 4;
> > > > ```
> > > > 
> > > > 
> > > > which the current approach doesn't allow.
> > > 
> > > 
> > > The issue is that MSVC wouldn't know/recognize/implement this attribute - so clang couldn't do that without breaking ABI compatibility with MSVC.
> > 
> > 
> > That's not important for everybody, and you could still `#if` around that code like
> > ```
> > #if __has_cpp_attribute(clang::compressed_bitfield)
> > #define BITFILED_T(ActualType, ABIType) static_assert(check_abi_compat<ActualType, ABIType>()); [[clang::compressed_bitfield]] ActualType
> > #else
> > #define BITFIELD_T(ActualType, ABIType) ABIType
> > ...
> > ```
> > 
> > 
> > Sure, it's not super pretty, but it gets you there and allows you to do more stuff. We could also ask the MSVC folks whether there would be interest to add such an attribute, since there is clearly interest in having a good ABI and stronger typing (CC @StephanTLavavej @CaseyCarter).
> 
> I think that's a somewhat orthogonal feature to the one @Endilll is proposing. While it might make sense to ask the MSVC and GCC folks if they'd be willing to entertain a similar attribute, the implementation landscape is much wider in C and those implementations may have similar ABI issues. If an implementation ignores the `compressed_bitfield` attribute, you get potential ABI issues; if an implementation ignores the `debug_info_type` attribute, you get the default debugging experience but no change to codegen behavior. So I think there may be room for both features.
> 
> We have a `preferred_name` attribute already for specifying "please display this name instead of that name" in diagnostics; this seems somewhat similar in nature in that it's "please display this type instead of that bit-field type" in the debugger, so mayyybe we want to name this `preferred_type` instead? If this could also be useful for diagnostics which mention types and can be triggered with a bit-field, that might also be a reason to use a different name (I'm not certain we have many/any such diagnostics today though.) I'm not opposed to the current name, but more thinking that it might be too specific and we may want a path to more generalization in the future.

While I agree that a 'pack-at-the-bit-level' type attribute might be valuable, I too see that as a 'different' problem.  There is plenty of code (even in Clang itself) where and Enum is being stored as a bitfield, and more interesting debugging information could be generated for it.  I don't mind the name, I think there IS a nice level of symmetry to `preferred_type`, which I think ALSO has the advantage of being useful (by name!) for non-debug diagnostics.

One thing I REALLY hate about our pattern of using bitfields for enums, is that it becomes pretty trivial to add an entry to an enum and forget to increase the size of the bitfield.  I think such a attribute could cause a diagnostic of `bitfield says it holds an ENUM type, but can't store all the values`, which would be INCREDIBLY valuable (not that I'm pushing that this patch IMPLEMENT that, but more that I'm saying such a name COULD do that, and probably should as well).

So in conclusion, I don't mind the name, but also prefer something more general.  

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


More information about the cfe-commits mailing list