Conversation
… its first real event main went red the moment #991 (the subtraction metric) and #992 (the #973 ARM miscompile fix) were both on it. Not a conflict: both were green on their own bases, #992 merged first and grew the selector, and #991's pins were measured against the pre-#992 file. The classic stale-base merge — except this time something noticed, which is the entire point of the lane. selector_lines_code 18,480 -> 18,582 ceiling, moved WRONG way selector_wildcard_arms_code 62 -> 63 ceiling, moved WRONG way selector_lines_total 29,616 -> 29,839 track selector_wildcard_arms_total 105 -> 106 track THE GROWTH IS CORRECT AND THE WAIVER SAYS SO. #973 was a real miscompile: an i64 compare needs a register PAIR, so `alloc_consecutive_pair` spilled the then-arm and `pop_operand` reloaded it into the register still holding the else-arm — both `it` arms then moved the same source and the then-value always won. The fix reserves the just-popped operand (`pop_operand_committed`), generalizing #677's discipline. This is a SOUNDNESS INVARIANT, not a lowering patch — there is no hand-written arm to delete against it, so "grew without deleting" is the right outcome and the waiver records why rather than the ceiling being quietly moved. Evidence carried in the reason: 264/360 -> 360/360 executed vs wasmtime on both ARM legs, 10/10 frozen anchors unchanged, and where bytes move they SHRINK (sel_i64_lt_s 68 B -> 64 B). The added `_ =>` is in the reservation helper's match over operand provenance, where the fallback is the CONSERVATIVE answer (reserve it) rather than a silent skip. Counted honestly rather than written to dodge the pin. Both ceilings RE-BANK at the new value, so the next unexplained growth still reds — a waiver is bound to a value, not an amnesty. claim_check 47/47. status.json regenerated. Refs #242, #973 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
#978 pinned the MC/DC job to `dtolnay/rust-toolchain@1.96.1` for a stated reason: its floors (decisions / conditions / proved / dead) are derived from that exact compiler on the CI host, and the pin exists so a Rust release cannot red the gate with no code change. #984 bumped it to @1.100.0 — as if it were a routine action-version update. FOR THIS ACTION THE REF IS THE COMPILER. So a bot bump that reads like "dependabot: bump action from X to Y" silently changes which rustc the MC/DC surface is built with, and every subsequent run measures a DIFFERENT compiler against 1.96.1-derived floors. The pin was defeated by the one update shape nobody inspects. It is already biting: PR #994 (this branch, before this commit) failed exactly one check — MC/DC — for no reason other than being based on a main that now installs 1.100.0. Two changes: * .github/workflows/ci.yml — restored @1.96.1, with the reason inline at the pin so the next reader does not have to reconstruct it from two issues. * .github/dependabot.yml — `ignore: dtolnay/rust-toolchain` for the github-actions ecosystem. Bumping it is a deliberate act that must RE-MEASURE the floors, which is not a bot's job. This is the same class as the 0.x-minor rule (#849/#965): an update whose CATEGORY is wrong, so the automation's category-based judgement is wrong too. There the fix was "hold 0.x-minor because minor IS major for 0.x"; here it is "this action's ref is not an action version at all". claim_check 47/47. Refs #242, #912, #978 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L
Contributor
Author
|
Superseded by #993, which merged as
Both PRs converged on the same two fixes independently — this one from the coordinator side after main went red, #993 from its own lane after a fix agent pushed to its branch. #993 was CLEAN at 60/60 while this sat with a check pending, so merging the superset was the shorter path to un-redding main. Verified on the merged tree: Nothing from this branch is lost. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
main went red the moment #991 (the subtraction metric) and #992 (the #973 ARM miscompile fix) were both on it.
Not a conflict. Both were green on their own bases; #992 merged first and grew the selector, and #991 pinned numbers measured against the pre-#992 file. The classic stale-base merge — except this time something noticed, which is the entire point of the metric lane.
selector_lines_codeselector_wildcard_arms_codeselector_lines_totalselector_wildcard_arms_totalThe growth is correct, and the waiver says so
#973 was a real miscompile: an i64 compare needs a register pair, so
alloc_consecutive_pairspilled the then-arm andpop_operandreloaded it into the register still holding the else-arm. Bothitarms then moved the same source and the then-value always won. The fix reserves the just-popped operand (pop_operand_committed), generalizing #677 discipline.That is a soundness invariant, not a lowering patch — there is no hand-written arm to delete against it, so "grew without deleting" is the right outcome. The waiver records why, with the evidence in the reason rather than in a commit nobody re-reads: 264/360 -> 360/360 executed vs wasmtime on both ARM legs, 10/10 frozen anchors unchanged, and where bytes move they shrink (
sel_i64_lt_s68 B -> 64 B).The added
_ =>is in the reservation helper match over operand provenance, where the fallback is the conservative answer (reserve it) rather than a silent skip. Counted honestly rather than written to dodge the pin.Both ceilings re-bank at the new value, so the next unexplained growth still reds — a waiver is bound to a value, not an amnesty.
claim_check47/47.status.jsonregenerated.Refs #242, #973