[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 14 07:44:48 PST 2025


AaronBallman wrote:

> > @zixu-w and @ChuanqiXu9 Just want to clarify policies about changes reversion. As far as I understand it was one example in some private codebase with no reproducer publicly available. I can understand and completely agree if it breaks any existing test or llvm-build bot. But in this case I think it would be good to at least wait for the review. Such reversion without providing a reproducer does not allow to fix the issues or even verify that it is related to this PR. This PR fixes another crash with clear reproducers.
> > Sorry, didn't notice that it is LLVM bootstrap build but still, no still I don't see steps to reproduce in #126973.
> 
> CC @AaronBallman for the policy related things.

Our documented policy is at https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy

The promise for a public reproducer is in line with our policy, but reverting a change after it's been in the tree for two weeks is not particularly timely (but not unreasonable either, sometimes it takes a while to churn through downstream team suites). However, it's not clear to me why the revert couldn't have been handled in the downstream given the lack of details on the problem and the fact that upstream does not seem to be obviously broken. For example, the issue could ultimately boil down to interactions with changes carried in the downstream.

That said, I don't think anyone's doing anything wrong (certainly nobody is acting in bad faith), and there was more than a day for reviewers or the patch author to have questioned the revert before the revert was committed.

> Personally I do agree it is frustrating if the patch gets reverted without being able to reproduce it.
> 
> And my point in the above post is, **if** we revert it in the trunk and it was backported to the release branch, we should revert it in the release branch too. My point is majorly the **if**.

I think the rule of thumb should be to revert in the release branch if the revert happens in trunk. CC @tstellar @tru in case they have a different opinion.

https://github.com/llvm/llvm-project/pull/121245


More information about the cfe-commits mailing list