[rust-compiler] Store unsupported AST nodes typed instead of via serde_json#37
Merged
Conversation
…e_json Replace UnsupportedNode.original_node (Option<serde_json::Value>) with a typed OriginalNode enum so lowering stashes and codegen restores the node without a serde round-trip. This removes the Babel-AST deserializer monomorphization from the binary: react_compiler_reactive_scopes serde codegen drops from ~249K LLVM-IR lines (53% of the crate) to ~0, with no behavior change.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 966ee9c9d3
ℹ️ 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 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 address that feedback".
Member
Author
Binary size (shipped release
|
| build | bytes | size |
|---|---|---|
main |
6,090,416 | 5.81 MiB |
| this PR | 5,677,632 | 5.41 MiB |
| Δ | −412,784 | −6.78% |
Baseline reproduced byte-for-byte across a clean rebuild, so the delta is solid.
This is the deserialize-side reduction alone. The serialize side (identifier_loc_index's whole-body to_value, ~38.9K IR lines) is still in the binary, so the follow-up that removes it would cut further.
AssignmentPattern is shared between PatternLike and Expression (the latter carries it for error-recovery positions), so as_expression must convert it rather than fall back to a placeholder — restoring the prior from_value::<Expression> behavior for that tag. Caught in review.
…s sync Captures the typed OriginalNode change (Rust source under react-compiler/) as a git-apply patch in patches/, which `just patch` re-applies over the freshly-synced upstream tree. Verified it applies cleanly onto main (upstream + codemod + 0001).
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
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.
Problem
cargo llvm-linesshows the Rust compiler's generated code is dominated by serde'sserde::private::de::content::*machinery — the buffering (de)serializer for the#[serde(tag = "type")]+#[serde(flatten)]Babel-AST enums. In the oxc use case (no Babel/JSON boundary), that serde is reached entirely through one internal round-trip:UnsupportedNode.original_node, stored asserde_json::Value, is serialized in lowering (to_value) and re-parsed in codegen (from_value) to re-emit nodes the compiler does not model.Change
Replace
original_node: Option<serde_json::Value>with a typed enum:to_value).from_value).PatternLike::as_expressionreproduces the exactfrom_value::<Expression>coercion for the 7 variantsPatternLikeshares withExpression.hir → astdependency. The field was opaque JSON specifically to keep HIR decoupled from the AST crate; the coupling is already semantic (the field holds a Babel node), and a genericInstructionValue<N>would be far more churn.Impact (
cargo llvm-lines, dev profile)react_compiler_reactive_scopesreact_compiler_loweringThe entire deserialize-side bloat is removed. Lowering's serialize side is unchanged — that is the
identifier_loc_indexwhole-body serialization, a separate follow-up (it carries behavior risk, so it is not bundled here).Behavior preservation
No behavior change. The old code round-tripped
typed → Value → typed, an identity for well-formed nodes, and everyoriginal_nodeis built from a typed node — so the variant already encodes what thetype-tag dispatch resolved to. Covered by the rewritten codegen unit tests, the newPatternLike::as_expressiontests, and theunknown_statement_loweringintegration test.Verification
cargo build --workspace,cargo clippy, and all Rust tests for the touched crates pass.yarn snap --rust) was not run locally: the vendored harness assumes the upstreamcompiler/layout andreact_compiler_*package names, and the snap runner hits atsWatchTDZ under Node 24. Please let CI run the full gate.🤖 Generated with Claude Code