[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

Thu Mar 31 11:58:47 PDT 2022

yihanaa added a comment.

In D122822#3420162 <https://reviews.llvm.org/D122822#3420162>, @rsmith wrote:

> I'm concerned about the direction of this patch given @rjmccall's comments on https://reviews.llvm.org/D112626 -- presumably the way we'd want to address those comments would be to convert a `__builtin_dump_struct(a, f)` call into a single `f("...",  a.x, a.y, a.z)` call in Sema,  and that approach doesn't seem compatible with generating code to loop over array elements.
>
> I'm also concerned that this builtin is making a lot of design decisions on behalf of the programmer, meaning that either it does exactly what you want or it's not suitable for your use and there's not much you can do about it. A different design that let the programmer choose whether to recurse into structs and arrays and similarly let the programmer choose how to format the fields would seem preferable, but I'm not confident there's a good way to expose that in C (though in C++ there seem to be good designs to choose from). As an example of how that manifests here, the choice to print each element as a separate line for a struct member of type `const char str[256];` is probably going to make the output very unreadable, but choosing to print every array of `char` using `"%s"` will be equally bad for arrays that don't contain text -- the "right" answer for a dumping facility probably involves checking whether the array contains only printable characters and trimming a trailing sequence of zeroes, but that seems like vastly more complexity than we'd want in code generated by a builtin.

I'm also a newbie and only gradually integrated into the community with the help of @erichkeane , @aaron.ballman and others many times, tthis problem may be a bit difficult for me at the moment, but I'd be like to do a bigger overhaul of the whole builtin if that would make it more great.😉

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822