Skip to content

refactor(opy-rs): simplify redundant compiler paths - #406

Merged
Teakowa merged 67 commits into
mainfrom
codex/massive-simplification
Sep 28, 2026
Merged

Teakowa merged 67 commits into
mainfrom
codex/massive-simplification

Conversation

@e54-bot

@e54-bot e54-bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR contains the accumulated, behavior-preserving opy-rs simplifications accepted by the user as the completed scope of the current task. The original 30% LOC target was not reached; the measured result is stated below. Earlier work removed discarded numeric text and ignored span plumbing from lowering, reused shared HIR traversal queries, consolidated loop action envelopes, removed infallible wrappers and redundant parser token clones, split lowering by responsibility, shared provider UTF-16 position conversion, simplified settings parsing, removed private HIR validation forwarding helpers, and centralized repeated compiler, parser, and preprocessing construction paths.

Recent changes consolidate global/player variable lookup and switch/conditional-jump lowering, share compiler integration-failure and initializer-order diagnostics, and unify alias resolution, annotation spans, binary-expression construction, delimited-item parsing, lexer token/identifier handling, quoted settings scanning, HIR dump formatting, and CLI diagnostic conversion. Operator folds reuse existing unary/binary helpers; compiler passes share Workshop Value child traversal; compression/decompression share alphabet and component decoding; rule-name renderers share their template value schema; compiler and lowering share directive lookup. String merging no longer keeps a name-only token forwarder, Blizzard Global spacing reads widths from its canonical glyph table rather than building a temporary duplicate, reconstruction uses the lexer identifier rule, cased progress-bar text lowering now lives with its sole action-call consumer, and declaration lowering lives in a dedicated submodule. Declaration allocation checks implicit slot collisions and gathers global/player/subroutine entries in one source-order pass. Named allocation reserves special high slots and normal allocation through one entry point; declaration plans now own their names directly instead of building and converting an intermediate borrowed list. Macro expansion now mutates the already-private HIR clone in place, retaining statement-list splicing and macro replacement while removing per-variant reconstruction. The expander borrows source macro templates and parameter names, keeping only the mutable body copy needed for each expansion. Source-to-HIR rule formatting and compiler rule/settings formatting now share one streaming filter for the same invisible characters. Global and player initializer lowering now reuse the existing generated-rule path. Rule insertion and rule/condition/action provenance registration now share one compiler path. Logical and/or folding now shares one branch implementation while preserving strict and non-strict operand-return behavior. Literal string extraction and numeric vector extraction now have one shared implementation reused by both optimizer passes. Size optimization now shares the exact numeric 0/1 to Boolean conversion across loop bounds, modification operands, indexed-variable arguments, and chase values. Rule and subroutine parsing now carry annotation/directive results in one state value instead of six mutable arguments; this also removes a too_many_arguments exemption and discarded subroutine-state bindings. Reverse-emitter action and value calls now share manifest/member lookup and kind diagnostics, reuse one manifest-call path, and remove the forwarding-only member-action wrapper. Settings expression evaluation now indexes HIR constants by borrowed expressions, removing one deep-copy map and a second reference map. HIR validation now visits each condition once instead of recursing through both the generic child fields and a second condition path. Cased-glyph records now construct their shared five-field shape once per character; the alphabet output was compared with the pinned OverPy oracle. Logical or and and parsers now share their left-associative chain loop. Lexer, f-string, and settings parsing share one simple escape map; macro argument substitution reuses the lexer identifier-continuation rule. Rule, declaration, and control-flow parser headers now share colon/body-indent handling; tabular and splitDictArray now share compression component-shape and magnitude checks. Declaration source-order visibility now delegates to one shared predicate while subroutine definitions retain their separate visibility rule. HIR program traversal now has one canonical walker reused by tooling. Indexed expression decomposition now returns one canonical root and source-order indices, replacing the compiler recursive target splitter and the source lowerer duplicate walk. Synthetic texture setup HIR now uses small constructors for spanless calls, receiver calls, and statements instead of repeating the same AST fields through the whole generated rule. The macro JavaScript runtime now calls its sole QuickJS implementation directly, removing the unused single-implementation JsEngine trait and its replaceability claim. Self-modification detection now borrows the operand list and clones only the value kept in the rewritten action. Reverse reconstruction reuses its subroutine-name set instead of scanning the source table again, and compile reports move their exact text after normalization instead of cloning it. HIR expression and statement types own both read and write span selection; macro expansion uses those mutable accessors instead of maintaining backend-specific setters. Delimited call/event argument, array, and dictionary parsing now share the separator loop while each item parser keeps its own syntax and diagnostics. Reverse reconstruction shares disabled-rule diagnostics across subroutine and normal rules, and emits global/player variable tables through one formatter. No test, fixture, snapshot, or validation files were changed.

