rsmith added subscribers: rjmccall, rsmith.
rsmith added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to