[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

Mon Mar 27 08:12:02 PDT 2023

ilya-biryukov added a comment.

In D146426#4209620 <https://reviews.llvm.org/D146426#4209620>, @aaron.ballman wrote:

> Thank you for offering to do that in a follow-up, but please, next time wait for there to be agreement on the patch before landing it. Multiple reviewers expressed concerns that this is papering over a larger bug and didn't have the chance to respond to your comments. Reverting recently broken patches is fine because that gets us back into a better state, but pushing temporary hacks is a different situation entirely (those have a tendency to become permanent and cause problems in the future because the design is more confused). (No need to revert your changes that have already landed, that'll just cause churn, but those changes should be reverted in your follow-up patch.)

Sorry about that. This looked like a small change and I tried to rush it in to address the crash we had in production.
I wouldn't mind a revert from someone else on the basis of not getting agreement here.
FWIW, new cases started showing up not handled by this patch, so the discussion was fully warranted and I think my original approach does not even help with that.

I tried to look at the problem more closely and I think there is a way to avoid getting null pointers entirely, but we'll have to make the recovery a bit more robust in case of errors.
See D146971 <https://reviews.llvm.org/D146971> for the first attempt at this approach, let's continue the discussion there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146426