Merge train 211: keep observable user code out of a folded builder (v0.5.1589)

[proggeramlug]

This train lands #10361 as v0.5.1589, on d83e0414e1.

• fix(hir): keep observable user code out of a folded builder (#10357) #10361 (#10357) — keeps observable user code out of a folded builder. The builder fold may skip statements between a {} binding and its stores, which sinks the allocation below them; this pins that folding cannot move anything a program can observe.

The conflict was not a conflict

#10361 reported nine conflicting hunks in crates/perry-hir/src/lower/builder_fold.rs against main. It carries two commits, and the first — 5f86350b96 — is patch-id identical to 2cb9c76f86 already on main (both hash to 9bfff5497276e77d0b3fddf76e7018aa12a24f3f). That is #10353, which the PR forked from before it rebase-merged under a different SHA. Git's diff3 cannot see the two sides are the same patch, so it reports both independently touching the same lines.

The resolution is therefore to skip that commit, not to hand-merge it. Cherry-picking only the real commit applies with zero conflicts. Hand-merging nine hunks of fold-safety logic against a copy of itself was the actual risk here, and the source proof records the skip by finding 5f86350b96's patch-id on main, not by assuming it.

Train repair

The file-size cap. Post-pick, builder_fold.rs is 2118 lines — 118 over the 2000 cap. Split at a pre-existing // --- banner that already separated two independent subsystems: the fold-safety machinery stays, and #6812's compile-time builder WIDTH scan (empty_builder_width_hints and its private helpers) moves to builder_fold_width_hints.rs. 1743 + 385 lines.

Following the method.rs → method_static.rs precedent: a sibling file plus #[path] mod and a pub(crate) use re-export, so super::builder_fold::empty_builder_width_hints at lower_module_fn.rs:925 needs no change. Verified a pure move — 333 code lines out, 334 in, the single addition being the use swc_ecma_ast as ast; the moved block needs. No visibility fixes, no orphaned attributes, no gate JSON names this file.

Validation

Validated head df08e26fe0. Five-package release build pinned and hash-verified, and re-verified after the gap run.

The acceptance evidence is compile-and-run, because it has to be. HIR unit tests pin which shapes fold; only the semantics suites pin that folding them changes nothing a program can see. Both run inside the validated pin:

builder_fold_gap_semantics         5 passed, 0 failed
builder_fold_conversion_semantics  7 passed, 0 failed

• Crate suites: codegen 1571, runtime 3984, stdlib 139, hir 433, transform 137, cli 1139 — all green except main's one known runtime failure.
• All nine preflight gates pass, including the file-size cap after the split.
• Gap: filters builder, fold, object, literal, class, spread. builder, fold and spread are clean — the first two being the most direct for this change.

Reds attributed

Three fixtures A/B'd against main's own artifact set — all identical on both arms, with distinct build stamps per arm: test_issue_4034_object_literal_semantics (node=1, main=0, train=0), test_issue_3580_arguments_object_semantics (node=0, main=0, train=0), test_issue_927_jwt_verify_returns_object (node=1, main=0, train=0). The first matters most: this PR folds builders into object literals, so a prior verdict measured on binaries predating the change would not have transferred.

The three class fixtures need no A/B: node itself exits 1 inside its own module resolution (run_main, package_json_reader, esm/resolve), which is binary-independent, and this train does not touch those fixtures. test_issue58_object_string is listed verbatim in run_parity_tests.sh's SKIP_TESTS on main.

Before merging, the pushed head and unchanged main are checked again. After merging, the rewritten commits are checked for preserved authorship and the main tree must match the validated train exactly.

Summary by CodeRabbit

• Bug Fixes

  • Fixed builder folding to safely handle implicit conversions, including string conversion, arithmetic, comparisons, and template literals.
  • Prevented temporal dead zone errors when user code observes a builder during conversion.
  • Preserved correct behavior across closures, globals, exports, var bindings, and dynamic scope features.

• Performance

  • Improved builder allocation sizing for certain predictable loop patterns.

• Documentation

  • Updated the documented release version to 0.5.1589.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 54cfdbb0-3dce-418b-82b4-1d3b6111d88e

📥 Commits

Reviewing files that changed from the base of the PR and between d83e041 and df08e26.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (7)

• CLAUDE.md
• Cargo.toml
• changelog.d/10361-builder-fold-toprimitive.md
• crates/perry-hir/src/lower/builder_fold.rs
• crates/perry-hir/src/lower/builder_fold_width_hints.rs
• crates/perry-hir/tests/builder_fold_conversion.rs
• crates/perry/tests/builder_fold_conversion_semantics.rs

---

📝 Walkthrough

Walkthrough

Builder folding now analyzes whether conversions can observe a builder before initialization. It restricts unsafe folds, extracts width-hint scanning into a separate module, adds regression tests, and updates the package version and changelog.

Changes

Builder folding soundness

Layer / File(s)	Summary
Observability-aware folding crates/perry-hir/src/lower/builder_fold.rs, changelog.d/10361-builder-fold-toprimitive.md	The folding pass now tracks FoldScope observers and Visible bindings. Converting expressions and gap statements fold only when their reads are safe.
Width-hint scan extraction crates/perry-hir/src/lower/builder_fold.rs, crates/perry-hir/src/lower/builder_fold_width_hints.rs	The width-hint scan moved into a dedicated module. It records bounded widths for supported empty-object build loops.
Regression coverage and release records crates/perry-hir/tests/builder_fold_conversion.rs, crates/perry/tests/builder_fold_conversion_semantics.rs, Cargo.toml, CLAUDE.md	Tests cover observer ordering, binding resolution, conversions, globals, loops, and runtime results. The documented and workspace versions change to 0.5.1589.

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant SourceProgram
  participant BuilderFold
  participant FoldScope
  participant Visible
  participant GeneratedProgram
  SourceProgram->>BuilderFold: provide module AST
  BuilderFold->>FoldScope: scan observers and positions
  BuilderFold->>Visible: collect resolvable bindings
  BuilderFold->>BuilderFold: validate conversions and fold candidates
  BuilderFold->>GeneratedProgram: emit folded or unfurled builder code

Loading

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch train211r

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}