Skip to content

fix(runtime): inherit Error subclass names (#9440) #9511

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

Closed
proggeramlug wants to merge 1 commit into from
Closed

fix(runtime): inherit Error subclass names (#9440) #9511

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
9 changes: 9 additions & 0 deletions changelog.d/9511-error-name-ownership.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
### Fixed

- **Error subclasses no longer expose their default `name` as an own,
enumerable property.** `name` now remains on the appropriate Error-family
prototype until user code explicitly assigns it, matching Node across
`JSON.stringify`, `Object.getOwnPropertyNames`, `Object.keys`, `for…in`,
object spread, property descriptors, and `util.inspect`. Error construction
also preserves Node's observable own-key order: `stack` precedes an optional
`message`.
9 changes: 1 addition & 8 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 @@ -978,17 +978,10 @@ pub(super) fn compile_method(
.cloned()
.map(|slot| ctx.block().load(DOUBLE, &slot))
.unwrap_or_else(|| undef_lit.clone());
let kind_idx = ctx.strings.intern(&pname_owned);
let kind_handle_global =
format!("@{}", ctx.strings.entry(kind_idx).handle_global);
let blk = ctx.block();
let kind_box = blk.load(DOUBLE, &kind_handle_global);
let kind_bits = blk.bitcast_double_to_i64(&kind_box);
let kind_raw =
blk.and(I64, &kind_bits, crate::nanbox::POINTER_MASK_I64);
blk.call_void(
"js_error_subclass_default_init",
&[(DOUBLE, &this_box), (DOUBLE, &msg_box), (I64, &kind_raw)],
&[(DOUBLE, &this_box), (DOUBLE, &msg_box)],
);
}
("".to_string(), 0)
Expand Down
103 changes: 36 additions & 67 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 @@ -601,8 +601,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
// family) — HIR captures `extends_expr` for any unknown Ident,
// INCLUDING the built-ins, so we'd otherwise eat the more-correct
// Error-init path below. The built-in arms handle their own
// semantics (Error sets this.message + this.name; streams allocate
// a registry handle). Anything else with an extends_expr is a
// semantics (Error installs own message/stack slots; streams
// allocate a registry handle). Anything else with an extends_expr is a
// real runtime-value parent and routes through this dispatch.
// The classic node:stream / Web-Streams names are only the
// genuine built-in parents when HIR did NOT capture an
Expand Down Expand Up @@ -1169,8 +1169,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
}
// Built-in parent (Error, TypeError, RangeError, etc.)
// — user classes extending them need `super(message)` to
// assign `this.message = args[0]` and `this.name = parent_name`
// so downstream `err.message` / `err.name` access works.
// install the own non-enumerable `message`/`stack` slots;
// `name` resolves from the Error-family prototype.
// `instanceof Error` walking the extends chain is handled
// elsewhere; this just makes `err.message` non-undefined.
if matches!(
Expand Down Expand Up @@ -1274,6 +1274,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
let this_slot = ctx.this_stack.last().cloned();
if let Some(this_slot) = this_slot {
let blk = ctx.block();
// #9410/#9440: capture the own non-enumerable
// `stack` before installing `message`, matching
// V8's observable own-key order. Its lazy getter
// still reads `name`/`message` after `super()`.
let this_for_stack = blk.load(DOUBLE, &this_slot);
blk.call_void(
"js_error_subclass_capture_stack",
&[(DOUBLE, &this_for_stack)],
);
// Capture can collect, so derive the raw receiver
// from a fresh load for the remaining stores.
let this_box = blk.load(DOUBLE, &this_slot);
let this_bits = blk.bitcast_double_to_i64(&this_box);
let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64);
Expand All @@ -1295,58 +1306,27 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
&[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)],
);
}
// this.name = <parent_name> as default (can be
// overridden by the subclass constructor body).
let name_idx = ctx.strings.intern("name");
let name_handle_global =
format!("@{}", ctx.strings.entry(name_idx).handle_global);
let name_val_idx = ctx.strings.intern(&parent_name);
let name_val_global =
format!("@{}", ctx.strings.entry(name_val_idx).handle_global);
let blk = ctx.block();
let name_key_box = blk.load(DOUBLE, &name_handle_global);
let name_key_bits = blk.bitcast_double_to_i64(&name_key_box);
let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64);
let name_val_box = blk.load(DOUBLE, &name_val_global);
blk.call_void(
"js_object_set_field_by_name",
&[
(I64, &this_handle),
(I64, &name_key_raw),
(DOUBLE, &name_val_box),
],
);
// `name` is inherited from the Error-family
// prototype. Do not stamp it here: untouched Error
// subclasses must have no own `name`; a later
// `this.name = ...` remains an ordinary enumerable
// own assignment (#9440).
// #5127: `super(message, options)` must forward the
// ES2022 `cause` option. The instance is a generic
// object, so install a non-enumerable `cause`
// property from args[1] when present.
if let Some(opts_val) = lowered_args.get(1) {
let blk = ctx.block();
// The message store above can collect. Reload
// the rooted receiver before applying `cause`.
let this_box = blk.load(DOUBLE, &this_slot);
let this_bits = blk.bitcast_double_to_i64(&this_box);
let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64);
blk.call_void(
"js_error_apply_cause_to_object",
&[(I64, &this_handle), (DOUBLE, opts_val)],
);
}
// #9410: `stack`. `super(message)` into a built-in
// Error stamps `message`/`name`/`cause` onto the
// already-allocated plain instance and stops there,
// so `new (class extends Error {})("x").stack` was
// `undefined` while `new Error("x").stack` is a
// string. The frame is captured HERE, at the
// construction site; the `name: message` head is
// formatted on read, because a subclass
// constructor assigns `this.name` after `super()`
// returns and Node reports the assigned name.
let blk = ctx.block();
// Reload `this` from its slot: the stamps above
// can collect, and a DOUBLE held across a
// collecting call is the bare-pointer hazard
// #8770 is about.
let this_for_stack = blk.load(DOUBLE, &this_slot);
blk.call_void(
"js_error_subclass_capture_stack",
&[(DOUBLE, &this_for_stack)],
);
}
}
bind_derived_this_after_super(ctx);
Expand Down Expand Up @@ -1591,11 +1571,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
}

