Skip to content

fix(compile): imported default ctor arity matches the synthesized forwarding ctor (#10258) - #10259

Closed
proggeramlug wants to merge 1 commit into
PerryTS:mainfrom
proggeramlug:fix/10258-imported-default-ctor-arity
Closed

proggeramlug wants to merge 1 commit into
PerryTS:mainfrom
proggeramlug:fix/10258-imported-default-ctor-arity

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #10258 (OpenCode v1.18.30 runtime wall, tracker #10107).

Root cause

For an exported class with no own constructor whose extends clause is a runtime value (class SystemError extends Data.Error {} — effect v4's Data.Error is an IIFE-returned class re-exported as a const), the two modules disagreed about the standalone constructor's arity:

  • the defining module synthesizes <prefix>__<Class>_constructor(this, __forward_arg0..7): ctor_arity::synthesized_ctor_param_count cannot resolve the parent's arity by name, so it forwards the fixed UNRESOLVED_PARENT_FWD_ARITY = 8 band;
  • the importing module built ImportedClass.constructor_param_count from the class's own constructor only (run_pipeline.rs imported_class_from_hir) = 0, declared the symbol as (double this), and marshal_imported_ctor_args truncated the call to zero arguments.

The callee then read its forwarding parameters from registers the caller never set: the parent ran without the argument object (with an unfixed compiler the repro's three-argument case even prints [7,1.385e-309,0]). Same-module construction and explicit constructor(a) { super(a) } were unaffected, because they do not go through the imported symbol with a mismatched arity.

In OpenCode this made effect's PlatformError.SystemError lose _tag / module / method, so Effect.catchReason("PlatformError", "NotFound", …) in FSUtil.readFileStringSafe never matched and the first missing optional config file (~/.config/opencode/config.json) became a fatal Error: Unexpected error / [object Object] for every command.

Fix

  • perry_codegen::context_free_ctor_param_count(class) (in codegen/ctor_arity.rs, next to synthesized_ctor_param_count and documented as having to agree with it) answers the arity whenever it is decidable from the class definition alone: own constructor → its params, native parent → 0, no heritage → 0, runtime-value heritage only (extends_expr, no extends_name) → UNRESOLVED_PARENT_FWD_ARITY. It returns None when the ancestor walk is needed.
  • The importer uses it for constructor_param_count and keeps the previous value for None, so every other shape is unchanged.

Verification (perrymaster, Linux x86_64, release build on main 4945fc1 + #10250)

https://claude.ai/code/session_01As1fetJAqDFib4n7Wm5Suo

Summary by CodeRabbit

  • Bug Fixes

    • Fixed imported classes without their own constructors so arguments are correctly forwarded to runtime parent classes.
    • Restored expected error details when constructing classes across modules, preventing certain missing-file configuration errors from becoming fatal.
  • Tests

    • Added regression coverage for exported and re-exported classes, including inherited constructors, accessors, methods, and multi-argument parent constructors.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d325ffaa-71ed-49bc-ade6-97d4ec736726

📥 Commits

Reviewing files that changed from the base of the PR and between 4945fc1 and a65aceb.

📒 Files selected for processing (7)
  • changelog.d/10258-imported-default-ctor-arity.md
  • crates/perry-codegen/src/codegen/ctor_arity.rs
  • crates/perry-codegen/src/codegen/mod.rs
  • crates/perry-codegen/src/lib.rs
  • crates/perry/src/commands/compile/run_pipeline.rs
  • crates/perry/tests/source_graph_export_regressions.rs
  • crates/perry/tests/source_graph_export_regressions/issue_10258.rs

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


📝 Walkthrough

Walkthrough

The compiler now derives imported class constructor arity from the shared synthesis rules. Classes with runtime-value parents use the forwarding arity, so cross-module construction passes arguments to the parent. A regression test covers exports, re-exports, inheritance, accessors, methods, and multi-argument constructors.

Changes

Imported constructor arity

Layer / File(s) Summary
Context-free constructor arity API
crates/perry-codegen/src/codegen/ctor_arity.rs, crates/perry-codegen/src/codegen/mod.rs, crates/perry-codegen/src/lib.rs
Adds context_free_ctor_param_count, hoists UNRESOLVED_PARENT_FWD_ARITY, and re-exports the helper through the codegen API.
Imported class integration and regression coverage
crates/perry/src/commands/compile/run_pipeline.rs, crates/perry/tests/source_graph_export_regressions.rs, crates/perry/tests/source_graph_export_regressions/issue_10258.rs, changelog.d/10258-imported-default-ctor-arity.md
Imported class metadata uses the helper and falls back when arity depends on ancestor resolution. The regression test verifies argument forwarding across the listed export and inheritance cases. The changelog records the fix.

Priority: ⬆️ High

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

Change: Bug fix · Severity of issue fixed: High

Sequence Diagram(s)

sequenceDiagram
  participant imported_class_from_hir
  participant context_free_ctor_param_count
  participant synthesized_ctor_param_count
  imported_class_from_hir->>context_free_ctor_param_count: derive constructor_param_count
  context_free_ctor_param_count-->>imported_class_from_hir: return fixed arity or None
  imported_class_from_hir->>synthesized_ctor_param_count: use matching forwarding arity
  synthesized_ctor_param_count-->>imported_class_from_hir: forward constructor arguments
Loading

Merge Risk: ⚪ Minimal · up to a65ac

The constructor-forwarding fix is covered across imported runtime-parent classes and related inheritance forms, with no remaining actionable merge risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: correcting imported constructor arity to match the synthesized forwarding constructor.
Description check ✅ Passed The description is detailed and relevant. It explains the issue, root cause, fix, linked issue, regression coverage, and verification results. It does not reproduce the template headings or checklist,…
Linked Issues check ✅ Passed The implementation satisfies #10258. context_free_ctor_param_count returns the fixed unresolved-parent forwarding arity for runtime-only heritage and returns None for ancestor-dependent shapes. Th…
Out of Scope Changes check ✅ Passed The changes stay within #10258. The helper, its re-exports, importer integration, changelog entry, and regression test all support constructor-arity handling or its verification. No unrelated behavior…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. (1 skipped: 1 …
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 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

Copy link
Copy Markdown
Contributor Author

Scope note after further testing: this PR fixes only the heritage form that lowers to extends_expr alone (class X extends (Data.Error as any) {}). A Member or Ident heritage (class SystemError extends Data.Error {}, class X extends NamedErr {} — what effect v4 actually uses) sets extends_name, so context_free_ctor_param_count returns None and the importer still declares 0 params while the defining module still emits the 8-slot forwarding band. Verified on a build with this PR: the cast form is fixed, the other three forms still lose their arguments. A follow-up that makes the two arity sources agree for every heritage form is in progress on fix/10258-ctor-arity-all-heritage and will supersede this PR; holding this one open only as the reduced case and test base.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed via merge train 189: #10266 (v0.5.1567). Verified merged main is byte-identical to the validated train. The broader constructor follow-up continues in #10265; issue #10258 remains open until that follow-up lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant