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

[Wang Yihan via Phabricator via cfe-commits]

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 = dumpRecord(CGF, CanonicalType, FieldLV, Align, Func, Lvl + 1);
       Res = CGF.Builder.CreateAdd(TmpRes, Res);
       continue;
     }
----------------
yihanaa wrote:
> yihanaa wrote:
> > rsmith wrote:
> > > After this patch, this case no longer prints the field name. Eg: https://godbolt.org/z/o7vcbWaEf
> > > 
> > > ```
> > > #include <stdio.h>
> > > 
> > > struct A {};
> > > 
> > > struct B {
> > >     A a;
> > > };
> > > 
> > > 
> > > int main() {
> > >     B x;
> > >     __builtin_dump_struct(&x, &printf);
> > > }
> > > ```
> > > 
> > > now prints:
> > > 
> > > ```
> > > B {
> > >     A {
> > >     }
> > > }
> > > ```
> > > 
> > > This seems like an important regression; please can you take a look? This also suggests to me that there's a hole in our test coverage.
> > Thanks for taking the time to review my patch and writing the Compiler Explorer examples, I'll take a look at this  problem
> > After this patch, this case no longer prints the field name. Eg: https://godbolt.org/z/o7vcbWaEf
> > 
> > ```
> > #include <stdio.h>
> > 
> > struct A {};
> > 
> > struct B {
> >     A a;
> > };
> > 
> > 
> > int main() {
> >     B x;
> >     __builtin_dump_struct(&x, &printf);
> > }
> > ```
> > 
> > now prints:
> > 
> > ```
> > B {
> >     A {
> >     }
> > }
> > ```
> > 
> > This seems like an important regression; please can you take a look? This also suggests to me that there's a hole in our test coverage.
> 
> I'm sorry for introducing this bug. 😔 Do you think the following is correct behavior? If yes I will try to fix it.
> ```
> struct B {
>     struct A a = {
>     }
> }
> ```
I have submitted a patch to fix this. https://reviews.llvm.org/D122920


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122248