restore_inline_constructor_scope(ctx, saved_scope);
} else if let Some(error_kind) = {
} else if let Some(_error_kind) = {
// Issue #573: walk the chain from `effective_parent_class`
// upward; if it terminates at an Error-like built-in,
// emit the same Error init the no-parent-class branch
// does (sets this.message + this.name). Without this,
// emit the same Error init the no-parent-class branch does
// (own non-enumerable `stack`/`message`; inherited `name`). Without this,
// `class C extends Error {}; class D extends C { ctor(m){
// super(m); } }` reaches here with `effective_parent_class
// = C` (no own ctor) and a parent of "Error" (not in
Expand Down Expand Up @@ -1633,6 +1613,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
let this_slot = ctx.this_stack.last().cloned();
if let Some(this_slot) = this_slot {
let blk = ctx.block();
// The indirect all-implicit chain is still an Error
// construction site. Capture `stack` first for the same
// own-key order as the direct built-in arm above.
let this_for_stack = blk.load(DOUBLE, &this_slot);
blk.call_void(
"js_error_subclass_capture_stack",
&[(DOUBLE, &this_for_stack)],
);
let this_box = blk.load(DOUBLE, &this_slot);
let this_bits = blk.bitcast_double_to_i64(&this_box);
let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64);
Expand All @@ -1650,25 +1638,6 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
&[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)],
);
}
let name_idx = ctx.strings.intern("name");
let name_handle_global =
format!("@{}", ctx.strings.entry(name_idx).handle_global);
let name_val_idx = ctx.strings.intern(&error_kind);
let name_val_global =
format!("@{}", ctx.strings.entry(name_val_idx).handle_global);
let blk = ctx.block();
let name_key_box = blk.load(DOUBLE, &name_handle_global);
let name_key_bits = blk.bitcast_double_to_i64(&name_key_box);
let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64);
let name_val_box = blk.load(DOUBLE, &name_val_global);
blk.call_void(
"js_object_set_field_by_name",
&[
(I64, &this_handle),
(I64, &name_key_raw),
(DOUBLE, &name_val_box),
],
);
}
} else if let Some(ctor) = ctx
.imported_class_ctors
Expand Down
52 changes: 18 additions & 34 deletions crates/perry-codegen/src/lower_call/new_error_init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,15 @@ use crate::expr::FnCtx;
use crate::nanbox::POINTER_MASK_I64;
use crate::types::{DOUBLE, I64};

