[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 27 11:23:58 PDT 2022


aaron.ballman added a comment.

In D122920#3476388 <https://reviews.llvm.org/D122920#3476388>, @yihanaa wrote:

> In D122920#3472379 <https://reviews.llvm.org/D122920#3472379>, @asoffer wrote:
>
>> In D122920#3471865 <https://reviews.llvm.org/D122920#3471865>, @yihanaa wrote:
>>
>>> In D122920#3471654 <https://reviews.llvm.org/D122920#3471654>, @erichkeane wrote:
>>>
>>>> @yihanaa : I'd suggest seeing the conversation that @rsmith @aaron.ballman and I are having about this builtin here: https://reviews.llvm.org/D124221
>>>>
>>>> In general it looks like the three of us think that this builtin needs an overhaul in implementation/functionality in order to be further useful. While we very much appreciate your attempts to try to improve it, we believe there needs to be larger-scale changes.
>>>
>>> Thanks for your reply, I would like to get involved and help if needed
>>
>> While I eagerly await `__builtin_reflect_struct`, this change still provides significant value in fixing a regression with `__builtin_dump_struct`. Should we proceed with this change and that proposal independently?
>
> Agree

While we'd usually be happy to take the fix-in-hand and apply it, part of the discussion on the other thread is whether to remove `__builtin_dump_struct` entirely. Because of that, I don't think we should make substantial changes in this area until it's clear we're keeping the builtin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920



More information about the cfe-commits mailing list