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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
