erichkeane added a comment.

In D122248#3403636 <https://reviews.llvm.org/D122248#3403636>, @yihanaa wrote:

> In D122248#3403518 <https://reviews.llvm.org/D122248#3403518>, @aaron.ballman 
> wrote:
>
>> In D122248#3403478 <https://reviews.llvm.org/D122248#3403478>, @erichkeane 
>> wrote:
>>
>>> If it is ok, I think we should probably change the format of the 'dump' for 
>>> fields.  Using the colon to split up the field from the value is 
>>> unfortunate, may I suggest replacing it with '=' instead?  As well as 
>>> printing the size after a colon.  So for:
>>>
>>>   void foo(void) {
>>>     struct Bar {
>>>       unsigned c : 1;
>>>       unsigned : 3;
>>>       unsigned : 0;
>>>       unsigned b;
>>>     };
>>>   
>>>     struct Bar a = {
>>>       .c = 1,
>>>       .b = 2022,
>>>     };
>>>   
>>>     __builtin_dump_struct(&a, &printf);
>>>   }
>>>
>>> Output:
>>>
>>>   struct Bar {
>>>   unsigned int c : 1 = 1
>>>   unsigned int : 3  = 0
>>>   unsigned int : 0 = 
>>>   unsigned int b = 2022
>>>   }
>>>
>>> What do you all think?
>>
>> I think that's a good idea for clarity. For the case where we have no value, 
>> I wonder if we want to do something like: `unsigned int : 0 = 
>> <uninitialized>` (or something else to make it exceptionally clear that 
>> there's nothing missing after the `=`)?
>
> how
>
> In D122248#3403518 <https://reviews.llvm.org/D122248#3403518>, @aaron.ballman 
> wrote:
>
>> In D122248#3403478 <https://reviews.llvm.org/D122248#3403478>, @erichkeane 
>> wrote:
>>
>>> If it is ok, I think we should probably change the format of the 'dump' for 
>>> fields.  Using the colon to split up the field from the value is 
>>> unfortunate, may I suggest replacing it with '=' instead?  As well as 
>>> printing the size after a colon.  So for:
>>>
>>>   void foo(void) {
>>>     struct Bar {
>>>       unsigned c : 1;
>>>       unsigned : 3;
>>>       unsigned : 0;
>>>       unsigned b;
>>>     };
>>>   
>>>     struct Bar a = {
>>>       .c = 1,
>>>       .b = 2022,
>>>     };
>>>   
>>>     __builtin_dump_struct(&a, &printf);
>>>   }
>>>
>>> Output:
>>>
>>>   struct Bar {
>>>   unsigned int c : 1 = 1
>>>   unsigned int : 3  = 0
>>>   unsigned int : 0 = 
>>>   unsigned int b = 2022
>>>   }
>>>
>>> What do you all think?
>>
>> I think that's a good idea for clarity. For the case where we have no value, 
>> I wonder if we want to do something like: `unsigned int : 0 = 
>> <uninitialized>` (or something else to make it exceptionally clear that 
>> there's nothing missing after the `=`)?
>
> How to judge whether this field is initialized? Maybe this memory has been 
> initialized by memset

He means a special-case for the 0-size bitfield, which HAS no value (actually, 
wonder if this is a problem with the no-unique-address types as well?).  I 
might suggest `N/A` instead of `uninitialized`, but am open to bikeshedding.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122248/new/

https://reviews.llvm.org/D122248

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to