yihanaa added a comment. In D122248#3403637 <https://reviews.llvm.org/D122248#3403637>, @erichkeane wrote:
> 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. I'm sorry, I misunderstood @aaron.ballman. In D122248#3403637 <https://reviews.llvm.org/D122248#3403637>, @erichkeane wrote: > 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. I'm sorry I misunderstood what you meant @aaron.ballman. 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