/// Stamp `message`, `name` and `stack` onto the freshly allocated instance of
/// an Error-family subclass, mirroring the `SuperCall` Error-like arm in
/// Stamp `stack` and (when supplied) `message` onto the freshly allocated
/// instance of an Error-family subclass, mirroring the `SuperCall` Error-like arm in
/// `expr/this_super_call.rs`.
///
/// `name` deliberately stays on the terminating Error-family prototype. A
/// plain assignment such as `error.name = "Custom"` then creates the ordinary
/// enumerable own property required by `[[Set]]`, while an untouched instance
/// keeps `name` out of every own-key consumer (#9440).
///
/// Returns `true` when the class's `extends` chain does terminate at an Error
/// family base and the init was emitted — the caller then skips its
/// imported-ctor fallback. Returns `false` (emitting nothing) otherwise.
Expand Down Expand Up @@ -52,9 +57,19 @@ pub(super) fn emit_default_error_init(
break;
}
}
if let Some(kind) = error_kind {
if error_kind.is_some() {
let this_slot_for_err = ctx.this_stack.last().cloned().unwrap_or_default();
let blk = ctx.block();
let this_for_stack = blk.load(DOUBLE, &this_slot_for_err);
// V8 creates `stack` before `message`; preserve that observable own-key
// order (`["stack", "message"]`) while the stack head itself remains
// lazy and therefore sees the later message/name values.
blk.call_void(
"js_error_subclass_capture_stack",
&[(DOUBLE, &this_for_stack)],
);
// The capture allocates, so reload the rooted receiver before deriving
// the raw pointer consumed by the message store.
let this_box = blk.load(DOUBLE, &this_slot_for_err);
let this_bits = blk.bitcast_double_to_i64(&this_box);
let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64);
Expand All @@ -72,37 +87,6 @@ pub(super) fn emit_default_error_init(
&[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)],
);
}
let name_idx = ctx.strings.intern("name");
let name_handle_global = format!("@{}", ctx.strings.entry(name_idx).handle_global);
let name_val_idx = ctx.strings.intern(&kind);
let name_val_global = format!("@{}", ctx.strings.entry(name_val_idx).handle_global);
let blk = ctx.block();
let name_key_box = blk.load(DOUBLE, &name_handle_global);
let name_key_bits = blk.bitcast_double_to_i64(&name_key_box);
let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64);
let name_val_box = blk.load(DOUBLE, &name_val_global);
blk.call_void(
"js_object_set_field_by_name",
&[
(I64, &this_handle),
(I64, &name_key_raw),
(DOUBLE, &name_val_box),
],
);
// #9410: `stack`. This arm stamps `message` and `name` onto an
// ordinary class instance; nothing ever filled `stack`, so
// `new MyError("x").stack` was `undefined` where the base
// `new Error("x").stack` is a string. The runtime installs a
// lazily-formatted own accessor and captures the FRAME here,
// at the construction site.
let blk = ctx.block();
// Reload `this`: the `message`/`name` stamps above can
// collect, so the earlier `this_box` may be stale (#8770).
let this_for_stack = blk.load(DOUBLE, &this_slot_for_err);
blk.call_void(
"js_error_subclass_capture_stack",
&[(DOUBLE, &this_for_stack)],
);
return true;
}
false
Expand Down
11 changes: 4 additions & 7 deletions crates/perry-codegen/src/runtime_decls/objects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,10 @@ pub fn declare_phase_b_objects(module: &mut LlModule) {
VOID,
&[I64, I64, DOUBLE],
);
// #6469: spec default Error-init for the synthesized standalone ctor of a
// no-own-ctor `class X extends Error {}` (this, message, name-string ptr).
module.declare_function(
"js_error_subclass_default_init",
VOID,
&[DOUBLE, DOUBLE, I64],
);
// #6469/#9440: spec default Error-init for the synthesized standalone ctor
// of a no-own-ctor `class X extends Error {}` (this, message). `name`
// remains inherited from the terminating Error-family prototype.
module.declare_function("js_error_subclass_default_init", VOID, &[DOUBLE, DOUBLE]);
module.declare_function(
"js_object_set_field_by_name_nonconfigurable",
VOID,
Expand Down
Loading
Loading