Skip to content

fix(compiler): honor disabled optimizations for array indexing - #677

Merged
e54-bot merged 3 commits into
mainfrom
fix/672-disable-opt-indexing
Oct 29, 2026
Merged

e54-bot merged 3 commits into
mainfrom
fix/672-disable-opt-indexing

Conversation

@e54-bot

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

Copy link
Copy Markdown
Contributor

Under #!disableOptimizations, arr[0] and arr[0.0] emitted First Of while pinned OverPy 9.7.17 emits Value In Array. Index expressions now lower through valueInArray, leaving the existing value optimizer to select firstOf when enabled.

Adds a canonical differential fixture for integer/decimal zero and nonzero indexing, including default, disabled, and re-enabled optimization states. The ablation pass removed the redundant lowering rewrite instead of adding a separate optimization-state branch. No new mechanism or dependency is needed.

Fixes #672

Verification

All commands completed in the foreground:

  • pnpm install --dir tools/overpy/oracle — installed pinned OverPy 9.7.17.
  • cargo build --locked -p opy-cli --features compatibility --bins — passed (CARGO_INCREMENTAL=0).
  • Pinned oracle CLI compilation followed by target/debug/opy-compat project-compare --source <scratch>/source.opy --root <scratch> --reference <scratch>/reference.workshop — before editing, canonical mismatch firstOf versus valueInArray; after the final change, equivalent: true, zero differences. Scratch files are under /Users/teakowa/Repos/wrightkit/.task-scratch/orch/scratch-672.
  • python3 tools/overpy/run_oracle.py --fixture synthetic/structural-index-optimization --update, then the same command without --update — snapshot generated and independently rechecked against the pinned oracle; passed.
  • cargo test -p opy-rs --test differential — passed, including the new fixture.
  • 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; one existing ignored test.
  • python3 -m unittest discover -s tools/overpy/tests — 52 passed.
  • python3 tools/overpy/run_native.py --binary target/debug/opy-cli --semantic-binary target/debug/opy-compat — 145 matches, 15 recorded gaps, zero unexpected divergences; new fixture canonically matches.
  • python3 tools/overpy/probe_builtins.py --binary target/debug/opy-compat --jobs 4 — 6,114 probes, zero unexplained findings.
  • git diff --check and git diff --cached --check — passed.

Unverified

Hosted CI and independent PR review have not completed. Existing recorded corpus/probe gaps remain; this PR makes no broader compatibility claim. UI gates do not apply because no UI files changed.

Lower index expressions through valueInArray and let the existing value
optimizer choose firstOf only when optimizations are enabled.

Add a pinned-oracle differential fixture covering integer and decimal zero
indices, nonzero indices, and default/disabled/re-enabled optimization states.

Fixes #672
@e54-bot

e54-bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Ready for independent review of commit 7304cf3 on fix/672-disable-opt-indexing.

Issue #672 is implemented and pushed. Before editing, the pinned OverPy 9.7.17 canonical comparison reproduced firstOf versus valueInArray; the final implementation compares equal with zero differences. The new differential fixture covers disabled integer/decimal zero indexing, nonzero indexing, and enabled/re-enabled controls.

All applicable local gates completed in the foreground: formatting, stable and Rust 1.88.0 Clippy, full workspace tests, differential corpus, 52 Python tests, native corpus comparison (145 matches / 15 recorded gaps / zero unexpected divergences), and 6,114 builtin probes (zero unexplained findings). Ablation removed the duplicate lowering rewrite and reused the existing optimizer. A single-pass simplify review found no additional changes needed.

Hosted CI and independent review remain pending. Please verify the issue acceptance criteria and optimization-state controls against the pinned oracle; no merge has been performed.

@e54-bot

e54-bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Fixed finding 1 from the review (672-r1.md) in commit ff50b54 (fix(compiler): preserve strict empty-array zero indexing (#672)), pushed to fix/672-disable-opt-indexing.

Strict empty-array indexing now reaches the existing zero-index optimizer rewrite, emitting canonical firstOf([]) for both [][0] and [][0.0]. Disabled optimization retains valueInArray, and strict nonzero indexing remains unchanged. Extended the existing synthetic/structural-index-optimization differential fixture with these boundaries and regenerated/rechecked its snapshot against pinned OverPy 9.7.17. No cleanup or redesign was included.

Verification completed in the foreground on the final implementation:

  • The extended fixture reproduced the mismatch before the fix; after the fix, the direct canonical oracle comparison reports equivalent: true, zero differences.
  • Oracle fixture regeneration and independent recheck passed.
  • cargo fmt --all -- --check passed.
  • Stable and Rust 1.88.0 Clippy passed with --workspace --all-targets --all-features -- -D warnings.
  • cargo test --workspace --all-targets --all-features passed, including differential tests; one existing manual resource-baseline test remains ignored.
  • Python harness tests: 52 passed.
  • Native corpus: 145 matches, 15 recorded gaps, no unexpected divergences.
  • Builtin probe: 6,114 probes, zero unexplained findings.
  • Staged diff checks and commit/push hooks passed.

All findings in the review file are fixed; none were deferred or contradicted the approved issue/architecture. There are no GitHub review threads to resolve. PR #677 is ready for follow-up review of ff50b54. Hosted CI and independent follow-up review remain pending; no merge was performed.

@e54-bot

e54-bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Fixed round-2 finding 1 (672-r2.md) in commit 9bba189 (fix(compiler): preserve strict empty-array fractional indexes (#672)), pushed to fix/672-disable-opt-indexing with the explicit refspec HEAD:refs/heads/fix/672-disable-opt-indexing.

Strict empty-array indexing now selects firstOf only for an original numeric index of exactly zero. Nonzero fractions such as 0.1 and 0.49 retain their original values in valueInArray. Added both cases to the existing synthetic/structural-index-optimization differential fixture and regenerated/rechecked its snapshot against pinned OverPy 9.7.17. The correction stays within the existing index optimizer; its numeric handling was folded into a match to satisfy the existing no-growth size guard without bypassing hooks or extracting modules.

Verification completed in the foreground on the final code:

  • Direct canonical comparison of the extended fixture against freshly compiled pinned OverPy output: equivalent: true, zero differences; existing integer/decimal zero and disabled/re-enabled controls remain covered.
  • Oracle fixture regeneration and recheck passed.
  • cargo fmt --all -- --check passed.
  • Stable and Rust 1.88.0 Clippy passed with --workspace --all-targets --all-features -- -D warnings.
  • cargo test --workspace --all-targets --all-features passed, including differential tests; one existing ignored test remains.
  • Python harness tests: 52 passed.
  • Native corpus: 145 matches, 15 recorded gaps, no unexpected divergences.
  • Builtin probe: 6,114 probes, zero unexplained findings.
  • Size guard, staged diff checks, commit hooks, and push hooks passed.

The sole requested round-2 finding is fixed; none was deferred or contradicted the approved issue or architecture. No GitHub review threads exist to resolve. PR #677 is ready for follow-up review of 9bba189. Hosted CI and independent follow-up review remain pending. No merge was performed.

@e54-bot
e54-bot merged commit df61222 into main Oct 29, 2026
8 checks passed
@e54-bot
e54-bot deleted the fix/672-disable-opt-indexing branch October 29, 2026 12:42
@e54-bot e54-bot mentioned this pull request Oct 29, 2026
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: disableOptimizations emits First Of for arr[0]

2 participants