Acceptance status

At the user's direction, the accumulated result is accepted as the completed scope of this task, although the original 30% target was not reached. Using the established scope and method (Rust files under crates, excluding tests and benches), the count moved from 41,202 to 39,151 lines: 2,051 net lines removed (4.98%). Reaching the original target would require at least 10,310 further net deletions. The final candidate sweep found no broad deletion at that scale that preserves the requested behavior and immutable evaluation boundaries; apparent large repetitions belong to separate CST/HIR schemas, distinct compiler phases, or context-specific lowering.

Verification

  • cargo fmt --all -- --check, git diff --check, cargo test --workspace --all-targets --all-features, and cargo clippy --workspace --all-targets --all-features -- -D warnings on exact source at 2773225 (539 passed, 1 ignored across 12 suites)
  • All five hosted CI jobs on 129c209 passed: stable Rust, Rust 1.85, compiler compatibility, and both JS runtime jobs
  • All five hosted CI jobs on 0ffdfea passed: stable Rust, Rust 1.85, compiler compatibility, and both JS runtime jobs
  • Hosted CI on e80ada8 was superseded before every job completed: stable Rust, Rust 1.85, and macOS passed; compiler compatibility and Windows were still in progress
  • All five hosted CI jobs on 481be7e passed: stable Rust, Rust 1.85, compiler compatibility, and both JS runtime jobs
  • All five hosted CI jobs on c9217c0 passed: stable Rust, Rust 1.85, compiler compatibility, and both JS runtime jobs
  • All five hosted CI jobs on 135a633 passed: stable Rust, Rust 1.85, compiler compatibility, and both JS runtime jobs
  • All five hosted CI jobs on 2773225 passed: stable Rust, Rust 1.85, compiler compatibility, and both JS runtime jobs
  • Full pinned builtin probe on exact source 9e9039d: 5,152 probes (match: 4,208; both-reject: 586; native-accepts: 334; native-rejects: 24), 0 unexplained differences; the two approved gaps matched 12 probes each. The later 863cc56 change is limited to parser directive-state grouping. 69 manifest functions had no valid sample call.
  • Differential opy-compat project-compare during the macro in-place refactor on 9 successful macro-bearing real-world corpus projects: old and new binaries produced identical structural comparison results (4 matched the pinned oracle; 5 retained the same existing differences)
  • cargo test -p opy-rs --lib texture (3 passed, including the pinned texture setup oracle comparison)
  • python3 -m unittest discover -s tools/overpy/tests (37 passed on exact source at 2773225)
  • Pinned OverPy 9.7.10 structural comparisons for the all-letter cased-progress, mixed-precedence and/or plus f-string escape, escaped settings string, and macro identifier-boundary inputs all reported equivalent with no differences

Reviewed candidate retained

opy-provider::Server::handle_message still returns Option<Value> although current production branches all return a response. Its in-module test directly calls .expect() on the result; changing that call would modify an immutable test, so the wrapper remains.

@e54-bot
e54-bot marked this pull request as ready for review September 28, 2026 07:15

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One behavior regression remains in the public HIR validator; details inline.

Comment thread crates/opy-rs/src/hir/validate.rs Outdated
@e54-bot
e54-bot requested a review from Teakowa September 28, 2026 09:40

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The HIR validation error precedence regression is fixed, the regression test covers both Program::validate() and parse_value(), and CI is green on the updated head.

@Teakowa
Teakowa merged commit 396f489 into main Sep 28, 2026
5 checks passed
@Teakowa
Teakowa deleted the codex/massive-simplification branch September 28, 2026 09:50
@e54-bot e54-bot mentioned this pull request Sep 28, 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.

2 participants