[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122248#3403468 <https://reviews.llvm.org/D122248#3403468>, @aaron.ballman wrote:

> In D122248#3403349 <https://reviews.llvm.org/D122248#3403349>, @erichkeane wrote:
>
>> In D122248#3403343 <https://reviews.llvm.org/D122248#3403343>, @yihanaa wrote:
>>
>>> In D122248#3403315 <https://reviews.llvm.org/D122248#3403315>, @aaron.ballman wrote:
>>>
>>>> In D122248#3403143 <https://reviews.llvm.org/D122248#3403143>, @yihanaa wrote:
>>>>
>>>>> 1. Support zero-width bitfield, named bitfield and unnamed bitfield.
>>>>> 2. Add a release notes.
>>>>>
>>>>> The builtin function __builtin_dump_struct behaves for zero-width bitfield and unnamed bitfield as follows
>>>>>
>>>>>   int printf(const char *fmt, ...);
>>>>>   
>>>>>   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);
>>>>>   }
>>>>>   
>>>>>   int main() {
>>>>>     foo();
>>>>>     return 0;
>>>>>   }
>>>>>
>>>>> Output:
>>>>>
>>>>>   struct Bar {
>>>>>   unsigned int c : 1
>>>>>   unsigned int  : 0
>>>>>   unsigned int b : 2022
>>>>>   }
>>>>
>>>> Thank you for the release note and additional test coverage. I'm wondering why we handle the zero-width bit-field differently from the anonymous one (e.g., why do we not have `unsigned int : 3` before the `unsigned int : 0`? It seems a bit odd to drop that from the output.
>>>
>>> Thanks, I don't know what the value of this zero-width bitfield should output, can it be a empty value as same as unnamed-bitfield’ she field name?
>>>
>>> for example:
>>>
>>>   struct Bar {
>>>   unsigned int c : 1
>>>   unsigned int  : 0
>>>   unsigned int  :
>>>   unsigned int b : 2022
>>>   }
>>
>> I would definitely expect this, yes.
>
> Oh wow... I was seeing the `:` in the output and thinking that was showing me the bit width after the colon (given that we show the type information), not the value. I wonder how many other folks are tripped up by that sort of thing? In the meantime, now that I understand the printing format better, I would expect that output as well.

Now that you mention it: SHOULD we emit the size of the bitfield on that line?  The 'colon' is quite unfortunate there for exactly that reason (the way you misread it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122248