Skip to content

[codex] fix imported zero-arg super constructor dispatch #5171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
proggeramlug merged 1 commit into from
Jun 15, 2026
Merged

[codex] fix imported zero-arg super constructor dispatch #5171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions crates/perry-codegen/src/codegen/artifacts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,25 +351,30 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
// through to the next ancestor when `class_table`'s
// entry for an imported class returned a stub with
// `constructor: None` (stubs always have None) — even
// though the source module did have a real ctor with
// params. Result: `class Child extends Parent { x =
// though the source module did have a real ctor/effect.
// Result: `class Child extends Parent { x =
// "y" }` (no own ctor, parent in another module) had
// its synthesized ctor with ZERO params, so the user's
// `new Child("arg")` lost the arg before reaching
// Parent_constructor. Refs #420.
let imported_ctor_params = opts
// Parent_constructor. Explicit zero-arg ctors and
// field-initializer ctors still stop the walk even with
// zero adopted params. Refs #420.
let imported_ctor = opts
.imported_classes
.iter()
.find(|i| i.local_alias.as_deref().unwrap_or(&i.name) == pname.as_str())
.map(|ic| ic.constructor_param_count)
.unwrap_or(0);
.filter(|ic| {
ic.constructor_param_count > 0
|| ic.has_own_constructor
|| ic.has_instance_fields
});
if let Some(pclass) = class_table.get(pname.as_str()) {
if let Some(pctor) = &pclass.constructor {
found_params = pctor.params.clone();
break;
}
if imported_ctor_params > 0 {
for i in 0..imported_ctor_params {
if let Some(imported_ctor) = imported_ctor {
for i in 0..imported_ctor.constructor_param_count {
found_params.push(perry_hir::Param {
id: 0xFFFF_0000 + i as u32,
name: format!("__forward_arg{}", i),
Expand All @@ -385,10 +390,10 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
cur = pclass.extends_name.clone();
} else if let Some(stub) = imported_class_stubs.iter().find(|c| c.name == pname)
{
// Imported stub — params not in HIR; use its ctor
// param count as a synthetic count of unnamed args.
if imported_ctor_params > 0 {
for i in 0..imported_ctor_params {
// Imported stub — params not in HIR; use effectful
// ctor metadata as a synthetic count of unnamed args.
if let Some(imported_ctor) = imported_ctor {
for i in 0..imported_ctor.constructor_param_count {
found_params.push(perry_hir::Param {
id: 0xFFFF_0000 + i as u32,
name: format!("__forward_arg{}", i),
Expand Down
16 changes: 9 additions & 7 deletions crates/perry-codegen/src/codegen/method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,11 @@ pub(super) fn compile_method(
break;
};
let has_local_body = pc.constructor.is_some();
let has_imported_ctor = ctx.imported_class_ctors.contains_key(pname);
let has_imported_ctor = ctx
.imported_class_ctors
.get(pname)
.map(|ctor| ctor.stops_constructor_walk())
.unwrap_or(false);
if has_local_body || has_imported_ctor {
break;
}
Expand Down Expand Up @@ -376,10 +380,10 @@ pub(super) fn compile_method(
.map(|c| c.params.len())
.unwrap_or(0);
(sym, pcount)
} else if let Some((sym, n)) =
} else if let Some(ctor) =
ctx.imported_class_ctors.get(&pname_owned).cloned()
{
(sym, n)
(ctor.symbol, ctor.param_count)
} else {
// No callable ctor symbol — bail.
stmt::lower_stmts(&mut ctx, &method.body).with_context(|| {
Expand All @@ -397,10 +401,8 @@ pub(super) fn compile_method(
let _ = std::mem::take(&mut ctx.pending_declares);
return Ok(());
}
} else if let Some((sym, n)) =
ctx.imported_class_ctors.get(&pname_owned).cloned()
{
(sym, n)
} else if let Some(ctor) = ctx.imported_class_ctors.get(&pname_owned).cloned() {
(ctor.symbol, ctor.param_count)
} else {
("".to_string(), 0)
};
Expand Down
9 changes: 7 additions & 2 deletions crates/perry-codegen/src/codegen/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ mod string_pool;

pub use helpers::resolve_target_triple;
pub(crate) use helpers::{default_target_triple, write_barriers_enabled};
pub(crate) use opts::CrossModuleCtx;
pub use opts::{
AppMetadata, CompileOptions, FpContractMode, ImportedClass, NamespaceEntry, NamespaceEntryKind,
};
pub(crate) use opts::{CrossModuleCtx, ImportedCtor};

use artifacts::{emit_module_artifacts, ModuleArtifactsCtx};
use function::compile_function;
Expand Down Expand Up @@ -1063,7 +1063,12 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
let ctor_name = format!("{}__{}_constructor", ic.source_prefix, ic.name);
(
effective_name.to_string(),
(ctor_name, ic.constructor_param_count),
ImportedCtor {
symbol: ctor_name,
param_count: ic.constructor_param_count,
has_own_constructor: ic.has_own_constructor,
has_instance_fields: ic.has_instance_fields,
},
)
})
.collect(),
Expand Down
23 changes: 22 additions & 1 deletion crates/perry-codegen/src/codegen/opts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,10 @@ pub struct ImportedClass {
pub source_prefix: String,
/// Number of constructor parameters (needed for dispatch).
pub constructor_param_count: usize,
/// Whether the source class declared its own constructor body.
pub has_own_constructor: bool,
/// Whether the source class has instance fields that require initializer replay.
pub has_instance_fields: bool,
/// Method names defined on this class.
pub method_names: Vec<String>,
/// Per-method explicit param counts, parallel to `method_names`. Issue #235:
Expand Down Expand Up @@ -487,6 +491,23 @@ pub struct ImportedClass {
pub source_class_id: Option<u32>,
}

/// Constructor metadata for a class imported from another module.
#[derive(Debug, Clone)]
pub(crate) struct ImportedCtor {
pub symbol: String,
pub param_count: usize,
pub has_own_constructor: bool,
pub has_instance_fields: bool,
}

impl ImportedCtor {
/// True when constructor resolution must stop at this imported class even
/// when its standalone constructor takes zero user parameters.
pub(crate) fn stops_constructor_walk(&self) -> bool {
self.param_count > 0 || self.has_own_constructor || self.has_instance_fields
}
}

/// Cross-module import context, bundled into a single struct to avoid
/// adding five more individual parameters to every compile_* function.
/// Built once in `compile_module` from `CompileOptions`.
Expand Down Expand Up @@ -603,7 +624,7 @@ pub(crate) struct CrossModuleCtx {
/// Imported class constructor function names. Maps class_name →
/// full constructor symbol (e.g. "Editor" → "hone_editor_...__Editor_constructor").
/// Populated from `opts.imported_classes`.
pub imported_class_ctors: std::collections::HashMap<String, (String, usize)>,
pub imported_class_ctors: std::collections::HashMap<String, ImportedCtor>,
/// Compile-time i18n table for resolving `Expr::I18nString` against
/// the project's default locale. `None` when i18n is not configured.
/// Built from `opts.i18n_table` once at the top of `compile_module`
Expand Down
4 changes: 2 additions & 2 deletions crates/perry-codegen/src/expr/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ pub(crate) struct FnCtx<'a> {
/// `ctx.classes` chain (which mis-picks same-named cross-module parents).
pub class_init_chains:
&'a std::collections::HashMap<String, Vec<(String, Vec<perry_hir::ClassField>)>>,
/// Imported class constructor names: class_name → (ctor_fn_name, param_count).
pub imported_class_ctors: &'a std::collections::HashMap<String, (String, usize)>,
/// Imported class constructor metadata, keyed by effective imported class name.
pub imported_class_ctors: &'a std::collections::HashMap<String, crate::codegen::ImportedCtor>,
/// Per-function param signature: `(declared_param_count,
/// has_rest_param)`. Used by FuncRef call sites to know whether
/// to bundle trailing arguments into a rest array.
Expand Down
28 changes: 13 additions & 15 deletions crates/perry-codegen/src/expr/this_super_call.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,29 +625,27 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
// PgSerialBuilder → PgColumnBuilder → ColumnBuilder chain
// where only ColumnBuilder has a ctor body).
// Walk up the parent chain to find the first class with a
// local constructor body OR a cross-module ctor stub WITH
// declared params. JS spec requires `class Mid extends Base {}`
// local constructor body OR a cross-module ctor stub that must
// run. JS spec requires `class Mid extends Base {}`
// followed by `class Leaf extends Mid` calling `super(...)` to
// reach Base's ctor body (Mid has no ctor → implicit forward).
// Refs #420 (drizzle's PgSerialBuilder → PgColumnBuilder →
// ColumnBuilder where only ColumnBuilder has a body).
//
// We must skip past imported ctors with param_count=0 too —
// those represent empty-bodied derived classes whose imported
// standalone ctor would otherwise eat the incoming args
// without forwarding. Walking past them and dispatching
// directly to the ancestor-with-real-params standalone ctor
// preserves the args end-to-end.
// Imported empty-derived classes with no fields still get walked
// past so their synthesized standalone ctor does not eat forwarded
// args. Explicit zero-arg ctors and field-initializer ctors stop
// the walk because their body/initializers must run.
let mut effective_parent_name = parent_name.clone();
let mut effective_parent_class = parent_class;
loop {
let has_local_body = effective_parent_class.constructor.is_some();
let has_real_imported_ctor = ctx
let has_effectful_imported_ctor = ctx
.imported_class_ctors
.get(&effective_parent_name)
.map(|(_, n)| *n > 0)
.map(|ctor| ctor.stops_constructor_walk())
.unwrap_or(false);
if has_local_body || has_real_imported_ctor {
if has_local_body || has_effectful_imported_ctor {
break;
}
let Some(grandparent_name) = effective_parent_class
Expand Down Expand Up @@ -791,7 +789,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
],
);
}
} else if let Some((ctor_name, param_count)) = ctx
} else if let Some(ctor) = ctx
.imported_class_ctors
.get(&effective_parent_name)
.cloned()
Expand All @@ -807,7 +805,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
// silently drops `super(...)` for imported parents and the subclass
// ends up with only its own fields, breaking hono-base inheritance.
let undef_lit = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED));
while lowered_args.len() < param_count {
while lowered_args.len() < ctor.param_count {
lowered_args.push(undef_lit.clone());
}
let this_slot = ctx.this_stack.last().cloned();
Expand All @@ -826,11 +824,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
ctor_args.push((DOUBLE, la.as_str()));
}
ctx.pending_declares.push((
ctor_name.clone(),
ctor.symbol.clone(),
crate::types::VOID,
ctor_param_types,
));
ctx.block().call_void(&ctor_name, &ctor_args);
ctx.block().call_void(&ctor.symbol, &ctor_args);
}

// After the parent body has run (which may have set `this.config`
Expand Down
47 changes: 23 additions & 24 deletions crates/perry-codegen/src/lower_call/new.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,10 @@ fn effective_constructor_param_count(ctx: &FnCtx<'_>, class: &perry_hir::Class)
}
let mut parent = class.extends_name.as_deref();
while let Some(pname) = parent {
if let Some((_sym, n)) = ctx.imported_class_ctors.get(pname) {
return *n;
if let Some(ctor) = ctx.imported_class_ctors.get(pname) {
if ctor.stops_constructor_walk() {
return ctor.param_count;
}
}
match ctx.classes.get(pname).copied() {
Some(pc) => {
Expand Down Expand Up @@ -1352,21 +1354,20 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
// SuperCall Error-like arm in expr.rs.
//
// BUT: if `class_name` is an imported stub with a cross-module
// ctor that has REAL params, defer to that path — the source
// ctor with a real body/effect, defer to that path — the source
// module's ctor body knows the real param order
// (e.g. `constructor(public statusCode, msg)` where args[0] is
// statusCode, not message). Running Error-init here would
// assign the wrong arg to `message` and corrupt the instance.
// When the imported ctor's param_count is 0, the source had no
// own ctor (codegen synthesized an empty 0-param ctor for the
// bare-extends-Error case), so calling it is a no-op and we
// still need Error-init to populate `this.message` / `this.name`.
let imported_ctor_has_real_params = ctx
// When the imported ctor is a synthesized empty 0-param ctor for the
// bare-extends-Error case, calling it is a no-op and we still need
// Error-init to populate `this.message` / `this.name`.
let imported_ctor_has_body_or_fields = ctx
.imported_class_ctors
.get(class_name)
.map(|(_, n)| *n > 0)
.map(|ctor| ctor.stops_constructor_walk())
.unwrap_or(false);
if !found_inherited_ctor && !imported_ctor_has_real_params {
if !found_inherited_ctor && !imported_ctor_has_body_or_fields {
// Trace the chain to find the first Error-like ancestor name.
let mut error_kind: Option<String> = None;
let mut cur = class.extends_name.clone();
Expand Down Expand Up @@ -1488,12 +1489,12 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
let mut effective_class_name = lookup_class.clone();
let mut effective_extends = class.extends_name.clone();
loop {
let has_real_ctor = ctx
let has_effectful_ctor = ctx
.imported_class_ctors
.get(&effective_class_name)
.map(|(_, n)| *n > 0)
.map(|ctor| ctor.stops_constructor_walk())
.unwrap_or(false);
if has_real_ctor {
if has_effectful_ctor {
break;
}
// v0.5.759: stop walking ONLY for the leaf class (the user's
Expand Down Expand Up @@ -1527,16 +1528,16 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
effective_class_name = parent;
effective_extends = parent_class.extends_name.clone();
}
if let Some((ctor_name, param_count)) = ctx
if let Some(ctor) = ctx
.imported_class_ctors
.get(&effective_class_name)
.cloned()
.filter(|(_, _)| effective_class_name != lookup_class)
.filter(|_| effective_class_name != lookup_class)
{
// Walked to an ancestor — call its ctor with this and forwarded args.
let undef_lit =
crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED));
while lowered_args.len() < param_count {
while lowered_args.len() < ctor.param_count {
lowered_args.push(undef_lit.clone());
}
let mut ctor_args: Vec<(crate::types::LlvmType, &str)> =
Expand All @@ -1549,19 +1550,17 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
ctor_args.push((DOUBLE, la.as_str()));
}
ctx.pending_declares.push((
ctor_name.clone(),
ctor.symbol.clone(),
crate::types::VOID,
ctor_param_types,
));
ctx.block().call_void(&ctor_name, &ctor_args);
} else if let Some((ctor_name, param_count)) =
ctx.imported_class_ctors.get(class_name).cloned()
{
ctx.block().call_void(&ctor.symbol, &ctor_args);
} else if let Some(ctor) = ctx.imported_class_ctors.get(class_name).cloned() {
// Pad missing optional args with TAG_UNDEFINED so the constructor
// doesn't read garbage from stale registers.
let undef_lit =
crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED));
while lowered_args.len() < param_count {
while lowered_args.len() < ctor.param_count {
lowered_args.push(undef_lit.clone());
}
// Pass `this` as NaN-boxed double (same as compile_method's this_arg).
Expand All @@ -1575,11 +1574,11 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
ctor_args.push((DOUBLE, la.as_str()));
}
ctx.pending_declares.push((
ctor_name.clone(),
ctor.symbol.clone(),
crate::types::VOID,
ctor_param_types,
));
ctx.block().call_void(&ctor_name, &ctor_args);
ctx.block().call_void(&ctor.symbol, &ctor_args);
}
} // end !found_inherited_ctor
}
Expand Down
Loading