[PATCH] D112626: Convert float to double on __builtin_dump_struct

Thu Dec 2 12:34:13 PST 2021

rjmccall added a comment.

In D112626#3167520 <https://reviews.llvm.org/D112626#3167520>, @rafaelfranco wrote:

> Hey all! Thanks for taking the time to review my patch and writing the Compiler Explorer examples and everything. I had no idea this was the essentially the wrong approach to this, I'd be happy to do a bigger overhaul of the whole builtin if that would make it more correct, but as Aaron points out I'm very new to this project (and C++ too) and essentially clueless as to how to do that, I submitted this patch because it looked like it was simple enough to issue the fpext to get the float promoted.
> If you give me some pointers I'd be more than happy to give it a shot, I should have time in the coming weeks. As a seasoned printf debugger 😄 this builtin is pretty useful to me and I'd like to fix it rather than deprecating or otherwise removing it.

Thanks for this.  I'd be happy to help you through it.  And yeah, I'm definitely not arguing for deprecating/removing the builtin long-term; I just want the implementation to be on a sound technical footing.

> As for the static map, it looks to me like it would be fairly straightforward to replace it with a simple helper function?

Yeah, a helper function that just returns a format specifier for a type seems like the way to go, and that would be a reasonable short-term patch.  When this code moves into Sema, hopefully we can find a way to merge that with the logic we already have for `printf` checking.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112626