Skip to content

fix(compiler): preserve trailing comparisons under aggressive size optimization - #681

Merged
e54-bot merged 2 commits into
mainfrom
fix/671-optimize-for-size-aggressive
Oct 29, 2026
Merged

e54-bot merged 2 commits into
mainfrom
fix/671-optimize-for-size-aggressive

Conversation

@e54-bot

@e54-bot e54-bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

When both #!optimizeForSize and #!optimizeForSizeAggressive are enabled, a trailing single-statement comparison if incorrectly became Skip If. Restrict the existing tail rewrite to negated conditions so comparisons retain the canonical If / End structure produced by pinned OverPy 9.7.17.

Add the structural-size-aggressive-control-flow differential fixture and replace the old text-based tests with canonical Workshop comparison against its pinned oracle snapshot. The fixture covers default mode, the aggressive directive alone, both directives together, and preserved negation lowering. Ablation removed the obsolete comparison detection/inversion path; no new production mechanism or dependency was added.

Fixes #671

Verification

All gates ran in the foreground. Rust builds used CARGO_INCREMENTAL=0; scratch data and reports are under .task-scratch/orch/scratch-671 outside the worktree.

  • pnpm install --dir tools/overpy/oracle --frozen-lockfile — installed pinned OverPy 9.7.17.
  • cargo build --locked -p opy-cli --features compatibility --bin opy-cli --bin opy-compat — passed.
  • opy-compat project-compare --source <repro.opy> --root <scratch> --reference <oracle.workshop> — canonical workshop-rs 1.28.2 comparison reproduced the original divergence before the fix; after the fix, equivalent: true, zero differences. Both size directives were enabled.
  • python3 tools/overpy/run_oracle.py --fixture synthetic/structural-size-aggressive-control-flow --update — generated the differential snapshot from the pinned oracle.
  • cargo test -p opy-rs --lib aggressive_size_control_flow_matches_the_pinned_oracle — failed before the production fix; passed afterward.
  • cargo fmt --all -- --check — passed.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — passed.
  • cargo +1.88.0 clippy --workspace --all-targets --all-features -- -D warnings — passed.
  • cargo test --workspace --all-targets --all-features — passed, including differential coverage; one existing ignored test.
  • python3 -m unittest discover -s tools/overpy/tests — 52 tests passed.
  • python3 tools/overpy/run_native.py --binary target/debug/opy-cli --semantic-binary target/debug/opy-compat --results <scratch>/native-results --report <scratch>/native-report.json — 170 fixtures: 155 match, 15 known gaps, zero unexpected divergences. The first attempt hit a 60-second tool timeout; the complete foreground rerun passed.
  • python3 tools/overpy/probe_builtins.py --binary target/debug/opy-compat --jobs 4 --report <scratch>/probe-report.json — 6,114 probes, zero unexplained differences. Invoked through a scratch-only wrapper that retains temporary probe directories under the requested scratch root; probe generation and comparison were unchanged.
  • python3 tools/overpy/structural_gate.py --binary target/debug/opy-compat --workdir <scratch>/structural-gate --report <scratch>/structural-report.json — both pinned Bastion entry points passed, zero unrecorded differences.
  • git diff --check — passed.

Limitations

UI gates do not apply: this repository has no ui/ directory. Live game behavior and remote CI were not verified locally.

An exploratory check combining #!disableOptimizations with both size directives found a separate pre-existing upstream/native control-flow difference. That combination is outside the reported reproduction and remains unchanged; this PR makes no parity claim for it.

…timization

Restrict the tail Skip If rewrite to negated conditions so comparison
conditions retain the pinned OverPy canonical control-flow structure.
Replace text-based expectations with an oracle-backed differential fixture
covering default directives, aggressive alone, combined directives, and
preserved negation lowering.

Fixes #671
@e54-bot

e54-bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Addressed review finding 1 (P2) from 671-r1.md in commit d750d71, pushed to fix/671-optimize-for-size-aggressive.

The trailing aggressive size rewrite now requires the optimized condition to remain a genuine negation. Negated and double-negated comparisons retain the pinned oracle's canonical If structure; not g retains its matching rewrite. The existing structural-size-aggressive-control-flow differential fixture now covers all six comparison operators with both single and double negation. A small predicate helper beside rules.rs keeps that existing oversized file within the enforced no-growth limit. No cleanup or redesign is included.

Verification on the pushed code, all run in the foreground:

  • The expanded canonical regression failed before the fix and passed afterward. The rebuilt opy-compat project-compare reports equivalent: true, zero differences, comparing workshop-rs 1.28.2 canonical programs against the regenerated pinned OverPy 9.7.17 fixture snapshot. No textual output comparison was used for this finding.
  • cargo fmt --all -- --check passed.
  • Stable and Rust 1.88.0 cargo clippy --workspace --all-targets --all-features -- -D warnings passed.
  • cargo test --workspace --all-targets --all-features passed (one existing ignored test).
  • python3 -m unittest discover -s tools/overpy/tests passed (52 tests).
  • Native corpus: 155 matches, 15 known gaps, zero unexpected divergences.
  • Builtin-call probe: 6,114 probes, zero unexplained differences.
  • Pinned Bastion structural gate: both entry points passed, zero recorded or unrecorded differences.
  • Size guard and git diff --check passed. Python temporary directories were retained under the authorized scratch directory through a scratch-only runner.

No supplied findings remain unaddressed and no issue/architecture mismatch was found. There are no GitHub review threads to resolve; the finding was supplied in the external review file. The PR is ready for follow-up review. This comment is the explicit handoff for the external review workflow.

Remote CI and live game execution have not been verified by this local run.

@e54-bot
e54-bot merged commit 018d3e7 into main Oct 29, 2026
8 checks passed
@e54-bot
e54-bot deleted the fix/671-optimize-for-size-aggressive branch October 29, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

opy-rs: optimizeForSizeAggressive emits Skip If for a trailing comparison if

2 participants