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