[rust-compiler] Apply compiler renames to SWC module after conversion#36509
Merged
Conversation
github-actions
Bot
added
the
React Core Team
…ntend
After the cluster-1 BytePos shift, `ConvertCtx::position()` emitted
`loc.column` and `loc.index` as 0-based UTF-8 byte offsets. Babel emits
them as 0-based UTF-16 code unit offsets (matching JS string indexing).
For files containing any character above U+FFFF (e.g. an emoji like
🔴 U+1F534), the two diverge by +2 per such character because the
char is 4 bytes in UTF-8 but 2 code units in UTF-16.
Precompute a `utf16_offsets: Vec<u32>` table in `ConvertCtx::new`
that maps each source byte index to its 0-based UTF-16 code unit
offset. `position()` then looks up `index` directly and computes
`column` as `index - utf16_index_of_line_start`. O(1) per call; the
table costs ~4× the source length in memory, which is bounded for
fixture/file inputs.
Considered an alternative that walks the source line on each
`position()` call to count UTF-16 code units. More memory-frugal but
O(line length) per call. The precomputed table wins on O(1) lookup
and the per-call cost matters because `position()` is invoked on
every node, comment, and reference in the converter.
Clamp the byte index in `position()` to the sentinel at
`utf16_offsets.len() - 1`. Synthetic spans (e.g. compiler-generated
imports given `BytePos(1)`) can point past EOF in degenerate cases;
clamping avoids a panic.
Line numbers stay 1-based and the binary-search remains keyed on
byte offsets, since the underlying `line_offsets` table is byte-based.
Fixes 4 e2e parity fixtures (3 targeted + 1 latent):
- effect-derived-computations/invalid-derived-computation-in-effect.js
- error.invalid-derived-computation-in-effect.js
- fbt/error.todo-multiple-fbt-plural.tsx
- (one additional latent fixture passes for free)
Test plan:
- bash compiler/scripts/test-e2e.sh --variant swc:
Before: Total 1770/1795
After: Total 1774/1795 (4 fixed)
- bash compiler/scripts/test-e2e.sh --variant babel: 1788/1795 (unchanged)
- bash compiler/scripts/test-e2e.sh --variant oxc: 1702/1795 (unchanged)
- cargo test --workspace: 56 passed, 0 failed
…e info
`convert_scope.rs::visit_fn_decl` allocated a fresh `BindingId` for
every `function x()` declaration. Babel and OXC collapse same-named
function declarations in the same hoist scope into one binding, with
the second declaration registered as a `constantViolations` reference
(reassignment) rather than a new binding.
For `function-declaration-redeclare.js` the SWC variant emitted
`const x_0 = t0; return x_0;` because the compiler saw two distinct
bindings and renamed one. Babel's output is `let x; ... x = t0;
return x;` because there is one binding that gets reassigned.
In `visit_fn_decl`, check whether the hoist scope already has a
binding for the name. If yes, record the redeclaration's ident
position in a new `redeclaration_refs` map and skip adding a fresh
binding. `build_scope_info` overlays this map onto
`reference_to_binding` so the second function's ident resolves to the
first binding's `BindingId`.
Fixes 1 e2e parity fixture:
- function-declaration-redeclare.js
`valid-setState-in-effect-from-ref-function-call.js` and its sibling
`valid-setState-in-useEffect-controlled-by-ref-value.js` still fail.
Those have a distinct root cause: the SWC frontend discards the
compiler's `renames` output (`lib.rs:91-98`) instead of applying it
to the emitted SWC AST the way the babel adapter does via
`applyRenames`. That fix is its own commit.
Test plan:
- bash compiler/scripts/test-e2e.sh --variant swc:
Before: Total 1774/1795
After: Total 1775/1795 (1 fixed)
- bash compiler/scripts/test-e2e.sh --variant babel: 1788/1795 (unchanged)
- bash compiler/scripts/test-e2e.sh --variant oxc: 1702/1795 (unchanged)
- cargo test --workspace: 56 passed, 0 failed
The React Compiler's `RenameVariables` pass records identifier renames
in `CompileResult::Success.renames` as `Vec<BindingRenameInfo>` with
`{original, renamed, declaration_start}`. It does not rewrite the AST
itself; the frontend has to apply them. The Babel adapter does this
via `applyRenames` + `scope.rename()` in `BabelPlugin.ts:206-234`.
The SWC frontend was discarding the field via `..` in the
`CompileResult::Success` destructure, so inner-shadowing renames like
`ref → ref_0` never made it into the emitted output.
Add `apply_renames.rs` with a single pass that mirrors the Babel
semantics:
1. Re-run `build_scope_info` on the post-compile SWC module to get a
binding table and `reference_to_binding` keyed by source position.
2. Match `BindingRenameInfo` entries to bindings by `declaration_start`
and `name`, producing the set of `BindingId`s to rename.
3. Walk `reference_to_binding` to collect every position (declaration
plus references) belonging to a renamed binding.
4. A `VisitMut` rewrites `Ident.sym` at matching `span.lo.0`, expands
`Prop::Shorthand` and `ObjectPatProp::Assign` (with or without a
default) into key-value form so `{ref}` becomes `{ref: ref_0}`
instead of `{ref_0}`, and skips `MemberProp::Ident` so `x.ref`
stays `x.ref`.
Wire into `lib.rs::transform`: destructure `renames` from the
`CompileResult::Success` arm, and if `program_json` is `None` but
`renames` is non-empty (the `@outputMode:"lint"` path) clone the
original input module so we still have something to rewrite.
Fixes 2 e2e parity fixtures:
- valid-setState-in-effect-from-ref-function-call.js (`ref → ref_0`)
- valid-setState-in-useEffect-controlled-by-ref-value.js (`data → data_0`)
Test plan:
- bash compiler/scripts/test-e2e.sh --variant swc:
Before: Total 1775/1795
After: Total 1777/1795 (2 fixed)
- bash compiler/scripts/test-e2e.sh --variant babel: 1788/1795 (unchanged)
- bash compiler/scripts/test-e2e.sh --variant oxc: 1702/1795 (unchanged)
- cargo test --workspace: 56 passed, 0 failed
poteto
force-pushed
the
lauren/swc-07-redeclared-fn-binding
branch
from
812b8e5 to
90b4f64
Compare
poteto
force-pushed
the
lauren/swc-08-apply-renames
branch
from
93883a0 to
78afea4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The React Compiler's
RenameVariablespass records identifier renamesin
CompileResult::Success.renamesasVec<BindingRenameInfo>with{original, renamed, declaration_start}. It does not rewrite the ASTitself; the frontend has to apply them. The Babel adapter does this
via
applyRenames+scope.rename()inBabelPlugin.ts:206-234.The SWC frontend was discarding the field via
..in theCompileResult::Successdestructure, so inner-shadowing renames likeref → ref_0never made it into the emitted output.Add
apply_renames.rswith a single pass that mirrors the Babelsemantics:
build_scope_infoon the post-compile SWC module to get abinding table and
reference_to_bindingkeyed by source position.BindingRenameInfoentries to bindings bydeclaration_startand
name, producing the set ofBindingIds to rename.reference_to_bindingto collect every position (declarationplus references) belonging to a renamed binding.
VisitMutrewritesIdent.symat matchingspan.lo.0, expandsProp::ShorthandandObjectPatProp::Assign(with or without adefault) into key-value form so
{ref}becomes{ref: ref_0}instead of
{ref_0}, and skipsMemberProp::Identsox.refstays
x.ref.Wire into
lib.rs::transform: destructurerenamesfrom theCompileResult::Successarm, and ifprogram_jsonisNonebutrenamesis non-empty (the@outputMode:"lint"path) clone theoriginal input module so we still have something to rewrite.
Fixes 2 e2e parity fixtures:
ref → ref_0)data → data_0)Test plan:
Before: Total 1775/1795
After: Total 1777/1795 (2 fixed)