erichkeane wrote: > I'm sorry that I wasn't able to more usefully reduce the failure cases. When > such regressions show up, we usually don't have any meaningful context on the > code. For our own code, we have guidelines to try to limit complexity which > makes reduction more tractable, but third-party libraries are harder. > > For publicly-available code, it's not clear to me how much of the burden > should fall on people that identify the problem. I want to do as much of this > work as I can, it's difficult to balance the urgency of providing some > reproducer (it gets hard to push for a fix if we wait a week for a good > reproducers), the quality of reduction, and other work/deadlines. (As > mentioned, the timing was difficult this time as this landed just before a > holiday).
I agree here for the most part. BUT our problem was that until we saw the reproducer from the bug report, it wasn't clear that this was a regression that required a revert. I think that if someone is going to request a revert that they have to show that we have a regression, and not 'the patch doing what it was supposed to'. I'll note that ~4 of us spent HOURS trying to figure this out as well, so we ALSO need to balance the work/deadlines of those trying to determine whether it is worth a revert. For example, on this patch and a few recent ones, we've had a handful of folks bringing 'regressions' to us that were just the intent of the patch. It was not clear in this case that the regression shown here wasn't just another case of "we diagnose something we didn't before, but that is because we wanted to". Because it was a "crash-after-invalid", it wasn't particularly serious, particularly as it was a crash that happened with this flag before. This is VERY different from "diagnoses improperly" (deserves a revert) and "crashes on valid" (also deserves a revert). In this case we actually DIDN'T get to either of the two above, but the minimal reproducer let a few of us spend a few hours figuring out that this is a C++ Core issue, depending on your interpretation of a core issue this is either valid or invalid code. Within a few hours of determining this was the case, we reverted to leave the 'previous' behavior until the standard is clarified. In the end, your reproducer may very well still 'fail' (if WG21 decides that way), and wouldn't be a regression when we make that change. TLDR; the request for a reduction was because it was associated with requests for a revert in a case it wasn't clear met the barrier for revert. https://github.com/llvm/llvm-project/pull/89807 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
