refactor(minifier): centralize reorder stability checks#24699
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merging this PR will not alter performance
Comparing Footnotes
|
5ef3e63 to
5c65a14
Compare
Merge activity
|
Single-use substitution checked the same reorder-safety decision in separate forms for ordinary identifier reads and member assignment targets. That duplication made future stability rules easy to apply to one consumer but miss in another. Route both consumers through shared resolved-symbol stability and closed-over block-scoped-read predicates. Each identifier reference is resolved once, after which the symbol-level checks share the result. The refactor preserves three non-obvious boundaries: unresolved references remain blocked through `is_none_or`, non-simple member objects remain conservative through the `_ => true` arm, and the inverted ordinary-read condition is equivalent to the previous `resolved && !mutated && !closed_over` check. The closed-over predicate is named for what it structurally proves rather than implying that every such read is currently in the TDZ. TDZ fixtures use parameter callees so future import handling cannot make them pass vacuously. There is no behavior or output change. Verified with 621 passing `oxc_minifier` tests and 42 ignored tests, clippy with warnings denied, formatting, `just minsize`, and allocation regeneration; minsize and allocation snapshots are byte-identical. ## Stack 1. **#24699 — centralize reorder stability checks** 2. #24698 — guard reordered identifier reads AI assistance: Codex and Claude were used for implementation, adversarial review, and verification. The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.
5c65a14 to
6e92cc0
Compare
A locally read-only identifier is not necessarily stable across a side-effecting call. ESM imports are live bindings, Script-root bindings can be changed by another script, and computed assignment keys bypassed the existing closed-over-TDZ guard. Single-use substitution could therefore move a binding read before a call and change program behavior. The same incomplete stability query affected logical-assignment folding. A live imported member base could be cached before a call that reassigns the export, making `||=` or `??=` update the old object instead of the current binding. Build on the shared reorder predicates from #24699. Imports, Script-root bindings, unresolved references, syntactically mutated symbols, and closed-over block-scoped reads are treated conservatively across ordinary reads, assignment-target objects, computed keys, and member-base folding. Stable module bindings and pure replacements remain optimizable. ## Example ```js var key = "x"; function init(obj) { let value = mutate(); obj[key] = value; } ``` The previous substitution could produce: ```js var key = "x"; function init(obj) { obj[key] = mutate(); } ``` That captures `key` before `mutate()` runs, although another script can change the global binding during the call. The declaration is now retained so the read remains after `mutate()`. For imported member bases, main can rewrite `x.y || (mutate(), x.y = 3)` to `x.y ||= (mutate(), 3)`. When `mutate` reassigns the exported `x`, the original two-module runtime repro produces `3`, while the rewritten program produces `0`. The shared stability guard now prevents that fold. Plain computed assignments still inline stable identifiers and literals. ECMAScript evaluates the key expression and `GetValue` before the right-hand side, but defers `ToPropertyKey` until the final `PutValue`, so arbitrary key expressions remain conservative without losing safe simple-key substitutions. This improves the existing reorder guard; it does not add a general TDZ model. Sloppy mapped `arguments` mutation remains a pre-existing unsupported case and is outside this stack. Verified with the full `oxc_minifier` suite (622 passed, 42 ignored), clippy with warnings denied, formatting, minsize, allocation snapshots, runtime ordering probes, and repeat-compression checks. `cargo minsize` measured 18 additional raw bytes across `echarts.js` and `typescript.js`, with no iteration-count changes. System-allocation metrics are unchanged; arena changes are dominated by retained code with minor fold-path shifts. ## Stack 1. #24699 — centralize reorder stability checks 2. **#24698 — guard reordered identifier reads** AI assistance: Codex and Claude were used for implementation, adversarial review, and verification. The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.

Single-use substitution checked the same reorder-safety decision in separate forms for ordinary identifier reads and member assignment targets. That duplication made future stability rules easy to apply to one consumer but miss in another.
Route both consumers through shared resolved-symbol stability and closed-over block-scoped-read predicates. Each identifier reference is resolved once, after which the symbol-level checks share the result. The refactor preserves three non-obvious boundaries: unresolved references remain blocked through
is_none_or, non-simple member objects remain conservative through the_ => truearm, and the inverted ordinary-read condition is equivalent to the previousresolved && !mutated && !closed_overcheck. The closed-over predicate is named for what it structurally proves rather than implying that every such read is currently in the TDZ. TDZ fixtures use parameter callees so future import handling cannot make them pass vacuously.There is no behavior or output change. Verified with 621 passing
oxc_minifiertests and 42 ignored tests, clippy with warnings denied, formatting,just minsize, and allocation regeneration; minsize and allocation snapshots are byte-identical.Stack
AI assistance: Codex and Claude were used for implementation, adversarial review, and verification. The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.