fix(es/minifier): bound arguments parameter injection - #12053
Conversation
🦋 Changeset detectedLatest commit: 8819888 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will not alter performance
Comparing 
Footnotes
|
cf94dee to
d88a00c
Compare
**Description:** ECMAScript bitwise NOT first applies `ToInt32`, including wraparound for out-of-range numbers. Folding with direct Rust casts does not reproduce those conversions at the boundaries. This PR delegates numeric bitwise NOT folding to the existing `JsNumber` implementation and adds coverage for boundary, wraparound, and very large values. The simplifier now uses the same number semantics as the rest of the optimizer. **Related issue (if exists):** - Close: #12062 --- - #12043 - #12047 - #12050 - #12048 - #12051 - #12052 - #12053 - #12054 - #12055 - #12056 - #12057 - **#12058** (current) - #12049 - #12059 - #12060
**Description:** Several property-key optimizations rely on Rust number formatting or eagerly parse strings as numbers. This can merge distinct ECMAScript keys, including `Infinity` and `inf`, `-0` and `0`, or `"01"` and `"1"`. This PR introduces shared canonical-index parsing and ECMAScript number-to-string conversion, then uses them across property-key transforms, literal folding, and TypeScript name extraction. Each path now preserves property-key identity before optimizing it. **Related issue (if exists):** - Close: #12061 --- - #12043 - #12047 - **#12050** (current) - #12048 - #12051 - #12052 - #12053 - #12054 - #12055 - #12056 - #12057 - #12058 - #12049 - #12059 - #12060
…12049) **Description:** Enum evaluation currently rebuilds computed `NaN` and infinity values as identifiers. That turns numeric constants into apparent references and loses number-literal semantics as values move through the TypeScript transform and fast-DTS evaluators. This PR keeps computed enum numbers as `Number` literals in both paths, applies JavaScript number stringification when names are required, and corrects modulo evaluation for non-finite operands. **Related issue (if exists):** - #12043 - #12047 - #12050 - #12048 - #12051 - #12052 - #12053 - #12054 - #12055 - #12056 - #12057 - #12058 - **#12049** (current) - #12059 - #12060 Co-authored-by: Donny/강동윤 <kdy.1997.dev@gmail.com>
3a20526 to
fd17b86
Compare
d88a00c to
71aa787
Compare
**Description:** `Number#toString(radix)` folding currently converts integer values through `u128`. Unsafe integers can already have rounded representations, so this conversion can produce digits that do not match JavaScript for non-power-of-two radices. This PR restricts radix folding to safe integers, with a separate exact path for supported power-of-two radices below the `u128` bound. Values outside those limits remain for runtime evaluation. **Related issue (if exists):** - #12043 - #12047 - #12050 - #12048 - #12051 - #12052 - #12053 - #12054 - #12055 - #12056 - **#12057** (current) - #12058 - #12049 - #12059 - #12060
**Description:** `Array(length)` throws a `RangeError` when the length is fractional or otherwise invalid. Folding every small numeric length into an array literal suppresses that observable exception. This PR requires candidate lengths to be integers before applying the literal-folding optimization and adds runtime coverage for invalid lengths. Valid small arrays are still folded as before. **Related issue (if exists):** - #12043 - #12047 - #12050 - #12048 - #12051 - #12052 - #12053 - #12054 - #12055 - **#12056** (current) - #12057 - #12058 - #12049 - #12059 - #12060
**Description:** Computed `arguments` properties are currently parsed or cast directly as numeric indices. Non-canonical names and invalid indices can therefore be mistaken for parameter positions and replaced incorrectly. This PR resolves `arguments` slots through the shared canonical-index parser and ECMAScript number-to-string conversion before substituting parameters. An access is optimized only when its property denotes the same canonical argument index. **Related issue (if exists):** - Closes: #12066 --- - #12043 - #12047 - #12050 - #12048 - #12051 - **#12052** (current) - #12053 - #12054 - #12055 - #12056 - #12057 - #12058 - #12049 - #12059 - #12060
**Description:** `String.fromCharCode` applies ECMAScript `ToUint16` to every argument. The current floor-and-cast folding path does not match that behavior for negative, wrapping, or non-finite inputs. This PR adds `JsNumber::to_uint16` and uses it before folding ASCII `String.fromCharCode` results. Boundary coverage keeps the optimization limited to values whose output can be produced safely. **Related issue (if exists):** - #12043 - #12047 - #12050 - #12048 - #12051 - #12052 - #12053 - #12054 - **#12055** (current) - #12056 - #12057 - #12058 - #12049 - #12059 - #12060
**Description:** Enum template evaluation currently reads raw template source and converts atoms lossily. Escapes, interpolation, and lone surrogates can therefore diverge from the cooked JavaScript string used at runtime. This PR evaluates template quasis and static template member keys from cooked WTF-8 values in both the transform and fast-DTS paths. Lookup and concatenation retain lone surrogates without changing the surrounding enum evaluation model. **Related issue (if exists):** - #12043 - #12047 - #12050 - #12048 - #12051 - #12052 - #12053 - #12054 - #12055 - #12056 - #12057 - #12058 - #12049 - **#12059** (current) - #12060
**Description:** JavaScript string property indexing uses UTF-16 code units. Existing constant folding checked string-literal indices against WTF-8 byte length, which can treat out-of-bounds indices as valid for non-ASCII BMP characters, astral characters, and lone surrogates. In the expression simplifier, that mismatch could also reach an `unreachable!` branch. This PR validates and reads string-literal indices using UTF-16 code units in both the minifier and expression simplifier. It preserves the WTF-8 byte-length check as a fast upper-bound rejection, folds valid indices to the correct code unit, and handles UTF-16 out-of-bounds indices according to each pass's existing semantics. Coverage includes BMP non-ASCII characters, astral surrogate pairs, and lone surrogates. The change was verified with: - `cargo test -p swc_ecma_minifier -p swc_ecma_transforms_optimization` - `(cd crates/swc_ecma_minifier && ./scripts/exec.sh)` - `cargo fmt --all` - `cargo clippy --all --all-targets -- -D warnings` **Related issue (if exists):** - #12043 - #12047 - #12050 - #12048 - #12051 - #12052 - #12053 - **#12054** (current) - #12055 - #12056 - #12057 - #12058 - #12049 - #12059 - #12060
**Description:** The optimizer currently represents `NaN` and infinities in several forms, including identifiers, divisions, and numeric literals. As these forms move through multiple optimization passes, they can be folded inconsistently and acquire incorrect truthiness or stringification. This PR canonicalizes non-finite optimization results as raw-free `Lit::Num` values. Minifier and simplifier helpers then preserve ECMAScript behavior for these numbers, while codegen remains responsible for choosing valid output syntax. **Related issue (if exists):** - #12043 - #12047 - #12050 - **#12048** (current) - #12051 - #12052 - #12053 - #12054 - #12055 - #12056 - #12057 - #12058 - #12049 - #12059 - #12060
**Description:** The JSON parse optimization had a number fast path that cast through `i64`. That path could be unsafe for out-of-range values and for `-0`, changing observable results. This PR keeps the optimization but only uses the `i64` path when safe: when the number is an integer within `i64` range and does not alter the zero sign. Other finite numbers are serialized via `serde_json::Number::from_f64`, and non-finite handling is updated so `NaN` is rejected while `±Infinity` is emitted as `±2e308`. **Related issue (if exists):** - #12043 - #12047 - #12050 - #12048 - **#12051** (current) - #12052 - #12053 - #12054 - #12055 - #12056 - #12057 - #12058 - #12049 - #12059 - #12060
71aa787 to
8819888
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8819888f55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| /// Estimates the bytes added to a parameter list by single-byte generated | ||
| /// bindings and their separators. |
There was a problem hiding this comment.
When compression runs without mangling and keep_fargs is false, these generated identifiers are emitted as argument_0, argument_1, etc., rather than as single-byte bindings. For example, replacing arguments[5] in a parameterless function passes this estimate because six bindings plus separators are counted as 11 bytes, but the emitted parameter list alone is over 60 bytes and the access becomes the ten-byte argument_5, so the optimization still substantially grows output. Use the actual generated-name sizes, or apply the single-byte estimate only when mangling guarantees it.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
| let Some(idx) = argument_access_index(n) else { | ||
| return; | ||
| }; | ||
| let max_growth = n.size(self.unresolved_ctxt).saturating_sub(1); |
There was a problem hiding this comment.
When the index is a non-power-of-ten integer such as 9, SizeWithCtxt overestimates the literal because the current f64::size reports two bytes for this one-byte value. With mangling enabled, a function that already has four parameters therefore treats six added parameters (12 bytes including separators) as equal to the estimated savings from arguments[9], even though replacing the actual 12-byte access with a one-byte identifier saves only 11 bytes, so the supposedly guarded transformation grows output. Derive max_growth from the canonical emitted index length rather than the rough general expression-size heuristic.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
| }; | ||
|
|
||
| // We visit body two time, to use simpler logic in `inject_params_if_required` | ||
| // Visit the body twice to keep parameter injection local to each access. |
There was a problem hiding this comment.
With mangling enabled and accesses ordered as arguments[15], arguments[10], and arguments[5] in a parameterless function, the first traversal rejects the first two accesses and injects through index 5; the second traversal still rejects index 15 before injecting through index 10, then stops even though another traversal would now profitably inject through index 15. Longer descending sequences make the result traversal-order dependent and can leave arbitrarily many profitable replacements unapplied, so repeat until a traversal adds no parameters or determine the profitable injections before mutating the body.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
1 participant
Description:
Replacing an
argumentsaccess with a named parameter is not always smaller. A large index can require injecting many dummy parameters and separators, making the minified output substantially larger.This PR estimates the complete parameter-list growth with checked arithmetic and performs the substitution only when it is size-profitable. The existing optimization remains available for compact indices without allowing unbounded output growth.
Related issue (if exists):
argumentsindex #12108