From 251d38dadb6a8ad5025b11a1d74d03a8afd20996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 22 Aug 2026 06:47:21 +0200 Subject: [PATCH] fix(codegen): keep imported class shapes canonical --- crates/perry-codegen/src/codegen/mod.rs | 29 ++++-- .../src/lower_call/typed_shape_bake_tests.rs | 96 ++++++++++++++++++- .../src/lower_call/typed_shape_init.rs | 17 ++++ 3 files changed, 134 insertions(+), 8 deletions(-) diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 9574fe8a50..219be2401a 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1855,6 +1855,10 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> // a class only gets a site entry if module init will actually compose its // image. A site entry with no init store would hand every instance a // zeroed header, so that direction of the dependency is load-bearing. + let imported_stub_names: std::collections::HashSet<&str> = imported_class_stubs + .iter() + .map(|class| class.name.as_str()) + .collect(); let class_header_image_inits: std::collections::HashMap = { let mut inits: std::collections::HashMap = std::collections::HashMap::new(); @@ -1865,13 +1869,24 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> let Some(&class_id) = class_ids.get(class_name) else { continue; }; - let typed_layout = crate::lower_call::typed_shape_init::layout_at_allocation_in( - &class_table, - &class_keys_globals_map, - &class_init_chains_map, - class_name, - field_count, - ); + // An imported stub has no defining constructor body, so this + // module cannot prove that its layout is declarable before that + // constructor runs. More importantly, minting a typed ShapeId + // here while the producer minted an ordinary one gives the same + // runtime class two exact identities across modules. Keep the + // consumer on the canonical structural identity and validate the + // typed layout after the producer's constructor returns. + let typed_layout = if imported_stub_names.contains(class_name.as_str()) { + crate::target_layout::InlineTypedLayout::None + } else { + crate::lower_call::typed_shape_init::layout_at_allocation_in( + &class_table, + &class_keys_globals_map, + &class_init_chains_map, + class_name, + field_count, + ) + }; let gc_packed = crate::target_layout::inline_alloc_gc_packed(&triple, field_count, typed_layout); match inits.get(keys_global) { diff --git a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs index 7e9f4d680f..ac8442b926 100644 --- a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs +++ b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs @@ -35,7 +35,7 @@ //! generic pointer-mask branch, which mints a per-object mask and flips the //! state to `SIDE_MASK`. That branch needs no descriptor at all. -use crate::{compile_module, AppMetadata, CompileOptions}; +use crate::{compile_module, AppMetadata, CompileOptions, ImportedClass}; use perry_hir::types::Type; use perry_hir::{ BinaryOp, Class, ClassField, CompareOp, Expr, Function, Module, ModuleInitKind, Param, Stmt, @@ -463,3 +463,97 @@ fn an_undefined_constructor_completion_takes_the_inline_arm() { returning a primitive would not throw:\n{ir}" ); } + +fn imported_remote() -> ImportedClass { + ImportedClass { + name: "Remote".to_string(), + local_alias: None, + source_prefix: "producer_ts".to_string(), + constructor_param_count: 1, + has_own_constructor: true, + constructor_has_rest: false, + has_instance_fields: true, + method_names: vec!["read".to_string()], + method_param_counts: vec![0], + method_has_rest: vec![false], + method_has_synthetic_arguments: vec![false], + static_field_names: Vec::new(), + static_method_names: Vec::new(), + getter_names: Vec::new(), + setter_names: Vec::new(), + parent_name: None, + field_names: vec!["child".to_string()], + field_types: vec![Type::Union(vec![ + Type::Named("Remote".to_string()), + Type::Null, + ])], + source_class_id: Some(55), + return_shape_imports: Vec::new(), + } +} + +/// Import metadata may retain a name that a local class shadows. Such a class +/// still has its own constructor proof and must not be mistaken for the +/// body-less imported stub when choosing the at-allocation layout. +#[test] +fn a_local_class_shadowing_an_import_keeps_its_layout_proof() { + let module = loop_new_module( + "Remote", + Type::Union(vec![Type::Named("Remote".to_string()), Type::Null]), + Expr::Null, + ); + let mut opts = ir_opts(); + opts.imported_classes.push(imported_remote()); + + let ir = + String::from_utf8(compile_module(&module, opts).unwrap()).expect("LLVM IR should be UTF-8"); + assert!( + ir.contains(TYPED_SHAPE_MINT_CALL) && !ir.contains(DECLARE_CALL), + "the local constructor proof was suppressed by a shadowed import:\n{ir}" + ); +} + +/// A consumer knows an imported class's declared field types, but its HIR stub +/// deliberately has no constructor body. That absence is not proof that the +/// fields may be declared before the real cross-module constructor runs. +/// +/// More subtly, letting the consumer infer the declaration mints a dedicated +/// typed ShapeId here while the defining module may have minted the ordinary +/// structural id. Exact method guards compiled in the producer then reject +/// every instance allocated in this module despite the class id and keys being +/// identical. +#[test] +fn imported_pointer_layout_does_not_invent_a_consumer_typed_shape_id() { + let mut module = Module::new("imported_shape_consumer.ts"); + module.init = vec![Stmt::Let { + id: 20, + name: "instance".to_string(), + ty: Type::Named("Remote".to_string()), + mutable: false, + init: Some(Expr::New { + class_name: "Remote".to_string(), + args: vec![Expr::Null], + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + }), + }]; + + let mut opts = ir_opts(); + opts.imported_classes.push(imported_remote()); + + let ir = + String::from_utf8(compile_module(&module, opts).unwrap()).expect("LLVM IR should be UTF-8"); + assert!( + ir.contains("call i32 @js_object_shape_id_for_keys("), + "the consumer must share the producer's canonical structural ShapeId:\n{ir}" + ); + assert!( + !ir.contains(TYPED_SHAPE_MINT_CALL), + "an imported stub invented a consumer-local typed ShapeId:\n{ir}" + ); + assert!( + !ir.contains(DECLARE_CALL) && ir.contains("call void @js_gc_init_typed_shape_layout("), + "the imported layout must be validated after its real constructor, not declared before it:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/lower_call/typed_shape_init.rs b/crates/perry-codegen/src/lower_call/typed_shape_init.rs index b4cd99aa65..5f89e7c172 100644 --- a/crates/perry-codegen/src/lower_call/typed_shape_init.rs +++ b/crates/perry-codegen/src/lower_call/typed_shape_init.rs @@ -35,6 +35,23 @@ use crate::types::{I32, I64, PTR}; /// [`crate::typed_shape::class_layout_declarable_at_allocation`], which /// documents what they are and why they are enough. pub(super) fn layout_declared_at_allocation(ctx: &FnCtx<'_>, class_name: &str) -> bool { + // A consumer module's imported Class stub carries field names/types but + // not the defining constructor body. Treating `constructor: None` as a + // proof that those slots may be declared before construction lets the + // consumer mint a typed ShapeId while the producer mints the ordinary + // structural ShapeId. Besides overstating the constructor proof, that + // splits one runtime class across two exact identities, so a direct method + // guard compiled in the producer can never accept an instance allocated + // by the consumer. Imported classes stay on the validate-after-ctor path + // until producer-authored layout proof is part of cross-module metadata. + if ctx.imported_class_ctors.contains_key(class_name) + && ctx + .classes + .get(class_name) + .is_some_and(|class| class.constructor.is_none()) + { + return false; + } layout_declared_at_allocation_in(ctx.classes, ctx.class_keys_globals, class_name) }