Skip to content

fix(transform): preserve exported aggregate carriers - #9054

Merged
proggeramlug merged 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9053-exported-aggregate
Aug 29, 2026
Merged

fix(transform): preserve exported aggregate carriers#9054
proggeramlug merged 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9053-exported-aggregate

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevents aggregate scalar replacement from deleting arrays of object literals whose bindings are consumed from another module.

Changes

  • Treat local binding IDs recorded by Module.exports or exported_objects as references from a virtual external HIR region.
  • Keep exported aggregate carriers materialized while preserving scalar replacement for genuinely local non-escaping aggregates.
  • Add transform-level coverage for both export metadata paths and a native two-module compile/run regression covering length, iteration, property reads, and repeated-read identity.
  • Add the required PR-numbered changelog fragment without changing version metadata.

Related issue

Closes #9053

Test plan

  • cargo build --release clean (not run)
  • Full workspace test command passes (not run)
  • cargo fmt --all -- --check
  • python3 scripts/check_test_registration.py
  • ./scripts/pre-tag-check.sh --quick
  • cargo test -p perry-transform (120 passed)
  • cargo test -p perry --test issue_9053_exported_aggregate -- --nocapture
  • Added regression tests in the affected transform crate and Perry integration suite
  • Docs not required; no CLI, stdlib, or runtime API changed
  • Platform UI build not applicable

Screenshots / output

The two-module fixture now prints:

length: 2
identity: true
1 one
2 two

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md
  • My commits follow the loose conventional prefix convention
  • I have read CONTRIBUTING.md and agree to the Code of Conduct

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where exported arrays of objects could be incorrectly optimized away.
    • Preserved exported aggregate values and their object identities when consumed across modules.
    • Ensured exported arrays retain their length, iteration behavior, and element values.
    • Added regression coverage for cross-module exports, imports, array length, and object contents.

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 71d42813-a987-401e-84e2-179b87884772

📥 Commits

Reviewing files that changed from the base of the PR and between f7e573e and 6d17b4e.

📒 Files selected for processing (3)
  • changelog.d/9054-preserve-exported-aggregates.md
  • crates/perry-transform/src/aggregate_scalar.rs
  • crates/perry-transform/src/aggregate_scalar_export_tests.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The aggregate scalar pass now treats exported bindings as externally referenced. It preserves exported aggregate carriers and adds unit and integration tests for named exports, exported objects, cross-module imports, array identity, iteration, and element values.

Changes

Exported aggregate preservation

Layer / File(s) Summary
Track exported local bindings
crates/perry-transform/src/aggregate_scalar.rs, crates/perry-transform/src/aggregate_scalar_export_tests.rs, changelog.d/9054-preserve-exported-aggregates.md
The pass collects local IDs from module.exported_objects and named module.exports entries, then adds them to region_refs. Unit coverage verifies that the carrier and element aliases remain materialized.
Validate cross-module materialization
crates/perry/tests/issue_9053_exported_aggregate.rs
The integration test compiles a producer and consumer module, runs the compiled binary, and checks array length, identity, iteration, and object values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 6d17b

This change preserves exported aggregate values across modules without altering runtime APIs or deployment behavior, with regression coverage for length, iteration, property access, and identity. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses issue #9053 by preserving exported aggregate carriers for both export metadata paths and adding runtime coverage for length, iteration, property access, and identity. The provided con… Add or identify a test for an equivalent non-exported local aggregate that confirms scalar replacement still occurs when the carrier does not escape. Cite that test in the PR description or linked-issue assessment.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The implementation, transform tests, integration regression test, and changelog fragment are directly related to preserving exported aggregate carriers and resolving issue #9053. No unrelated code cha…
Title check ✅ Passed The title clearly and concisely describes the main change: preserving exported aggregate carriers during transformation.
Description check ✅ Passed The description includes all required sections, explains the fix, links issue #9053, lists concrete changes, documents test results and tests not run, and confirms checklist items.
Full details: Linked Issues check

Explanation

The PR addresses issue #9053 by preserving exported aggregate carriers for both export metadata paths and adding runtime coverage for length, iteration, property access, and identity. The provided context does not show regression coverage that confirms equivalent non-exported aggregates still scalarize.

Full details: Out of Scope Changes check

Explanation

The implementation, transform tests, integration regression test, and changelog fragment are directly related to preserving exported aggregate carriers and resolving issue #9053. No unrelated code changes are identified.

Full details: Docstring Coverage

Explanation

Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@proggeramlug
proggeramlug merged commit 10925b2 into PerryTS:main Aug 29, 2026
16 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aggregate_scalar deletes exported arrays and leaves undefined getters

1 participant