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

Thu Mar 31 12:50:30 PDT 2022

aaron.ballman added a comment.

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

> 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.

FWIW, I'm not concerned by this. My understanding of this builtin is that it exists solely as a developer debugging aid (https://reviews.llvm.org/D44093) predominately for kernel folks when a debugger may not be available. As such, I view this the same as our other `dump()` methods -- we try to dump useful information, but ultimately, nobody should be relying on this output or building anything around 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.

While this is interesting, I am not certain there's a lot of value to it as a debugging aid for times when a debugger is not available to you. It sounds like a fair amount of complexity, so I'd like to know whether there's a strong need for this much generality.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822