diff --git a/changelog.d/11014-effect-tagged-error-name.md b/changelog.d/11014-effect-tagged-error-name.md new file mode 100644 index 0000000000..7ead2aec7c --- /dev/null +++ b/changelog.d/11014-effect-tagged-error-name.md @@ -0,0 +1,12 @@ +Fixed inherited `name` on Effect tagged errors (#10890). + +A factory-created `Base.prototype.name` could be lost when a class expression +or function-local class declaration used a shared template parent instead of +its evaluated parent. Nested `super()` replay also replaced the instance's +class pin with a deeper Error ancestor, so property reads found `Error` before +the tag. Perry now retains the evaluated heritage and first constructor pin, +then reads inherited properties from that evaluation's prototype chain. + +The parity fixture covers distinct tags, `String(error)`, and Effect's nested +`Data.Error` inheritance shape. The pinned Effect package repro now matches +Node for `_tag`, `name`, `instanceof`, the declared field, and `String(error)`. diff --git a/crates/perry-hir/src/lower/lower_expr/arm_class.rs b/crates/perry-hir/src/lower/lower_expr/arm_class.rs index 4bd9822d1d..318b9dfb6b 100644 --- a/crates/perry-hir/src/lower/lower_expr/arm_class.rs +++ b/crates/perry-hir/src/lower/lower_expr/arm_class.rs @@ -226,6 +226,7 @@ pub(crate) fn lower_class_expr( || computed_statics.iter().any(|(_, value)| uses_self(value)) || computed_name_evaluations.iter().any(uses_self) }); + let has_static_methods = !class.static_methods.is_empty(); ctx.pending_classes.push(class); // #1772/#5893: a class EXPRESSION that carries per-evaluation static // fields, captures, or private elements lowers to a @@ -301,7 +302,15 @@ pub(crate) fn lower_class_expr( || !captured_args.is_empty() || !static_block_names.is_empty() || has_private_elements - || self_binding_used) + || self_binding_used + // A factory-created superclass is a fresh class object with its + // own mutable prototype. A shared ClassRef for the child links to + // the template prototype instead of that evaluated parent (e.g. + // Effect's Base.prototype.name = tag). Keep the runtime parent + // value on a fresh child class. The shared path remains for class + // expressions with static methods until those methods can be + // installed on fresh class objects. + || (parent_expr.is_some() && !has_static_methods)) { // #6438: a class expression WITH heritage (`class extends `) used // to be excluded here and fell back to the shared-template `ClassRef` diff --git a/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs b/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs index cf9f617a4b..89b4cbe636 100644 --- a/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs +++ b/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs @@ -200,6 +200,18 @@ pub(crate) fn lower_class_from_ast( Ok(expr) => (None, Some(parent_name), None, Some(Box::new(expr))), Err(_) => (None, Some(parent_name), None, None), } + } else if ctx.scope_depth > 0 && ctx.locals.lookup(ident.sym.as_ref()).is_some() { + // A function-local class declaration is a fresh class + // object each time its enclosing function runs. Preserve + // its static id for method/layout analysis, but also + // record the evaluated local as the actual superclass. + // Otherwise a fresh child class expression links to the + // shared template prototype and loses writes such as + // `Base.prototype.name = tag` (Effect TaggedError). + match lower_class_heritage_expr(ctx, super_class) { + Ok(expr) => (parent_cid, Some(parent_name), None, Some(Box::new(expr))), + Err(_) => (parent_cid, Some(parent_name), None, None), + } } else { (parent_cid, Some(parent_name), None, None) } diff --git a/crates/perry-runtime/src/object/class_registry/evaluation_heritage.rs b/crates/perry-runtime/src/object/class_registry/evaluation_heritage.rs index 303b3e5dac..1dc6718a87 100644 --- a/crates/perry-runtime/src/object/class_registry/evaluation_heritage.rs +++ b/crates/perry-runtime/src/object/class_registry/evaluation_heritage.rs @@ -205,6 +205,12 @@ pub(crate) fn pin_instance_constructing_class(inst: *mut ObjectHeader, classobj_ if class_ptr.is_null() || class_object_pinned_parent(class_ptr).is_none() { return; } + // A derived constructor may replay several fresh ancestors. Keep the + // first (most derived) class object: later super() legs would otherwise + // replace it with a deeper ancestor and lose the nearer prototype chain. + if instance_pinned_constructing_class(inst).is_some() { + return; + } // `js_class_object_pin_parent` already armed `CLASS_OBJECT_HERITAGE_PIN_LATCH` // before writing `class_ptr`'s own pin above (the ordering rule in // `registry_latch.rs`) — that write happens-before this one in this diff --git a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs index 235da8406e..c6d24e4cf1 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs @@ -525,6 +525,55 @@ unsafe fn inherited_proto_accessor_value( )) } +/// Read the actual prototype objects of a class whose parent is a fresh class +/// evaluation. The template-id walk below follows the parent's shared class +/// registry entry; that entry cannot see writes to this evaluation's +/// `Base.prototype` (such as an Effect tagged error's `name`). +unsafe fn evaluated_parent_instance_field( + decl_proto: *mut ObjectHeader, + key: *const crate::StringHeader, + receiver: f64, +) -> Option { + if decl_proto.is_null() || key.is_null() { + return None; + } + let mut link = Some(crate::value::js_nanbox_pointer(decl_proto as i64).to_bits()); + for _ in 0..32 { + let bits = link?; + if bits == crate::value::TAG_NULL { + return None; + } + let value = f64::from_bits(bits); + if crate::proxy::js_proxy_is_proxy(value) != 0 { + return super::super::prototype_chain::resolve_inherited_field_from_prototype( + decl_proto as usize, + bits, + key, + ); + } + let addr = match bits >> 48 { + 0x7FFD => (bits & crate::value::POINTER_MASK) as usize, + 0 if crate::value::addr_class::is_above_handle_band(bits as usize) => bits as usize, + _ => return None, + }; + let Some(header) = crate::value::addr_class::try_read_gc_header(addr) else { + return None; + }; + if header.obj_type != crate::gc::GC_TYPE_OBJECT { + return None; + } + let proto = addr as *mut ObjectHeader; + if let Some(value) = inherited_proto_accessor_value(proto, key, receiver) { + return Some(value); + } + if let Some(value) = super::super::field_get_set::own_data_field_by_name(proto, key) { + return Some(value); + } + link = super::super::prototype_chain::object_static_prototype(addr); + } + None +} + /// `constructor_side`: this walk serves a read on the class CONSTRUCTOR, so a /// name that is a declared INSTANCE member must not resolve through it. /// @@ -579,6 +628,43 @@ unsafe fn resolve_proto_chain_field_inner( receiver: Option, constructor_side: bool, ) -> Option { + if let Some(receiver) = receiver { + let receiver_value = JSValue::from_bits(receiver.to_bits()); + if receiver_value.is_pointer() { + let receiver_obj = receiver_value.as_pointer::(); + if !receiver_obj.is_null() { + if let Some(pin) = instance_pinned_constructing_class(receiver_obj) { + // A factory-created class can be evaluated again after + // this instance was built. Its template class id then + // points at the later evaluation. Resolve through this + // instance's pinned class object and its own prototype. + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(receiver); + let pin = scope.root_nanbox_f64(pin); + let key = scope.root_string_ptr(key as *mut crate::StringHeader); + let pin_obj = JSValue::from_bits(pin.get_nanbox_f64().to_bits()) + .as_pointer::(); + let proto_value = + super::super::field_get_set::class_object_prototype_value(pin_obj); + let proto = JSValue::from_bits(proto_value.bits()).as_pointer::(); + if !proto.is_null() { + let proto = scope.root_raw_mut_ptr(proto as *mut ObjectHeader); + if let Some(value) = proto.with_mut_ptr::(|proto| { + key.with_const_ptr::(|key| { + evaluated_parent_instance_field( + proto, + key, + receiver.get_nanbox_f64(), + ) + }) + }) { + return Some(value); + } + } + } + } + } + } // Resolved once: `class_instance_has_member` already walks the parent // chain, so a parent's instance method is excluded from a subclass's // constructor read too. diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index 0430c81ecd..edc34ef1ab 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -1118,30 +1118,50 @@ pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { // Object.prototype default. Some(crate::value::TAG_NULL) } else { - let registered_parent_proto = get_parent_class_id(class_id) - .filter(|parent_id| *parent_id != 0 && *parent_id != class_id) - .and_then(|parent_id| { - let parent_proto = class_decl_prototype_value(parent_id); - let parent_bits = parent_proto.to_bits(); - if (parent_bits >> 48) == 0x7FFD { - return Some(parent_bits); - } - // #10599: `parent_id` may be a RESERVED native-builtin class id - // rather than a declared class -- `builtin_parent_reserved_class_id` - // in perry-codegen wires this edge for `class Sub extends - // EventEmitter {}`, which has no `js_register_class_name` - // registration of its own. `class_decl_prototype_value` bails - // immediately for such an id (`class_name_for_id` is `None`), so - // without this fallback the lookup above always misses and - // execution falls through to the runtime-function-valued branch - // below, which also misses (there is no dynamic-parent VALUE for - // a statically-resolved reserved id) -- landing `Sub.prototype`'s - // `[[Prototype]]` on `Object.prototype` instead of - // `EventEmitter.prototype`. - reserved_native_parent_prototype_bits(parent_id) - }); - if registered_parent_proto.is_some() { - registered_parent_proto + // A dynamically evaluated class has its own prototype object even + // when it shares a template class id with other evaluations. Use the + // parent VALUE recorded at this class definition, before consulting + // the template's parent-id edge. The latter loses assignments such as + // Effect's `Base.prototype.name = tag` on the actual parent object. + let evaluated_parent_proto = { + let parent_value = dynamic_parent.get_nanbox_f64(); + if super::is_class_object_value(parent_value) { + let parent_obj = crate::value::JSValue::from_bits(parent_value.to_bits()) + .as_pointer::(); + let parent_proto = unsafe { + super::super::field_get_set::class_object_prototype_value(parent_obj) + }; + class_parent_prototype_bits(f64::from_bits(parent_proto.bits())) + } else { + None + } + }; + let parent_proto = evaluated_parent_proto.or_else(|| { + get_parent_class_id(class_id) + .filter(|parent_id| *parent_id != 0 && *parent_id != class_id) + .and_then(|parent_id| { + let parent_proto = class_decl_prototype_value(parent_id); + let parent_bits = parent_proto.to_bits(); + if (parent_bits >> 48) == 0x7FFD { + return Some(parent_bits); + } + // #10599: `parent_id` may be a RESERVED native-builtin class id + // rather than a declared class -- `builtin_parent_reserved_class_id` + // in perry-codegen wires this edge for `class Sub extends + // EventEmitter {}`, which has no `js_register_class_name` + // registration of its own. `class_decl_prototype_value` bails + // immediately for such an id (`class_name_for_id` is `None`), so + // without this fallback the lookup above always misses and + // execution falls through to the runtime-function-valued branch + // below, which also misses (there is no dynamic-parent VALUE for + // a statically-resolved reserved id) -- landing `Sub.prototype`'s + // `[[Prototype]]` on `Object.prototype` instead of + // `EventEmitter.prototype`. + reserved_native_parent_prototype_bits(parent_id) + }) + }); + if parent_proto.is_some() { + parent_proto } else { // A runtime function-valued superclass (including Intl service // constructors) has no class-id edge. Link the declared prototype @@ -1177,9 +1197,13 @@ pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { } }; if let Some(bits) = parent_proto_bits { + let bits = scope.root_heap_word_u64(bits); let proto = class_decl_prototype_object(class_id); if !proto.is_null() { - super::super::prototype_chain::object_set_static_prototype(proto as usize, bits); + super::super::prototype_chain::object_set_static_prototype( + proto as usize, + bits.get_heap_word_u64(), + ); } } diff --git a/test-files/test_issue_10890_tagged_error_name.ts b/test-files/test_issue_10890_tagged_error_name.ts new file mode 100644 index 0000000000..65d48cf5e3 --- /dev/null +++ b/test-files/test_issue_10890_tagged_error_name.ts @@ -0,0 +1,98 @@ +// Dynamic Error subclasses, like Effect's Schema.TaggedError, set the +// inherited name on a factory-created base prototype. +class Plain extends Error {} +Plain.prototype.name = "PlainTag"; +console.log("plain", new Plain("message").name); + +function makeBase(tag: string) { + class Base extends Error {} + Base.prototype.name = tag; + return class Tagged extends Base {}; +} +const Factory = makeBase("FactoryTag"); +console.log("factory", new Factory("message").name); + +function makeObjectBase(tag: string) { + const O = { Base: class extends Error {} }; + O.Base.prototype.name = tag; + return class Tagged extends O.Base {}; +} +const ObjectBase = makeObjectBase("ObjectTag"); +console.log("object-base", new ObjectBase("message").name); + +function makeDeepBase(tag: string) { + class Base extends Error {} + Base.prototype.name = tag; + const makeClass = (Ctor: typeof Base) => class Mid extends Ctor {}; + return class Tagged extends makeClass(Base) {}; +} +const DeepBase = makeDeepBase("DeepTag"); +console.log("deep-base", new DeepBase("message").name); + +function makeTagged(tag: string) { + class Base extends Error {} + Base.prototype.name = tag; + return class Tagged extends Base { + static _tag = tag; + }; +} +class First extends makeTagged("FirstTag") {} +class Second extends makeTagged("SecondTag") {} +const first = new First("message"); +const second = new Second("message"); +console.log("prototypes", First.prototype.name, Second.prototype.name); +console.log("two-tags", first.name, second.name, first instanceof Second); +console.log("to-string", String(first), String(second)); +console.log("own-name", Object.prototype.hasOwnProperty.call(first, "name")); + +// Effect's Data.Error adds factory-created ancestors beyond the tagged Base. +// Constructor replay must retain the nearest class evaluation: pinning the +// deepest ancestor makes this instance read Error.prototype.name instead. +const YieldableError = (function () { + class YieldableError extends Error { + toJSON() { + return { ...this }; + } + } + return YieldableError; +})(); +const DataError = (function () { + const classes = { + BaseEffectError: class extends YieldableError { + constructor(args: any) { + super(args?.message); + if (args) Object.assign(this, args); + } + }, + }; + return classes.BaseEffectError; +})(); +function makeSchemaClass(Base: any) { + const klass = class extends Base { + constructor(props: any = {}) { + super(props); + } + static get ast() { + return "ast"; + } + }; + return klass; +} +function makeEffectTagged(tag: string) { + class Base extends DataError {} + Base.prototype.name = tag; + class TaggedErrorClass extends makeSchemaClass(Base) { + static _tag = tag; + } + return TaggedErrorClass; +} +class NestedFirst extends makeEffectTagged("NestedFirstTag") {} +class NestedSecond extends makeEffectTagged("NestedSecondTag") {} +const nestedFirst = new NestedFirst({ message: "message" }); +const nestedSecond = new NestedSecond({ message: "message" }); +console.log( + "nested-tags", + nestedFirst.name, + nestedSecond.name, + nestedFirst instanceof NestedSecond, +);