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

yihanaa added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2113-2117 if (CanonicalType->isRecordType()) { - TmpRes = dumpRecord(CGF, CanonicalType, FieldPtr, Align, Func, Lvl + 1); + TmpRes = dumpRecord(CGF, CanonicalType, FieldLV, Align, Func, Lvl

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

yihanaa marked an inline comment as done. yihanaa added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2113-2117 if (CanonicalType->isRecordType()) { - TmpRes = dumpRecord(CGF, CanonicalType, FieldPtr, Align, Func, Lvl + 1); + TmpRes =

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

yihanaa added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2113-2117 if (CanonicalType->isRecordType()) { - TmpRes = dumpRecord(CGF, CanonicalType, FieldPtr, Align, Func, Lvl + 1); + TmpRes = dumpRecord(CGF, CanonicalType, FieldLV, Align, Func, Lvl

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

yihanaa marked 3 inline comments as done. yihanaa added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2113-2117 if (CanonicalType->isRecordType()) { - TmpRes = dumpRecord(CGF, CanonicalType, FieldPtr, Align, Func, Lvl + 1); + TmpRes =

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

rsmith added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2113-2117 if (CanonicalType->isRecordType()) { - TmpRes = dumpRecord(CGF, CanonicalType, FieldPtr, Align, Func, Lvl + 1); + TmpRes = dumpRecord(CGF, CanonicalType, FieldLV, Align, Func, Lvl

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

tambre added inline comments. Comment at: clang/docs/ReleaseNotes.rst:95-96 +- The builtin function __builtin_dump_struct would crash clang when the target + struct have bitfield. Now it fixed, and __builtin_dump_struct support dump + the bitwidth of bitfields. + This fixes

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7faa95624eb3: [clang][CodeGen]Fix clang crash and add bitfield support in… (authored by yihanaa, committed by erichkeane). Changed prior to commit:

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

aaron.ballman added a comment. Thank you for the fix and the improvements here, @yihanaa! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122248/new/ https://reviews.llvm.org/D122248 ___ cfe-commits

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

yihanaa added a comment. In D122248#3406180 , @erichkeane wrote: > In D122248#3406162 , @yihanaa wrote: > >> In D122248#3406145 , >> @aaron.ballman wrote: >> >>> LGTM

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

yihanaa updated this revision to Diff 418008. yihanaa added a comment. Put the dump format changes under the "Non-comprehensive list of changes in this release" heading instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122248/new/

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

erichkeane added a comment. In D122248#3406162 , @yihanaa wrote: > In D122248#3406145 , @aaron.ballman > wrote: > >> LGTM aside from a tiny nit with one of the release notes. > > I'd be happy to fix it > > In

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

yihanaa added a comment. In D122248#3406145 , @aaron.ballman wrote: > LGTM aside from a tiny nit with one of the release notes. I'd be happy to fix it In D122248#3406145 , @aaron.ballman wrote: > LGTM aside

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

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a tiny nit with one of the release notes. Comment at: clang/docs/ReleaseNotes.rst:140-144 +- Improve __builtin_dump_struct: + + - Support bitfields in struct and union. + + - Improve the

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

yihanaa added a comment. In D122248#3406117 , @erichkeane wrote: > LGTM! Please give Aaron a few hours (perhaps until tomorrow?) to take 1 last > look before committing. > > Also, if you lack commit rights and need someone to commit for you, please >

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

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. LGTM! Please give Aaron a few hours (perhaps until tomorrow?) to take 1 last look before committing. Also, if you lack commit rights and need someone to commit for you, please

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

yihanaa added a comment. Waitting for CI... 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

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

yihanaa updated this revision to Diff 417999. yihanaa marked an inline comment as done. yihanaa added a comment. Use ``FD->getDeclName().empty()`` instead of ``FD->getNameAsString().empty()`` Add a the format changes to release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

erichkeane added inline comments. Comment at: clang/docs/ReleaseNotes.rst:81 `Issue 50541 `_. - +- The builtin function __builtin_dump_struct would crash clang when the target + struct have bitfield. Now it fixed, and

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

yihanaa marked an inline comment as done. yihanaa added a comment. Maybe add the changes under "Attribute Changes in Clang"? Comment at: clang/docs/ReleaseNotes.rst:81 `Issue 50541 `_. - +- The builtin function

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

erichkeane added inline comments. Comment at: clang/docs/ReleaseNotes.rst:81 `Issue 50541 `_. - +- The builtin function __builtin_dump_struct would crash clang when the target + struct have bitfield. Now it fixed, and

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

yihanaa added a comment. In D122248#3405866 , @aaron.ballman wrote: > In D122248#3405644 , @yihanaa wrote: > >> In D122248#3405166 , @erichkeane >> wrote: >> >>> In

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

yihanaa updated this revision to Diff 417990. yihanaa added a comment. Support dump bitwidth of bitfields, and unnamed bitfields. for example: struct Bar { unsigned c : 1; unsigned : 3; unsigned : 0; unsigned b; }; struct Bar { unsigned int c : 1 = 0 unsigned int : 3 = 0

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

erichkeane added a comment. Agreed. `DeclarationName::getAsString` is in no way marked deprecated though, so you could call that? `FieldDecl->getDeclName()->getAsString()` or the `DeclarationName` `operator<<` would both be equivalent with no threat of deprecation. Repository: rG LLVM

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

aaron.ballman added a comment. In D122248#3405644 , @yihanaa wrote: > In D122248#3405166 , @erichkeane > wrote: > >> In D122248#3405062 , >> @aaron.ballman wrote: >>

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

yihanaa added a comment. In D122248#3405166 , @erichkeane wrote: > In D122248#3405062 , @aaron.ballman > wrote: > >> In D122248#3403734 , @yihanaa >> wrote: >> >>>

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

erichkeane added a comment. In D122248#3405062 , @aaron.ballman wrote: > In D122248#3403734 , @yihanaa wrote: > >> What if we don't emit '=' for zero-width bitfield, like this: >> >> struct Bar { >> unsigned

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

aaron.ballman added a comment. In D122248#3403734 , @yihanaa wrote: > What if we don't emit '=' for zero-width bitfield, like this: > > struct Bar { > unsigned c : 1; > unsigned : 3; > unsigned : 0; > unsigned b; > }; > > struct Bar { >

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

yihanaa added a comment. In D122248#3403698 , @erichkeane wrote: >> I'm sorry I misunderstood what you meant @aaron.ballman. >> >> Can we follow the lead of LLVM IR？it use 'undef' >> for example: >> >> struct T6A { >> unsigned a : 1; >>

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

erichkeane added a comment. > I'm sorry I misunderstood what you meant @aaron.ballman. > > Can we follow the lead of LLVM IR？it use 'undef' > for example: > > struct T6A { > unsigned a : 1; > unsigned : 0; > unsigned c : 1; > }; > > @__const.foo.a = private

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

yihanaa added a comment. In D122248#3403637 , @erichkeane wrote: > In D122248#3403636 , @yihanaa wrote: > >> In D122248#3403518 , >> @aaron.ballman wrote: >> >>> In

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

erichkeane added a comment. In D122248#3403636 , @yihanaa wrote: > In D122248#3403518 , @aaron.ballman > wrote: > >> In D122248#3403478 , @erichkeane >> wrote: >> >>>

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

yihanaa added a comment. In D122248#3403518 , @aaron.ballman wrote: > In D122248#3403478 , @erichkeane > wrote: > >> If it is ok, I think we should probably change the format of the 'dump' for >> fields.

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

yihanaa added a comment. In 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

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

aaron.ballman added a comment. In 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

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

erichkeane added a comment. 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

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

erichkeane added a comment. In D122248#3403468 , @aaron.ballman wrote: > In D122248#3403349 , @erichkeane > wrote: > >> In D122248#3403343 , @yihanaa >> wrote: >> >>>

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

aaron.ballman added a comment. In D122248#3403349 , @erichkeane wrote: > In D122248#3403343 , @yihanaa wrote: > >> In D122248#3403315 , >> @aaron.ballman wrote: >> >>>

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

yihanaa added a comment. In D122248#3403349 , @erichkeane wrote: > In D122248#3403343 , @yihanaa wrote: > >> In D122248#3403315 , >> @aaron.ballman wrote: >> >>> In

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

erichkeane added a comment. In D122248#3403343 , @yihanaa wrote: > In D122248#3403315 , @aaron.ballman > wrote: > >> In D122248#3403143 , @yihanaa >> wrote: >> >>> 1.

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

yihanaa added a comment. In D122248#3403315 , @aaron.ballman wrote: > In D122248#3403143 , @yihanaa wrote: > >> 1. Support zero-width bitfield, named bitfield and unnamed bitfield. >> 2. Add a release notes. >>

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

aaron.ballman added a comment. In 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

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

yihanaa added a comment. In D122248#3402924 , @erichkeane wrote: > Seems reasonable to me. Obviously needs release notes + the tests that Aaron > did, but I think this looks right. Thank you for reminding，i have add a release notes entry and a test

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

yihanaa updated this revision to Diff 417687. yihanaa added a comment. 1. Support zero-width bitfield. 2. Supported unnamed bitfield. 3. Add a release notes. The builtin function __builtin_dump_struct behaves for zero-width bitfield and unnamed bitfield as follows void test8(void) {

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

yihanaa added a comment. In D122248#3402875 , @aaron.ballman wrote: > Thanks for the fix! Can you please add a release note as well? Okay, i try to add some test case for zero-width bitfield and unnamed bitfield, and add a release notes. Repository:

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

erichkeane added a comment. Seems reasonable to me. Obviously needs release notes + the tests that Aaron did, but I think this looks right. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122248/new/ https://reviews.llvm.org/D122248

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

aaron.ballman added a comment. Thanks for the fix! Can you please add a release note as well? Comment at: clang/test/CodeGen/dump-struct-builtin.c:653 + // CHECK: call i32 (i8*, ...) @printf( + __builtin_dump_struct(, ); +} Can you add some test coverage

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

yihanaa updated this revision to Diff 417439. yihanaa added a comment. Remove useless comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122248/new/ https://reviews.llvm.org/D122248 Files: clang/lib/CodeGen/CGBuiltin.cpp

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

yihanaa added a comment. In D122248#3400408 , @xbolva00 wrote: >>> the remaining changes are code formatting > > Please remove code formatting changes. Okay, i have removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

yihanaa updated this revision to Diff 417435. yihanaa added a reviewer: xbolva00. yihanaa added a comment. Remove code formatting changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122248/new/ https://reviews.llvm.org/D122248 Files:

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

xbolva00 added a comment. >> the remaining changes are code formatting Please remove code formatting changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122248/new/ https://reviews.llvm.org/D122248