test(#10937): pin declared-number addition evaluation order - #10959
proggeramlug wants to merge 2 commits into
Conversation
Written and run before any production edit. Node 26.5.1 passes all six cases; installed Perry 0.5.1520 reports the two expected failures (6 vs 103, 10 vs 107), with exactly one conversion and a mutated late slot. Four right-associated/snapshot controls pass. The fixture asserts all six cases executed. The assigned base already carries the production fix from #10921.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds regression coverage for declared-number addition evaluation order. The test covers array elements and class fields across left-associated, right-associated, and snapshot expressions. A changelog fragment records the coverage. ChangesDeclared-number addition regression test
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to This change strengthens coverage for declared-number addition evaluation order without changing production behavior. It is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Landed on main in merge train 256 (#11018, v0.5.1638), main Carried at head Trains rebase-merge, so commits get new SHAs and GitHub cannot mark this PR merged. Closed as landed. |
Closes #10937.
The production fix already landed through #10921 in
2d370f625, before this branch's assigned basea022cf2e4. The sharedlower_guarded_numeric_addentry checks evaluation-order faithfulness for both callers. No production change is needed.This PR brings the declared-number counterexamples into the per-PR gap shards with a dedicated
test_gap_fixture. It coversnumber[]elements and fields onthis, plus right-associated and explicitly snapshotted controls. Each case requires exactly one conversion and a late-slot mutation to 100; the fixture asserts that all six cases executed.The test was committed first in
722f34710, after observing its failure and before any production edit.thisfieldsRoot cause: the fused fold loads every leaf before conversion. A left-associated chain must convert its inner pair before reading a later mutable element/field. The cold arm uses the already-loaded values, so it cannot repair the stale read.
Sabotage moved the existing faithfulness check from the shared fold to only the dynamic caller, then rebuilt the compiler and both static archives in the owned Linux worktree. The fixture failed with its own
#10937 declared-number addition order failures: 2message, while the separate dynamic-entry counterexample still returned Node's 102. All four controls passed, with one conversion and the mutation observed. Restoring the shared guard byte-for-byte returned all six cases to green. No sabotage remains in the diff.A planted
compile_error!("ISSUE_10937_COMPILE_SENTINEL")also made the wrapped cargo build fail with that exact diagnostic, proving the build check was live; it was removed before behavioral validation.Validation on Linux x86-64,
perry-devprofile,--test-threads=1, through/root/perry-suite.sh:test_parity_region_guards.ts. The new fixture contains six checked cases.Exact Rust suite commands (all builds/tests over SSH; push and
ghlocal):The first command's codegen library count was 1,663; the full second command supplies the 2,170 total above. Runtime was not rerun.
Lint was opened and inspected at the current head. Its only failed step is Public benchmark evidence freshness:
public artifact benchmark inputs changed; regenerate it with ./benchmarks/run_public_baseline.sh. This is the campaign's known structural failure; no other lint step failed.