diff --git a/changelog.d/9438-fancy-regex-split.md b/changelog.d/9438-fancy-regex-split.md new file mode 100644 index 0000000000..a09bc7fbbd --- /dev/null +++ b/changelog.d/9438-fancy-regex-split.md @@ -0,0 +1,14 @@ +### Fixed + +- **Regex separators that require fancy-regex now run the same + `RegExp.prototype[Symbol.split]` cursor algorithm as ordinary patterns.** The + old `find_iter` fallback emitted a trailing `""` after a zero-width match at + the end and discarded separator captures. The fancy lane now uses + `captures_from_pos` on the complete subject, performs the spec's sticky + `q`/`p` walk bounded by `q < size`, splices matched and unmatched captures, + and stops as soon as `limit` is reached. + + Coverage includes lookbehind and lookahead separators, captures, start/end + matches, empty subjects, limits, and the multiline `^` / `$` forms rewritten + onto the fancy lane by #9427. The pre-existing runtime test that pinned the + incorrect trailing element now asserts Node's result. diff --git a/changelog.d/9466-scope-aware-class-disambiguation.md b/changelog.d/9466-scope-aware-class-disambiguation.md new file mode 100644 index 0000000000..69e6525a2b --- /dev/null +++ b/changelog.d/9466-scope-aware-class-disambiguation.md @@ -0,0 +1,59 @@ +### Fixed + +- **Same-name `class` declarations at different lexical depths are distinct + classes again — the inner body is no longer silently dropped.** Perry keeps + two same-named classes apart by registering the second under a uniquified key + (`M$0`); the key was minted once per **name** instead of once per **scope**, so + the third and every later `class M` aliased onto the second's ClassId. Whichever + body registered first won, and the program ran the wrong methods with no + diagnostic: + + ```ts + class M { v() { return "top"; } } + function h() { + class M { v() { return "outer"; } } + function h2() { class M { v() { return "inner"; } } return new M().v(); } + return [new M().v(), h2()].join(","); + } + console.log(new M().v(), h()); // node: top outer,inner perry: top outer,outer + ``` + + Two defects, one symptom: + + 1. **The "already renamed" guard was per name, not per scope.** + `maybe_rename_colliding_class` returned early on + `class_renames.contains_key(name)` — but `class_renames` is inherited by + nested bodies (it is saved and restored per body, so an enclosing body's + alias is live while the nested one lowers). A nested body declaring the same + name therefore took the early return and registered its `class X` under the + **outer** body's key. The map now carries the source span of the scope that + minted each alias, so the guard means "this scope already renamed it" — the + idempotence the guard existed for — and every nested scope mints its own. + + 2. **Block scopes never ran the disambiguation scan at all.** Only function + bodies did, so two sibling `{ class Blk { … } }` blocks shared one ClassId + and the second ran the first's body: + + ```ts + { class Blk { v() { return "b1"; } } console.log(new Blk().v()); } // b1 + { class Blk { v() { return "b2"; } } console.log(new Blk().v()); } // node b2, perry b1 + ``` + + `class` is block-scoped, so the scan is now bracketed at every `{ … }`-shaped + scope — bare block, `if` / `else` branch, loop body, `try` / `catch` / + `finally`, and `switch` — mirroring `register_block_forward_lexicals` + (#6062), which brackets the same boundary for TDZ names: record only what the + scope changed, undo exactly that, so an alias owned by an enclosing scope + survives. + + This is an **identity** fix, not a naming one: each declaration now gets its own + ClassId, so `instanceof` across the shadowing boundary is correct in both + directions (an inner instance is not `instanceof` the outer class, and vice + versa), `Object.getPrototypeOf(inner) !== Outer.prototype`, and `class Sub + extends M` inside the inner scope extends the **inner** `M`. `.name` keeps + reporting the source name for all of them — the display-name override #9413 + (PR #9465) installed on this exact path is what carries it, and every newly + minted alias goes through the same `lower_class_decl` site that records it. + + Validated by `test-files/test_gap_9466_shadowed_class_identity.ts`, byte-identical + to `node --experimental-strip-types`. diff --git a/changelog.d/9495-strict-inherited-property-set-prototype-walk.md b/changelog.d/9495-strict-inherited-property-set-prototype-walk.md new file mode 100644 index 0000000000..15812036ca --- /dev/null +++ b/changelog.d/9495-strict-inherited-property-set-prototype-walk.md @@ -0,0 +1,96 @@ +### Fixed + +- **Strict `o.x += 1` against an INHERITED non-writable property, getter-only + accessor or setter now runs the prototype walk.** + + ```js + "use strict"; + const proto = {}; + Object.defineProperty(proto, "x", { value: 10, writable: false, configurable: true }); + const a = Object.create(proto); + a.x += 1; // node: TypeError Perry: silent, created own a.x = 11 + + const g = {}; + Object.defineProperty(g, "x", { get() { return 20; }, configurable: true }); + const b = Object.create(g); + b.x += 1; // node: TypeError Perry: silent, created own b.x = 21 + + const calls = []; + const s = {}; + Object.defineProperty(s, "x", { get() { return 30; }, set(v) { calls.push(v); }, configurable: true }); + const d = Object.create(s); + d.x += 1; // node: setter runs with 31, no own property + // Perry: setter NEVER ran, created own d.x = 31 + ``` + + ES2024 §10.1.9.2 (`OrdinarySetWithOwnDescriptor`): when the receiver has no + own property, `[[Set]]` walks to the parent and the *parent's* descriptor + decides — a non-writable data property rejects, a getter-only accessor + rejects, a setter runs with the original receiver, and only a writable data + property (or the end of the chain) creates a new own property. `PutValue` + then throws on a rejection iff the reference is strict. + + `o.x = v` was already right — it lowers to `Expr::PutValueSet` → + `js_put_value_set`, whose `ordinary_set_with_receiver` walks the chain — and + #9459 made the *sloppy* half of these spellings right as a side effect of + routing the sloppy `Expr::PropertySet` tail to that same entry. Only the + **strict** spellings that lower to `Expr::PropertySet` — compound and logical + assignment, `for`-of heads, expression-position destructuring targets — and + the strict object-by-name arms of `Expr::IndexSet` (`o["x"] += 1`, + `o[k] += 1`) were still wrong. This is a missing prototype walk, not a + missing `Throw` flag: the opposite direction from #9422 and a different + defect from #9459. + + Root cause: the strict tails ended in + `js_typed_feedback_object_set_field_by_name_fast` / + `js_typed_feedback_object_set_field_by_name` → `js_object_set_field_by_name`, + an **own-property** store. The shape-transition fast path inside it already + declines any receiver whose prototype is not the ordinary one, so every one of + these receivers fell to the slow branch — which appended an own property + without ever consulting the chain. + + Fix: the strict tails now reach the same receiver-aware `[[Set]]` the sloppy + tails and the `=` lane use, `js_put_value_set(target, key, value, receiver, + strict)`, so the two modes are one tail distinguished by the `Throw` flag + alone. The typed-feedback `PropertySet` site moves with the store: it is + registered in both modes (it describes the store, not its strictness) and + observed by the pure-recording `js_typed_feedback_observe_property_set`, + compile-gated on `PERRY_TYPED_FEEDBACK` exactly as #7480 step 4 gates every + other recording helper — a default build emits the bare `js_put_value_set` + call and nothing else. Nothing was left for the old dispatching wrapper to + decide (the receiver-aware entry makes the fast-path choice itself), so the + #7480 "dispatching wrappers still emitted in a default build" gate is + re-pointed at the method-call dispatcher the same fixture emits, and asserted + as a *call* rather than a symbol (the symbol match was satisfied by the + `declare` line alone). + + - `crates/perry-codegen/src/expr/property_set.rs` — + `lower_put_value_property_set_by_name` replaces the sloppy-only helper + and is the generic tail for both modes; `caller` / `arguments` keep their + `js_object_set_field_by_name` route (poisoned-accessor handling, unrelated + to either flag or walk). `emit_typed_feedback_property_set_observation` + carries the site. + - `crates/perry-codegen/src/expr/index_set.rs` — + `lower_object_index_set_put_value` replaces the sloppy-only helper on the + literal-string-key and string-typed-key object arms. + - `crates/perry-codegen/src/expr/typed_feedback.rs` — + `TypedFeedbackContract::put_value_set`. + - `test-files/test_gap_9495_strict_inherited_property_set.cts` — the three + inherited receivers plus a two-level chain, a class accessor on the chain + and a Proxy on the chain (receiver forwarded), across `+=`, `&&=`, `??=` + (short-circuit control), `for`-of heads, destructuring in statement and + expression position, `o["x"]`, `o[k]`, `o[anyKey]`, `[o[k]] = arr`, with + accepted-store controls (inherited writable data, new key beside an + inherited accessor, class-ref receiver) and the already-correct `=` / + `++` lanes — **both modes**, so "silent because sloppy" and "silent because + the walk never ran" are told apart by the setter-call log and `hasOwn`. + - `test-files/test_gap_9459_property_set_strictness.cts` — the strict + inherited twins are spelled `+=` now, as that file's comment promised. + + Left as it was: `js_class_field_set_fallback` (the class-field arm's + guard-miss path) and `js_object_set_field_by_property_id` (the + computed-runtime-members class route) are still own-property stores; neither + is reachable for the receivers above without a class-typed variable holding + an `Object.create`d value, and each is its own lane. A DECLARED `static` + field losing `K.n += 1` in both modes (found while building the fixture) is + a static-slot lane defect, not a walk, and is filed as #9526. diff --git a/changelog.d/9509-date-parse-tail.md b/changelog.d/9509-date-parse-tail.md new file mode 100644 index 0000000000..771ff01e62 --- /dev/null +++ b/changelog.d/9509-date-parse-tail.md @@ -0,0 +1,17 @@ +### Fixed + +- **ISO-shaped date parsing now consumes the complete clock tail instead of + silently discarding it.** Space-separated `AM` / `PM`, the GMT family, and + V8's fixed `EST` / `EDT` / `CST` / `CDT` / `MST` / `MDT` / `PST` / `PDT` + table are applied to the parsed instant; the same zone words work on + date-only and partial `YYYY` / `YYYY-MM` forms. Missing day/month components, + repeated whitespace, numeric offsets, and parenthesized comments retain + Node's measured behavior. + + Every other suffix must now make the parse fail. In particular, + `"2026-09-01 10:30GMT"`, `"...10:30EST"` and `"...10:30PM"` are Invalid + Date, matching Node, rather than plausible but wrong local instants produced + from the `HH:MM` prefix. The expanded #9449 parity fixture and focused + runtime tests cover both 12-hour boundaries, zone-plus-meridiem, all eight US + abbreviations, partial dates, date-only zone words, and invalid attached or + unknown tails with host-zone-independent assertions. diff --git a/changelog.d/9511-error-name-ownership.md b/changelog.d/9511-error-name-ownership.md new file mode 100644 index 0000000000..6395b5db28 --- /dev/null +++ b/changelog.d/9511-error-name-ownership.md @@ -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`. diff --git a/changelog.d/9531-cjs-default-table-mcp-logger-exec-order.md b/changelog.d/9531-cjs-default-table-mcp-logger-exec-order.md new file mode 100644 index 0000000000..82c15e6cbf --- /dev/null +++ b/changelog.d/9531-cjs-default-table-mcp-logger-exec-order.md @@ -0,0 +1,25 @@ +**One shared table for the CommonJS-default module set; claude-code's MCP +debug logger shape and exec/execFile callback order pinned (#9500).** + +The set of Node builtins whose `require()` / default import hands out a +distinct `.default` namespace was hand-maintained in five places (two of +them inside the HIR alone, already disagreeing on `ffi`, `inspector`, +`inspector/promises` and `wasi`); the method-call router's copy is the one +that drifted far enough to break `require('child_process').spawn` (#9485, +#9498). The table now lives once in `perry-dispatch`, built from one literal +per module, and the runtime's property-read and method-call paths, the +`default`-export resolver and the HIR's import lowering all derive from it. +Adding a module is one line; tests pin the table's shape, the HIR's +classification of every row, and the router test's list against the table in +both directions. No behaviour change. + +Two fixtures pin the issue's other findings. The MCP debug logger's exact +write shape — the `using`-downlevel fs wrapper, the timer/dispose buffered +writer, the graceful-shutdown cleanup set and the `appendFileSync` → ENOENT → +`mkdirSync(recursive)` recovery arm that is the only code creating the log +tree — is byte-compared to node; it fails on a pre-#9491 build (the append +did not throw, so the tree was never created) and passes on main. The +exec/execFile callback order is pinned as what node guarantees — completion +order, whichever API launched the child or came first; the inverted order for +two instant `echo`s is a same-turn batch-delivery artefact node flips with +submission order, not a rule. diff --git a/changelog.d/9532-set-receiver-root-across-value.md b/changelog.d/9532-set-receiver-root-across-value.md new file mode 100644 index 0000000000..fda555d99d --- /dev/null +++ b/changelog.d/9532-set-receiver-root-across-value.md @@ -0,0 +1,37 @@ +### Fixed + +- **`s.has(makeKey())` / `s.delete(makeKey())` on a module-level `Set` no + longer read a moved receiver.** `Expr::SetHas` and `Expr::SetDelete` + (`expr/bigint_set.rs`) lowered the receiver, masked it to a raw `i64` + handle, *then* lowered the value expression, and consumed the handle after + — the #6970 shape their Map twins (`MapGet` / `MapHas` / `MapDelete`) were + fixed for, found live by #9522's audit of every Map/Set/WeakMap lowering + (#9523). A function-local Set was already covered — `root_reload` re-derives + a shadow-slot load and its unmask below every collection point — but a + module-level Set is a `@perry_global_*` load, which that pass deliberately + does not reload, so an evacuating minor inside the value's evaluation left + `js_set_has` a from-space header. Measured: the new gap fixture SIGSEGVs on + unfixed main where node prints `bad=0`; the from-space quarantine reports the + fault address as retired by the first minor with a last-known object of + `GC_TYPE_SET`. + + Both arms now root the receiver in a `RootedGroup` before the value is + lowered and re-read it from the slot afterwards, exactly as the Map twins + do; when the value cannot collect nothing is pushed and the IR is unchanged. + `bigint_set.rs` joins the rooting migration ledger. + +- **`this.field.set(a, 1).set(b, 2)` consumes the receiver `js_map_set` + returns.** `lower_call/property_get/map_set.rs`'s `"set"` arm called the + helper as `void` and returned the receiver box it had read *before* the + call. `js_map_set` returns the receiver as it stands after the insert — for + a `class X extends Map` instance the runtime roots the movable + `ObjectHeader` across the grow and hands back the relocated address — so the + chained call could dispatch on a from-space pointer. The arm now re-boxes + the returned pointer, as `Expr::MapSet` already did. Latent (the minor must + fire inside the first insert's grow); pinned by a chained-set fixture. + + Fixtures: `test-files/test_gap_9523_set_receiver_roots_across_value.ts` + (fails on unfixed main, byte-identical to node fixed) and + `test_gap_9523_map_set_chain_returns_receiver.ts`; codegen contract in + `temp_root_coverage/set_receiver.rs`, sabotage-verified under both root + lowerings. diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 94c3c7f0d2..696690f59f 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -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) diff --git a/crates/perry-codegen/src/expr/bigint_set.rs b/crates/perry-codegen/src/expr/bigint_set.rs index 7683f57fc9..bc6f2460fa 100644 --- a/crates/perry-codegen/src/expr/bigint_set.rs +++ b/crates/perry-codegen/src/expr/bigint_set.rs @@ -9,6 +9,7 @@ use perry_hir::types::Type as HirType; use perry_hir::{BinaryOp, Expr}; use crate::nanbox::{double_literal, POINTER_MASK_I64}; +use crate::rooting::{operand_may_collect, with_rooted_group, RootedGroup}; use crate::type_analysis::{ is_bigint_expr, set_static_type_args, string_value_is_runtime_guaranteed, }; @@ -133,6 +134,48 @@ fn guarded_set_number_add(ctx: &mut FnCtx<'_>, set_handle: &str, value_box: &str ) } +/// The `Set` receiver's raw handle on the UNPROTECTED path of a `SetHas` / +/// `SetDelete` lowering (#9523), or `None` when the receiver is rooted. +/// +/// Both arms lower the receiver, then the value, then consume the receiver as +/// a raw `i64` — the #6970 shape their Map twins (`MapGet` / `MapHas` / +/// `MapDelete`) were fixed for. The receiver is now a [`RootedGroup`] operand, +/// pushed before `value` is lowered. Unboxing eagerly is only sound when the +/// group pushed nothing: then `value` cannot collect, `reread` hands the +/// original register back, and the emitted IR — register numbering included — +/// is exactly what it was before this change. On the protected path the +/// handle has to come from the *re-read* box, below the value's lowering, so +/// it is derived in [`reread_set_receiver`] instead. +fn eager_set_handle(ctx: &mut FnCtx<'_>, group: &RootedGroup<'_>) -> Result> { + if group.is_rooted() { + return Ok(None); + } + let s_box = group.reread(ctx, 0)?; + let blk = ctx.block(); + Ok(Some(unbox_to_i64(blk, &s_box))) +} + +/// Re-derive the `Set` receiver handle AFTER `value` has been lowered (#9523). +/// +/// Mirrors `math_simple.rs`'s `reread_map_set_receiver_and_key`: on the +/// protected path the box is read back out of its temp-root slot — mandatory, +/// since an evacuating minor inside the value's lowering rewrites the slot and +/// the register pushed beforehand names from-space — and the handle is +/// unboxed from that. On the unprotected path this is the eagerly computed +/// handle and nothing is emitted. +fn reread_set_receiver( + ctx: &mut FnCtx<'_>, + group: &RootedGroup<'_>, + s_handle_unrooted: &Option, +) -> Result { + if let Some(handle) = s_handle_unrooted { + return Ok(handle.clone()); + } + let s_box = group.reread(ctx, 0)?; + let blk = ctx.block(); + Ok(unbox_to_i64(blk, &s_box)) +} + fn guarded_set_number_has(ctx: &mut FnCtx<'_>, set_handle: &str, value_box: &str) -> String { let guard_raw = ctx .block() @@ -870,214 +913,230 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ); let use_string_set = is_static_string_set(ctx, set) && string_value_is_runtime_guaranteed(ctx, value); - let s_box = lower_expr(ctx, set)?; - let s_handle = { - let blk = ctx.block(); - unbox_to_i64(blk, &s_box) - }; - let i32_v = if use_i32_set { - let value_i32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I32)?; - let i32_v = { - let blk = ctx.block(); - blk.call( - I32, - "js_set_has_i32", - &[(I64, &s_handle), (I32, &value_i32.value)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetHas", - "collection_typed_value.set_has_i32", - &value_i32, - "set", - "int32_value_helper", - "js_set_has_i32", - "set_slot", - ); - i32_v - } else if use_u32_set { - let value_u32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::U32)?; - let i32_v = { - let blk = ctx.block(); - blk.call( - I32, - "js_set_has_u32", - &[(I64, &s_handle), (I32, &value_u32.value)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetHas", - "collection_typed_value.set_has_u32", - &value_u32, - "set", - "uint32_value_helper", - "js_set_has_u32", - "set_slot", - ); - i32_v - } else if use_f32_set { - let value_f32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::F32)?; - let i32_v = { - let blk = ctx.block(); - blk.call( - I32, - "js_set_has_f32", - &[(I64, &s_handle), (F32, &value_f32.value)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetHas", - "collection_typed_value.set_has_f32", - &value_f32, - "set", - "float32_value_helper", - "js_set_has_f32", - "set_slot", - ); - i32_v - } else if use_boolean_set { - let value_i1 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I1)?; - let i32_v = { - let blk = ctx.block(); - let value_i32 = blk.zext(I1, &value_i1.value, I32); - blk.call( - I32, - "js_set_has_bool", - &[(I64, &s_handle), (I32, &value_i32)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetHas", - "collection_typed_value.set_has_bool", - &value_i1, - "set", - "boolean_value_helper", - "js_set_has_bool", - "set_slot", - ); - i32_v - } else if use_number_set { - let v_box = lower_expr(ctx, value)?; - guarded_set_number_has(ctx, &s_handle, &v_box) - } else { - if use_string_set { - let value_ref = lower_expr_native( + // #9523: the receiver used to be unboxed to a raw `i64` HERE, before + // `value` was lowered, and consumed after — a bare SSA register across + // an arbitrary allocation, the #6970 shape `MapGet` / `MapHas` / + // `MapDelete` were fixed for. `s.has(makeKey())` with an evacuating + // minor inside `makeKey` handed `js_set_has` a from-space header. Root the + // receiver across the value's lowering and derive the handle from the + // re-read box; when the value cannot collect nothing is pushed and the + // eager unbox keeps the IR byte for byte. + let value_collects = operand_may_collect(ctx, value); + let i32_v = with_rooted_group(ctx, 1, |ctx, group| { + group.lower(ctx, set, value_collects)?; + let s_handle_unrooted = eager_set_handle(ctx, group)?; + let i32_v = if use_i32_set { + let value_i32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I32)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + blk.call( + I32, + "js_set_has_i32", + &[(I64, &s_handle), (I32, &value_i32.value)], + ) + }; + record_collection_typed_value_selected( ctx, - value, - crate::native_value::ExpectedNativeRep::StringRef, - )?; + "SetHas", + "collection_typed_value.set_has_i32", + &value_i32, + "set", + "int32_value_helper", + "js_set_has_i32", + "set_slot", + ); + i32_v + } else if use_u32_set { + let value_u32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::U32)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; let i32_v = { let blk = ctx.block(); - let i32_v = blk.call( + blk.call( I32, - "js_set_has_string", - &[(I64, &s_handle), (I64, &value_ref.value)], - ); - i32_v + "js_set_has_u32", + &[(I64, &s_handle), (I32, &value_u32.value)], + ) }; - record_collection_string_key_selected( + record_collection_typed_value_selected( ctx, "SetHas", - "collection_string_key.set_has", - &value_ref.value, + "collection_typed_value.set_has_u32", + &value_u32, "set", - "js_set_has_string", + "uint32_value_helper", + "js_set_has_u32", + "set_slot", ); + i32_v + } else if use_f32_set { + let value_f32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::F32)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + blk.call( + I32, + "js_set_has_f32", + &[(I64, &s_handle), (F32, &value_f32.value)], + ) + }; record_collection_typed_value_selected( ctx, "SetHas", - "collection_typed_value.set_has_string", - &value_ref, + "collection_typed_value.set_has_f32", + &value_f32, "set", - "string_value_helper", - "js_set_has_string", + "float32_value_helper", + "js_set_has_f32", "set_slot", ); i32_v - } else { - let v_box = lower_expr(ctx, value)?; + } else if use_boolean_set { + let value_i1 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I1)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; let i32_v = { let blk = ctx.block(); - blk.call(I32, "js_set_has", &[(I64, &s_handle), (DOUBLE, &v_box)]) + let value_i32 = blk.zext(I1, &value_i1.value, I32); + blk.call( + I32, + "js_set_has_bool", + &[(I64, &s_handle), (I32, &value_i32)], + ) }; - if receiver_i32_set { - record_collection_typed_value_fallback( - ctx, - "SetHas", - "collection_typed_value.set_has_generic", - &v_box, - "set", - "int32_value_helper", - "js_set_has", - "value_expr_not_native_i32", - ); - } else if receiver_u32_set { - record_collection_typed_value_fallback( - ctx, - "SetHas", - "collection_typed_value.set_has_generic", - &v_box, - "set", - "uint32_value_helper", - "js_set_has", - "value_expr_not_native_u32", - ); - } else if receiver_f32_set { - record_collection_typed_value_fallback( + record_collection_typed_value_selected( + ctx, + "SetHas", + "collection_typed_value.set_has_bool", + &value_i1, + "set", + "boolean_value_helper", + "js_set_has_bool", + "set_slot", + ); + i32_v + } else if use_number_set { + let v_box = lower_expr(ctx, value)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + guarded_set_number_has(ctx, &s_handle, &v_box) + } else { + if use_string_set { + let value_ref = lower_expr_native( ctx, - "SetHas", - "collection_typed_value.set_has_generic", - &v_box, - "set", - "float32_value_helper", - "js_set_has", - "value_expr_not_native_f32", - ); - } else if receiver_boolean_set { - record_collection_typed_value_fallback( + value, + crate::native_value::ExpectedNativeRep::StringRef, + )?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + let i32_v = blk.call( + I32, + "js_set_has_string", + &[(I64, &s_handle), (I64, &value_ref.value)], + ); + i32_v + }; + record_collection_string_key_selected( ctx, "SetHas", - "collection_typed_value.set_has_generic", - &v_box, + "collection_string_key.set_has", + &value_ref.value, "set", - "boolean_value_helper", - "js_set_has", - "value_expr_not_native_i1", + "js_set_has_string", ); - } else if receiver_number_set { - record_collection_number_key_fallback( + record_collection_typed_value_selected( ctx, "SetHas", - "collection_number_value.set_has_generic", - &v_box, + "collection_typed_value.set_has_string", + &value_ref, "set", - "number_value_helper", - "js_set_has", - "value_expr_not_numeric", - "value", + "string_value_helper", + "js_set_has_string", + "set_slot", ); + i32_v } else { - record_collection_string_key_fallback( - ctx, - "SetHas", - "collection_string_key.set_has_generic", - &v_box, - "set", - "js_set_has", - "receiver_or_value_not_static_string", - ); + let v_box = lower_expr(ctx, value)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + blk.call(I32, "js_set_has", &[(I64, &s_handle), (DOUBLE, &v_box)]) + }; + if receiver_i32_set { + record_collection_typed_value_fallback( + ctx, + "SetHas", + "collection_typed_value.set_has_generic", + &v_box, + "set", + "int32_value_helper", + "js_set_has", + "value_expr_not_native_i32", + ); + } else if receiver_u32_set { + record_collection_typed_value_fallback( + ctx, + "SetHas", + "collection_typed_value.set_has_generic", + &v_box, + "set", + "uint32_value_helper", + "js_set_has", + "value_expr_not_native_u32", + ); + } else if receiver_f32_set { + record_collection_typed_value_fallback( + ctx, + "SetHas", + "collection_typed_value.set_has_generic", + &v_box, + "set", + "float32_value_helper", + "js_set_has", + "value_expr_not_native_f32", + ); + } else if receiver_boolean_set { + record_collection_typed_value_fallback( + ctx, + "SetHas", + "collection_typed_value.set_has_generic", + &v_box, + "set", + "boolean_value_helper", + "js_set_has", + "value_expr_not_native_i1", + ); + } else if receiver_number_set { + record_collection_number_key_fallback( + ctx, + "SetHas", + "collection_number_value.set_has_generic", + &v_box, + "set", + "number_value_helper", + "js_set_has", + "value_expr_not_numeric", + "value", + ); + } else { + record_collection_string_key_fallback( + ctx, + "SetHas", + "collection_string_key.set_has_generic", + &v_box, + "set", + "js_set_has", + "receiver_or_value_not_static_string", + ); + } + i32_v } - i32_v - } - }; + }; + Ok(i32_v) + })?; let blk = ctx.block(); let bit = blk.icmp_ne(I32, &i32_v, "0"); let tagged = blk.select( @@ -1110,214 +1169,230 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ); let use_string_set = is_static_string_set(ctx, set) && string_value_is_runtime_guaranteed(ctx, value); - let s_box = lower_expr(ctx, set)?; - let s_handle = { - let blk = ctx.block(); - unbox_to_i64(blk, &s_box) - }; - let i32_v = if use_i32_set { - let value_i32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I32)?; - let i32_v = { - let blk = ctx.block(); - blk.call( - I32, - "js_set_delete_i32", - &[(I64, &s_handle), (I32, &value_i32.value)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetDelete", - "collection_typed_value.set_delete_i32", - &value_i32, - "set", - "int32_value_helper", - "js_set_delete_i32", - "set_slot", - ); - i32_v - } else if use_u32_set { - let value_u32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::U32)?; - let i32_v = { - let blk = ctx.block(); - blk.call( - I32, - "js_set_delete_u32", - &[(I64, &s_handle), (I32, &value_u32.value)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetDelete", - "collection_typed_value.set_delete_u32", - &value_u32, - "set", - "uint32_value_helper", - "js_set_delete_u32", - "set_slot", - ); - i32_v - } else if use_f32_set { - let value_f32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::F32)?; - let i32_v = { - let blk = ctx.block(); - blk.call( - I32, - "js_set_delete_f32", - &[(I64, &s_handle), (F32, &value_f32.value)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetDelete", - "collection_typed_value.set_delete_f32", - &value_f32, - "set", - "float32_value_helper", - "js_set_delete_f32", - "set_slot", - ); - i32_v - } else if use_boolean_set { - let value_i1 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I1)?; - let i32_v = { - let blk = ctx.block(); - let value_i32 = blk.zext(I1, &value_i1.value, I32); - blk.call( - I32, - "js_set_delete_bool", - &[(I64, &s_handle), (I32, &value_i32)], - ) - }; - record_collection_typed_value_selected( - ctx, - "SetDelete", - "collection_typed_value.set_delete_bool", - &value_i1, - "set", - "boolean_value_helper", - "js_set_delete_bool", - "set_slot", - ); - i32_v - } else if use_number_set { - let v_box = lower_expr(ctx, value)?; - guarded_set_number_delete(ctx, &s_handle, &v_box) - } else { - if use_string_set { - let value_ref = lower_expr_native( + // #9523: the receiver used to be unboxed to a raw `i64` HERE, before + // `value` was lowered, and consumed after — a bare SSA register across + // an arbitrary allocation, the #6970 shape `MapGet` / `MapHas` / + // `MapDelete` were fixed for. `s.has(makeKey())` with an evacuating + // minor inside `makeKey` handed `js_set_delete` a from-space header. Root the + // receiver across the value's lowering and derive the handle from the + // re-read box; when the value cannot collect nothing is pushed and the + // eager unbox keeps the IR byte for byte. + let value_collects = operand_may_collect(ctx, value); + let i32_v = with_rooted_group(ctx, 1, |ctx, group| { + group.lower(ctx, set, value_collects)?; + let s_handle_unrooted = eager_set_handle(ctx, group)?; + let i32_v = if use_i32_set { + let value_i32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I32)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + blk.call( + I32, + "js_set_delete_i32", + &[(I64, &s_handle), (I32, &value_i32.value)], + ) + }; + record_collection_typed_value_selected( ctx, - value, - crate::native_value::ExpectedNativeRep::StringRef, - )?; + "SetDelete", + "collection_typed_value.set_delete_i32", + &value_i32, + "set", + "int32_value_helper", + "js_set_delete_i32", + "set_slot", + ); + i32_v + } else if use_u32_set { + let value_u32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::U32)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; let i32_v = { let blk = ctx.block(); - let i32_v = blk.call( + blk.call( I32, - "js_set_delete_string", - &[(I64, &s_handle), (I64, &value_ref.value)], - ); - i32_v + "js_set_delete_u32", + &[(I64, &s_handle), (I32, &value_u32.value)], + ) }; - record_collection_string_key_selected( + record_collection_typed_value_selected( ctx, "SetDelete", - "collection_string_key.set_delete", - &value_ref.value, + "collection_typed_value.set_delete_u32", + &value_u32, "set", - "js_set_delete_string", + "uint32_value_helper", + "js_set_delete_u32", + "set_slot", ); + i32_v + } else if use_f32_set { + let value_f32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::F32)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + blk.call( + I32, + "js_set_delete_f32", + &[(I64, &s_handle), (F32, &value_f32.value)], + ) + }; record_collection_typed_value_selected( ctx, "SetDelete", - "collection_typed_value.set_delete_string", - &value_ref, + "collection_typed_value.set_delete_f32", + &value_f32, "set", - "string_value_helper", - "js_set_delete_string", + "float32_value_helper", + "js_set_delete_f32", "set_slot", ); i32_v - } else { - let v_box = lower_expr(ctx, value)?; + } else if use_boolean_set { + let value_i1 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I1)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; let i32_v = { let blk = ctx.block(); - blk.call(I32, "js_set_delete", &[(I64, &s_handle), (DOUBLE, &v_box)]) + let value_i32 = blk.zext(I1, &value_i1.value, I32); + blk.call( + I32, + "js_set_delete_bool", + &[(I64, &s_handle), (I32, &value_i32)], + ) }; - if receiver_i32_set { - record_collection_typed_value_fallback( - ctx, - "SetDelete", - "collection_typed_value.set_delete_generic", - &v_box, - "set", - "int32_value_helper", - "js_set_delete", - "value_expr_not_native_i32", - ); - } else if receiver_u32_set { - record_collection_typed_value_fallback( - ctx, - "SetDelete", - "collection_typed_value.set_delete_generic", - &v_box, - "set", - "uint32_value_helper", - "js_set_delete", - "value_expr_not_native_u32", - ); - } else if receiver_f32_set { - record_collection_typed_value_fallback( + record_collection_typed_value_selected( + ctx, + "SetDelete", + "collection_typed_value.set_delete_bool", + &value_i1, + "set", + "boolean_value_helper", + "js_set_delete_bool", + "set_slot", + ); + i32_v + } else if use_number_set { + let v_box = lower_expr(ctx, value)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + guarded_set_number_delete(ctx, &s_handle, &v_box) + } else { + if use_string_set { + let value_ref = lower_expr_native( ctx, - "SetDelete", - "collection_typed_value.set_delete_generic", - &v_box, - "set", - "float32_value_helper", - "js_set_delete", - "value_expr_not_native_f32", - ); - } else if receiver_boolean_set { - record_collection_typed_value_fallback( + value, + crate::native_value::ExpectedNativeRep::StringRef, + )?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + let i32_v = blk.call( + I32, + "js_set_delete_string", + &[(I64, &s_handle), (I64, &value_ref.value)], + ); + i32_v + }; + record_collection_string_key_selected( ctx, "SetDelete", - "collection_typed_value.set_delete_generic", - &v_box, + "collection_string_key.set_delete", + &value_ref.value, "set", - "boolean_value_helper", - "js_set_delete", - "value_expr_not_native_i1", + "js_set_delete_string", ); - } else if receiver_number_set { - record_collection_number_key_fallback( + record_collection_typed_value_selected( ctx, "SetDelete", - "collection_number_value.set_delete_generic", - &v_box, + "collection_typed_value.set_delete_string", + &value_ref, "set", - "number_value_helper", - "js_set_delete", - "value_expr_not_numeric", - "value", + "string_value_helper", + "js_set_delete_string", + "set_slot", ); + i32_v } else { - record_collection_string_key_fallback( - ctx, - "SetDelete", - "collection_string_key.set_delete_generic", - &v_box, - "set", - "js_set_delete", - "receiver_or_value_not_static_string", - ); + let v_box = lower_expr(ctx, value)?; + let s_handle = reread_set_receiver(ctx, group, &s_handle_unrooted)?; + let i32_v = { + let blk = ctx.block(); + blk.call(I32, "js_set_delete", &[(I64, &s_handle), (DOUBLE, &v_box)]) + }; + if receiver_i32_set { + record_collection_typed_value_fallback( + ctx, + "SetDelete", + "collection_typed_value.set_delete_generic", + &v_box, + "set", + "int32_value_helper", + "js_set_delete", + "value_expr_not_native_i32", + ); + } else if receiver_u32_set { + record_collection_typed_value_fallback( + ctx, + "SetDelete", + "collection_typed_value.set_delete_generic", + &v_box, + "set", + "uint32_value_helper", + "js_set_delete", + "value_expr_not_native_u32", + ); + } else if receiver_f32_set { + record_collection_typed_value_fallback( + ctx, + "SetDelete", + "collection_typed_value.set_delete_generic", + &v_box, + "set", + "float32_value_helper", + "js_set_delete", + "value_expr_not_native_f32", + ); + } else if receiver_boolean_set { + record_collection_typed_value_fallback( + ctx, + "SetDelete", + "collection_typed_value.set_delete_generic", + &v_box, + "set", + "boolean_value_helper", + "js_set_delete", + "value_expr_not_native_i1", + ); + } else if receiver_number_set { + record_collection_number_key_fallback( + ctx, + "SetDelete", + "collection_number_value.set_delete_generic", + &v_box, + "set", + "number_value_helper", + "js_set_delete", + "value_expr_not_numeric", + "value", + ); + } else { + record_collection_string_key_fallback( + ctx, + "SetDelete", + "collection_string_key.set_delete_generic", + &v_box, + "set", + "js_set_delete", + "receiver_or_value_not_static_string", + ); + } + i32_v } - i32_v - } - }; + }; + Ok(i32_v) + })?; let blk = ctx.block(); let bit = blk.icmp_ne(I32, &i32_v, "0"); let tagged = blk.select( diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 48bdd808f0..9be4d7eb80 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -357,27 +357,37 @@ fn lower_array_index_set_via_runtime_key( /// #9459: the SLOPPY object-by-name tail for `Expr::IndexSet`. /// -/// The two string-key arms below reach `js_typed_feedback_object_set_field_by_name` -/// → `js_object_set_field_by_name`, which has no `strict` parameter and rejects a -/// non-writable slot by throwing. #9426 carried `assignment_strict` to the ARRAY -/// element lanes; the object-by-name lanes on the same node still threw, so -/// `for (o[k] of …)` and `o[k] += 1` on a frozen object stopped a sloppy program -/// node runs to completion. +/// #9459 / #9495: the terminal store for the two string-key object arms below, +/// in BOTH modes. /// -/// `Set(O, ToPropertyKey(k), V, false)` is `js_put_value_set(target, key, value, -/// receiver, 0)` — the same entry sloppy `o[k] = v` already reaches, because +/// Those arms used to reach `js_typed_feedback_object_set_field_by_name` → +/// `js_object_set_field_by_name`: no `strict` parameter, so a rejected sloppy +/// write threw (#9459 -- #9426 had carried `assignment_strict` to the ARRAY +/// element lanes only), and an OWN-property store, so a strict write skipped +/// the prototype walk (#9495 -- an inherited setter never ran, an inherited +/// non-writable / getter-only property never threw, and an own property was +/// created where ES2024 SS10.1.9.2 creates none). +/// +/// `Set(O, ToPropertyKey(k), V, Throw)` is `js_put_value_set(target, key, value, +/// receiver, strict)` — the same entry `o[k] = v` already reaches, because /// `put_value_index_fast_path` keeps statically-known string keys off this file -/// entirely (`expr/proxy_reflect.rs`). Routing here makes the two spellings agree. +/// entirely (`expr/proxy_reflect.rs`). Routing here makes the spellings agree. +/// +/// The typed-feedback `PropertySet` site the arms carried moves with the store +/// (`property_set::emit_typed_feedback_property_set_observation`), keyed by the +/// same `operation` label each arm used. /// -/// Rooting is the #7639/#7201 window the strict arms open: receiver AND key are -/// live across `value`'s lowering, which is arbitrary user code and can drive an -/// evacuating minor. `target` and `receiver` are one evaluation of the base, so -/// the single lowered box fills both operand slots. -fn lower_sloppy_object_index_set( +/// Rooting is the #7639/#7201 window: receiver AND key are live across +/// `value`'s lowering, which is arbitrary user code and can drive an evacuating +/// minor. `target` and `receiver` are one evaluation of the base, so the single +/// lowered box fills both operand slots. +fn lower_object_index_set_put_value( ctx: &mut FnCtx<'_>, object: &Expr, index: &Expr, value: &Expr, + assignment_strict: bool, + operation: &str, ) -> Result { rooting::with_operands_rooted_across( ctx, @@ -387,17 +397,26 @@ fn lower_sloppy_object_index_set( lower_value_for_dynamic_index_set( ctx, value, - "index_set.sloppy_object_value_bits", - "sloppy_object_index_set_helper_edge", + "index_set.object_string_key_value_bits", + "object_string_key_index_set_helper_edge", ) }, |ctx, vals, (val_double, _val_bits)| { let (obj_box, key_box) = (vals[0].clone(), vals[1].clone()); let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - // Same guard the strict arms emit: `undefined[k] = 1` is a TypeError - // in BOTH modes -- `GetValue` on the base runs before `PutValue` - // ever consults `Throw`. - super::property_set::emit_nullish_write_guard(ctx, &obj_bits, "index", "iset.sloppy"); + // `undefined[k] = 1` is a TypeError in BOTH modes -- `GetValue` on + // the base runs before `PutValue` ever consults `Throw`. + super::property_set::emit_nullish_write_guard(ctx, &obj_bits, "index", "iset.object"); + super::property_set::emit_typed_feedback_property_set_observation( + ctx, + operation, + &obj_bits, + // #7640 section D: the SSO-safe key unbox can allocate, and + // `obj_bits` is the NaN-boxed receiver the group re-read -- + // no raw handle is derived from it before this. + |ctx| unbox_str_handle(ctx.block(), &key_box), + ); + let strict_flag = if assignment_strict { "1" } else { "0" }; let _ = ctx.block().call( DOUBLE, "js_put_value_set", @@ -406,7 +425,7 @@ fn lower_sloppy_object_index_set( (DOUBLE, &key_box), (DOUBLE, &val_double), (DOUBLE, &obj_box), - (I32, "0"), + (I32, strict_flag), ], ); Ok(val_double) @@ -1453,157 +1472,36 @@ pub(crate) fn lower( }); } if let Expr::String(literal) = index.as_ref() { + // #9459 / #9495: the receiver-aware `[[Set]]` in both modes -- + // see `lower_object_index_set_put_value`. The literal key is + // lowered as an operand (an interned-pool load), which is what + // the old strict arm did by hand. + // // #7154: the value expression can collect, and an evacuating // minor inside it relocates the receiver out from under - // `obj_box`. Root it across the evaluation and re-read below. - // The key is a literal, so it is not an operand here at all — - // it is interned into the string pool below. - // - // #9459: strict only -- see `lower_sloppy_object_index_set`. - if !assignment_strict { - return lower_sloppy_object_index_set(ctx, object, index, value); - } - return rooting::with_operands_rooted_across( + // `obj_box`; the helper roots it across the evaluation. + return lower_object_index_set_put_value( ctx, - &[object.as_ref()], - &[value.as_ref()], - |ctx| { - lower_value_for_dynamic_index_set( - ctx, - value, - "index_set.literal_string_value_bits", - "literal_string_index_set_helper_edge", - ) - }, - |ctx, vals, (val_double, _val_bits)| { - let obj_box = vals[0].clone(); - let key_idx = ctx.strings.intern(literal); - let key_handle_global = - format!("@{}", ctx.strings.entry(key_idx).handle_global); - let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - super::property_set::emit_nullish_write_guard( - ctx, - &obj_bits, - literal, - "iset.literal", - ); - let static_classref = super::index_get::index_object_is_class_or_proto_ref( - ctx, - object.as_ref(), - ); - let (obj_handle, key_raw) = { - let blk = ctx.block(); - let obj_handle = super::index_get::classref_preserving_handle( - blk, - &obj_bits, - static_classref, - ); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_raw = blk.and(I64, &key_bits, POINTER_MASK_I64); - (obj_handle, key_raw) - }; - let site_id = emit_typed_feedback_register_site( - ctx, - TypedFeedbackKind::PropertySet, - literal, - TypedFeedbackContract::object_set_by_name(), - ); - ctx.block().call_void( - "js_typed_feedback_object_set_field_by_name", - &[ - (I64, &site_id), - (I64, &obj_handle), - (I64, &key_raw), - (DOUBLE, &val_double), - ], - ); - Ok(val_double) - }, + object, + index, + value, + assignment_strict, + literal, ); } if is_string_expr(ctx, index) { - // #9459: strict only -- see `lower_sloppy_object_index_set`. - if !assignment_strict { - return lower_sloppy_object_index_set(ctx, object, index, value); - } - // #7154: see the literal-key arm above, plus the KEY, which - // sits in the same window. A non-literal string key is an - // ordinary heap string with no registered root of its own, so - // an evacuating minor inside the value's evaluation relocates - // it and leaves the register naming from-space — - // `unbox_str_handle` below would hand the setter a pre-move - // `StringHeader*` and the field would land under a garbage key. - // - // #7639: the receiver's window used to be derived from `value` - // ALONE, which is the half-measure #7201 named. The receiver is - // lowered before the KEY as well, so `o[f()] = 1` — a literal - // RHS that cannot collect, an allocating key that can — left it - // unguarded. As one operand group the receiver's window is the - // disjunction over everything after it, which is what #7201 - // established and what `guard_store_operand`'s two-argument - // form structurally could not say. - return rooting::with_operands_rooted_across( + // #9459 / #9495: as above. #7154 / #7639: the KEY sits in the + // same rooting window as the receiver -- a non-literal string + // key is an ordinary heap string with no registered root of its + // own, and `o[f()] = 1` lowers the receiver before an allocating + // key. The helper roots both as one operand group. + return lower_object_index_set_put_value( ctx, - &[object.as_ref(), index.as_ref()], - &[value.as_ref()], - |ctx| { - lower_value_for_dynamic_index_set( - ctx, - value, - "index_set.string_value_bits", - "string_index_set_helper_edge", - ) - }, - |ctx, vals, (val_double, _val_bits)| { - let (obj_box, key_box) = (vals[0].clone(), vals[1].clone()); - let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - super::property_set::emit_nullish_write_guard( - ctx, - &obj_bits, - "index", - "iset.string", - ); - let static_classref = super::index_get::index_object_is_class_or_proto_ref( - ctx, - object.as_ref(), - ); - let (obj_handle, key_handle) = { - let blk = ctx.block(); - // #7640 section D: the SSO-safe key unbox can - // allocate, so it goes FIRST — `obj_bits` is a - // NaN-boxed double the group re-read, but - // `obj_handle` is a raw `i64` no root can name. - let key_handle = unbox_str_handle(blk, &key_box); - let obj_handle = super::index_get::classref_preserving_handle( - blk, - &obj_bits, - static_classref, - ); - (obj_handle, key_handle) - }; - let site_id = emit_typed_feedback_register_site( - ctx, - TypedFeedbackKind::PropertySet, - "object[string_index]", - TypedFeedbackContract::object_set_by_name(), - ); - ctx.block().call_void( - "js_typed_feedback_object_set_field_by_name", - &[ - (I64, &site_id), - (I64, &obj_handle), - (I64, &key_handle), - (DOUBLE, &val_double), - ], - ); - // One group, one release, below the store. The inner-to-outer - // ordering obligation the two hand-written guards carried — - // `temp_root_truncate` is a stack CUT, so releasing the - // receiver first silently dropped the key's slot as well — is - // gone rather than merely documented. - Ok(val_double) - }, + object, + index, + value, + assignment_strict, + "object[string_index]", ); } // Fallback with runtime STRING_TAG check, matching IndexGet. diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index 8a43d9e808..0470370598 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -42,8 +42,8 @@ use super::{ class_field_store_layout_note_is_conforming, class_field_store_needs_layout_note, class_field_store_needs_string_addref, emit_jsvalue_slot_store_pointer_tested, emit_typed_feedback_register_site, expr_produces_non_pointer_bits_by_construction, lower_expr, - lower_expr_native, raw_f64_layout_fact, try_lower_pod_field_set, unbox_to_i64, FnCtx, - TypedFeedbackContract, TypedFeedbackKind, + lower_expr_native, raw_f64_layout_fact, try_lower_pod_field_set, + typed_feedback_emission_enabled, unbox_to_i64, FnCtx, TypedFeedbackContract, TypedFeedbackKind, }; /// Metadata-only class candidate for the runtime-guarded plain-field store. @@ -146,411 +146,8 @@ pub(crate) fn class_has_computed_runtime_members(ctx: &FnCtx<'_>, class_name: &s /// `js_object_set_field_by_name` — by-name dispatch, a `RuntimeHandleScope`, /// and a per-object side-table touch, for a store whose slot index is a /// compile-time constant. On `deeplist.ts` that one store was the benchmark. -pub(crate) fn try_lower_sloppy_class_field_store( - ctx: &mut FnCtx<'_>, - object: &Expr, - property: &str, - value: &Expr, -) -> Result> { - // Oversized modules full-outline the whole IC diamond into one call - // (#5334 lever B); that outlined runtime has no sloppy variant, so leave - // those modules on the unchanged path. - if crate::codegen::full_outline_ic_enabled() { - return Ok(None); - } - let Some(class_name) = receiver_class_name(ctx, object) - .or_else(|| guarded_declared_class_store_candidate(ctx, object)) - else { - return Ok(None); - }; - if class_has_computed_runtime_members(ctx, &class_name) { - return Ok(None); - } - // A compiled setter owns the name; never store into the slot behind it. - // (`class_field_global_index` also rejects accessors anywhere in the - // chain — this is the same check the strict arm makes first, kept so the - // two arms agree on which shapes are eligible.) - if ctx - .methods - .contains_key(&(class_name.clone(), format!("__set_{}", property))) - { - return Ok(None); - } - let Some(field_index) = - crate::type_analysis::class_field_global_index(ctx, &class_name, property) - else { - return Ok(None); - }; - let (Some(&expected_class_id), Some(keys_global_name)) = ( - ctx.class_ids.get(&class_name), - ctx.class_keys_globals.get(&class_name).cloned(), - ) else { - return Ok(None); - }; - let requires_raw_f64 = - crate::type_analysis::class_field_declared_type(ctx, &class_name, property) - .as_ref() - .is_some_and(crate::typed_shape::type_is_raw_f64_candidate); - if !requires_raw_f64 { - return try_lower_sloppy_class_field_boxed_store( - ctx, - object, - property, - value, - field_index, - expected_class_id, - &keys_global_name, - &class_name, - ); - } - - // Operand order mirrors the strict class-field arm below verbatim: the - // assignment reference is evaluated before the RHS. - // - // #7640 section C: this used to claim the receiver's relocation across an - // allocating RHS was "handled by the same statepoint re-read that arm - // relies on". That mechanism doesn't exist — RS4GC only relocates a value - // that is still `ptr addrspace(1)`-typed and live across the safepoint, - // and `recv_box` crosses it as a plain `double` (a `bitcast`/`ptrtoint` - // chain, dead before the call, per `function/precise_roots.rs`). The - // repair is deliberately split by receiver shape: - // - // * `object` a bare `Expr::LocalGet`/`Expr::This` — its value IS a load - // out of a shadow slot, and `root_reload.rs` (#7280) re-materialises - // that load (plus any pure `bitcast`/`ptrtoint`/`and`/… derived from - // it) below any collection point it doesn't dominate. Unconditional - // on RS4GC — it runs before either root lowering sees the IR, so it - // protects shadow (`PERRY_RS4GC=0`) and native (`=1`, default) - // identically. Verified: `scripts/gc_root_dominance_check.py - // --stale-registers`/`--statepoints`, both lowerings, on - // `test-files/test_gap_gc_class_field_receiver_rooting.ts`'s - // `setRawF64`/`setBoxed`/`setViaSetter` — zero hazards. - // * `object` anything else — e.g. `this.target.x = allocPoint(n).x`, - // where the receiver is itself a class-field READ — cannot use that - // repair: the receiver - // is a `phi` over two field-get paths, not a direct shadow-slot load, - // so `root_reload` has no root to re-derive from, and - // `--stale-registers`' pattern match only anchors on a direct - // `load double, ptr ` source. Confirmed by hand on this exact - // shape (`Holder.setOnThis` in the test above): the field-get result - // register is reused, unreloaded, after `allocPoint`'s call in the - // emitted IR. `with_class_store_operands` closes exactly this residual - // with an explicit operand group, while routing bare locals / `this` - // through the unchanged direct path. Its own collection predicate keeps - // a compound receiver with an inert RHS byte-identical too. - with_class_store_operands(ctx, object, value, |ctx, recv_box, val_double| { - // #7287: inside the fast clone of a #5093 class-field versioned loop, this - // store is covered by the preheader's hoisted shape check — emit the same - // inline plain-finite check + bare slot store the STRICT arm emits (see - // `lower`'s class-field arm), instead of the per-access diamond. - // - // Sound in sloppy mode for the same reason #7423 made the fast arm - // mode-independent: the preheader proved not-frozen, no per-receiver - // descriptors, matching class id and keys token, and an intact typed - // layout, and the loop's body is call-free so none of that can change while - // the clone runs. A store that reaches the raw slot could not have been - // *rejected* in either mode, so there is no sloppy/strict divergence to - // preserve. Everything else — a non-finite or NaN-boxed value — side-exits - // to the slow clone BEFORE storing, and the slow clone re-executes the whole - // iteration through this unchanged sloppy lowering. - if let Expr::LocalGet(recv_id) = object { - if let Some((fact, _)) = crate::expr::class_field_loop_fact_lookup( - &ctx.class_field_loop_facts, - *recv_id, - &class_name, - property, - ) - .filter(|(_, loop_idx)| *loop_idx == field_index) - { - let obj_ptr = fact.obj_ptr.clone(); - let side_exit_label = fact.side_exit_label.clone(); - let store_idx = ctx.new_block("class_field_loop_store.sloppy_fast"); - let store_label = ctx.block_label(store_idx); - { - let blk = ctx.block(); - let val_bits = blk.bitcast_double_to_i64(&val_double); - let finite = - crate::expr::class_field_inline_guard::emit_plain_finite_number_check( - blk, &val_bits, - ); - blk.cond_br(&finite, &store_label, &side_exit_label); - } - ctx.current_block = store_idx; - { - let header_skip = - crate::target_layout::object_header_size_bytes(ctx.target_triple) - .to_string(); - let blk = ctx.block(); - let fields_base = blk.gep(I8, &obj_ptr, &[(I64, &header_skip)]); - let field_ptr = - blk.gep(DOUBLE, &fields_base, &[(I64, &field_index.to_string())]); - // No `js_array_numeric_value_to_raw_f64` canonicalization is - // needed: INT32-boxed and NaN values — the only inputs it - // rewrites — cannot pass the finite check above. - // - // GC_STORE_AUDIT(POINTER_FREE): the finite check proved - // `val_double` is a genuine unboxed double, never a heap - // pointer — no edge, no write barrier. - blk.store(DOUBLE, &val_double, &field_ptr); - } - return Ok(Some(val_double)); - } - } - - let key_idx = ctx.strings.intern(property); - let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); - let field_idx_str = field_index.to_string(); - let expected_class_id_str = expected_class_id.to_string(); - let expected_shape_id = - crate::typed_shape::load_class_shape_id(ctx, &class_name, &keys_global_name); - - let (obj_bits, obj_handle, key_box, val_bits) = { - let blk = ctx.block(); - let obj_bits = blk.bitcast_double_to_i64(&recv_box); - let obj_handle = blk.and(I64, &obj_bits, POINTER_MASK_I64); - let key_box = blk.load(DOUBLE, &key_handle_global); - let val_bits = blk.bitcast_double_to_i64(&val_double); - (obj_bits, obj_handle, key_box, val_bits) - }; - - let fast_idx = ctx.new_block("class_field_sloppy_set.fast"); - let merge_idx = ctx.new_block("class_field_sloppy_set.merge"); - let fast_label = ctx.block_label(fast_idx); - let merge_label = ctx.block_label(merge_idx); - - // Emits the shape/flags/value precheck and branches to `fast_label` on a - // hit; leaves `ctx.current_block` on the freshly created miss block. - let subclass_arms = crate::expr::class_field_inline_guard::class_field_subclass_arms( - ctx, - &class_name, - property, - field_index, - true, - ); - let _miss_label = crate::expr::class_field_inline_guard::emit_class_field_inline_precheck( - ctx, - &obj_bits, - &obj_handle, - &expected_class_id_str, - &expected_shape_id, - true, - Some(&val_bits), - &fast_label, - &subclass_arms, - ); - - // Miss: the strict-aware runtime with `strict = 0`, so a rejected write - // stays a silent no-op exactly as sloppy `PutValue` requires. - { - let blk = ctx.block(); - let _ = blk.call( - DOUBLE, - "js_put_value_set", - &[ - (DOUBLE, &recv_box), - (DOUBLE, &key_box), - (DOUBLE, &val_double), - (DOUBLE, &recv_box), - (I32, "0"), - ], - ); - blk.br(&merge_label); - } - - ctx.current_block = fast_idx; - { - // arm64_32 watchOS: the fields region starts at `size_of::()` - // past the user pointer (16 on LP64 and ILP32 since #8047) — - // same derivation as the strict arm and the runtime setter. - let header_skip = - crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string(); - let blk = ctx.block(); - let obj_ptr = blk.inttoptr(I64, &obj_handle); - let fields_base = blk.gep(I8, &obj_ptr, &[(I64, &header_skip)]); - let field_ptr = blk.gep(DOUBLE, &fields_base, &[(I64, &field_idx_str)]); - // GC_STORE_AUDIT(POINTER_FREE): a guarded raw-f64 class slot holds - // numbers only, and the precheck rejected every value that is not a - // plain finite double, so no write barrier and no layout note are due. - let numeric_value = canonicalize_raw_f64_numeric_store_value(blk, &val_double); - blk.store(DOUBLE, &numeric_value, &field_ptr); - blk.br(&merge_label); - } - - ctx.current_block = merge_idx; - Ok(Some(val_double)) - }) -} - -/// The boxed-slot half of [`try_lower_sloppy_class_field_store`] — P1 (#5094). -/// -/// Same shape as the raw-f64 half: the #5093 inline precheck decides, a hit -/// stores straight into the packed slot, a miss goes to `js_put_value_set(..., -/// strict = 0)` so a rejected sloppy write stays a silent no-op. -/// -/// # Why the precheck alone licenses a guard-free boxed store -/// -/// `emit_class_field_inline_precheck` is a strict subset of the runtime's -/// `class_field_fast_contract`: on a hit, the guard call would have answered -/// "fast" too. For a SET it additionally proves the receiver is not frozen and -/// carries no per-object descriptors, and the process-global latch it reads -/// first is flipped by any prototype-level descriptor or accessor install. Add -/// the `__set_` refusal the caller already made, and every way a -/// `[[Set]]` could be *rejected* or *diverted* is excluded — which is the only -/// thing sloppy and strict `PutValue` disagree about. The value plays no part: -/// unlike the raw-f64 arm, a boxed slot accepts any `JSValue`, so this arm -/// passes `require_raw_f64 = false` and the plain-finite test is not emitted. -/// -/// # GC obligations -/// -/// All three are discharged by [`emit_jsvalue_slot_store_pointer_tested`], with -/// the same value-side predicates the strict guarded arm computes — the write -/// barrier (`expr_produces_non_pointer_bits_by_construction`), the layout note -/// (`class_field_store_needs_layout_note`) and the string demote -/// (`class_field_store_needs_string_addref`). Whatever survives those static -/// proofs is decided by ONE live test of the stored bits (#7511), so a genuine -/// pointer store still reaches the remembered set. Nothing here is keyed on -/// strictness, so this arm's GC behaviour is byte-identical to the strict one. -#[allow(clippy::too_many_arguments)] -fn try_lower_sloppy_class_field_boxed_store( - ctx: &mut FnCtx<'_>, - object: &Expr, - property: &str, - value: &Expr, - field_index: u32, - expected_class_id: u32, - keys_global_name: &str, - class_name: &str, -) -> Result> { - // The direct local/`this` path keeps the existing root-reload repair; the - // compound path gets the explicit operand root the #7640 note above says it - // lacked. - with_class_store_operands(ctx, object, value, |ctx, recv_box, val_double| { - // Computed before the block builder is borrowed below. - let barrier_needed = !expr_produces_non_pointer_bits_by_construction(ctx, value); - let layout_note_needed = class_field_store_needs_layout_note(ctx, value); - let string_addref_needed = class_field_store_needs_string_addref(ctx, value); - - let key_idx = ctx.strings.intern(property); - let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); - let field_idx_str = field_index.to_string(); - let expected_class_id_str = expected_class_id.to_string(); - let expected_shape_id = - crate::typed_shape::load_class_shape_id(ctx, class_name, keys_global_name); - - let (obj_bits, obj_handle, key_box, val_bits) = { - let blk = ctx.block(); - let obj_bits = blk.bitcast_double_to_i64(&recv_box); - let obj_handle = blk.and(I64, &obj_bits, POINTER_MASK_I64); - let key_box = blk.load(DOUBLE, &key_handle_global); - let val_bits = blk.bitcast_double_to_i64(&val_double); - (obj_bits, obj_handle, key_box, val_bits) - }; - - let fast_idx = ctx.new_block("class_field_sloppy_set.boxed_fast"); - let merge_idx = ctx.new_block("class_field_sloppy_set.boxed_merge"); - let fast_label = ctx.block_label(fast_idx); - let merge_label = ctx.block_label(merge_idx); - - // `set_value_bits` is `Some` so the not-frozen check is emitted; - // `require_raw_f64` is false, so the plain-finite value check is not. - let subclass_arms = crate::expr::class_field_inline_guard::class_field_subclass_arms( - ctx, - class_name, - property, - field_index, - false, - ); - let _miss_label = crate::expr::class_field_inline_guard::emit_class_field_inline_precheck( - ctx, - &obj_bits, - &obj_handle, - &expected_class_id_str, - &expected_shape_id, - false, - Some(&val_bits), - &fast_label, - &subclass_arms, - ); - - { - let blk = ctx.block(); - let _ = blk.call( - DOUBLE, - "js_put_value_set", - &[ - (DOUBLE, &recv_box), - (DOUBLE, &key_box), - (DOUBLE, &val_double), - (DOUBLE, &recv_box), - (I32, "0"), - ], - ); - blk.br(&merge_label); - } - - ctx.current_block = fast_idx; - { - // arm64_32 watchOS: the fields region starts at - // `size_of::()` past the user pointer — same derivation - // as every sibling arm and the runtime setter. - let header_skip = - crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string(); - let (field_ptr, field_addr) = { - let blk = ctx.block(); - let obj_ptr = blk.inttoptr(I64, &obj_handle); - let fields_base = blk.gep(I8, &obj_ptr, &[(I64, &header_skip)]); - let field_ptr = blk.gep(DOUBLE, &fields_base, &[(I64, &field_idx_str)]); - let field_addr = blk.ptrtoint(&field_ptr, I64); - (field_ptr, field_addr) - }; - emit_jsvalue_slot_store_pointer_tested( - ctx, - &field_ptr, - &val_double, - &obj_handle, - &field_idx_str, - string_addref_needed, - layout_note_needed, - &obj_bits, - &field_addr, - barrier_needed, - class_field_store_layout_note_is_conforming(ctx, class_name, field_index), - "class_field_set", - ); - ctx.block().br(&merge_label); - } - - ctx.current_block = merge_idx; - let stored = LoweredValue { - semantic: SemanticKind::JsValue, - rep: NativeRep::JsValue, - llvm_ty: DOUBLE, - value: val_double.clone(), - }; - ctx.record_lowered_value_with_access_mode( - "ClassFieldSet", - None, - "class_field_set.sloppy_boxed_store", - &stored, - Some(BoundsState::Guarded { - guard_id: "class_field_inline_precheck".to_string(), - }), - None, - Some(BufferAccessMode::CheckedNative), - None, - false, - false, - vec![ - format!("field={}", property), - format!("field_index={}", field_idx_str), - "receiver_proof=inline_precheck_exact_class".to_string(), - "field_layout_raw_f64=false".to_string(), - "store_guard_failure=js_put_value_set_sloppy".to_string(), - ], - ); - Ok(Some(val_double)) - }) -} +mod sloppy_class_field; +pub(crate) use sloppy_class_field::try_lower_sloppy_class_field_store; fn lower_runtime_property_set_by_name( ctx: &mut FnCtx<'_>, @@ -564,7 +161,7 @@ fn lower_runtime_property_set_by_name( assignment_strict: bool, ) -> Result { if !assignment_strict { - return lower_sloppy_property_set_by_name(ctx, object, property, value); + return lower_put_value_property_set_by_name(ctx, object, property, value, false); } // #7154: root the receiver across the value's evaluation, which allocates. // The group re-reads it as part of emitting the store, so no register of @@ -584,28 +181,46 @@ fn lower_runtime_property_set_by_name( }) } -/// #9459: the SLOPPY terminal store for `Expr::PropertySet`. +/// #9459 / #9495: the terminal by-name store for `Expr::PropertySet`, in BOTH +/// modes. +/// +/// `Set(O, P, V, Throw)` -- ordinary `[[Set]]` WITH the receiver. When the +/// receiver has no own property, ES2024 SS10.1.9.2 (`OrdinarySetWithOwnDescriptor`) +/// walks to the parent and lets the PARENT's descriptor decide: an inherited +/// non-writable data property or getter-only accessor rejects, an inherited +/// setter runs with the original receiver, and only a writable data property +/// (or the end of the chain) creates a new own property on the receiver. The +/// rejection is thrown iff `Throw` (SS6.2.5.7 `PutValue`, +/// `Throw = IsStrictReference`). That is exactly +/// `js_put_value_set(target, key, value, receiver, strict)`, the entry `o.x = v` +/// has always used through `Expr::PutValueSet`; routing every `PropertySet` +/// spelling here -- `o.x += 1`, `for (o.x of it)`, `[o.x] = arr` -- makes them +/// agree with it instead of diverging by lane. /// -/// `Set(O, P, V, false)` -- ordinary `[[Set]]` with the receiver, and a -/// rejection (frozen / sealed / non-writable own or inherited data property / -/// getter-only accessor / non-extensible new key) reported as `false` and -/// discarded rather than thrown. That is exactly `js_put_value_set(target, key, -/// value, receiver, 0)`, the entry sloppy `o.x = v` has always used through -/// `Expr::PutValueSet`; routing here makes `o.x += 1`, `for (o.x of it)` and -/// `[o.x] = arr` agree with it instead of throwing where node is silent. +/// #9459 brought the SLOPPY half here: its old tail rejected by throwing. #9495 +/// brings the STRICT half: its old tail, +/// `js_typed_feedback_object_set_field_by_name_fast` -> +/// `js_object_set_field_by_name`, was an OWN-property store with no prototype +/// walk, so a strict `o.x += 1` against an inherited setter never ran the setter, +/// an inherited non-writable / getter-only property never threw, and an own +/// property materialised where the spec creates none. +/// +/// The typed-feedback `PropertySet` site moves with the store +/// (`emit_typed_feedback_property_set_observation`): it describes the store, +/// not the store's strictness, so both modes register it. /// /// `target` and `receiver` are the same expression, evaluated ONCE -- the -/// property reference's base is one evaluation, and `with_operands_rooted` +/// property reference's base is one evaluation, and `with_operands_rooted_across` /// hands the single lowered box to both operand slots. /// -/// Rooting is the #7154 window the strict tail also opens: the receiver is live -/// across `value`'s lowering, which is arbitrary user code and can drive an -/// evacuating minor. -fn lower_sloppy_property_set_by_name( +/// Rooting is the #7154 window: the receiver is live across `value`'s +/// lowering, which is arbitrary user code and can drive an evacuating minor. +fn lower_put_value_property_set_by_name( ctx: &mut FnCtx<'_>, object: &Expr, property: &str, value: &Expr, + assignment_strict: bool, ) -> Result { rooting::with_operands_rooted_across( ctx, @@ -615,8 +230,8 @@ fn lower_sloppy_property_set_by_name( lower_value_for_dynamic_property_set( ctx, value, - "property_set.sloppy_dynamic_value_bits", - "sloppy_property_set_helper_edge", + "property_set.dynamic_value_bits", + "dynamic_property_set_helper_edge", ) }, |ctx, vals, (val_double, _val_bits)| { @@ -624,11 +239,17 @@ fn lower_sloppy_property_set_by_name( let key_idx = ctx.strings.intern(property); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - // The nullish receiver check is the same one the strict tail emits: - // `undefined.x = 1` is a TypeError in BOTH modes (GetValue on the - // base runs before PutValue's Throw flag is ever consulted). - emit_nullish_write_guard(ctx, &obj_bits, property, "pset_sloppy"); + // `undefined.x = 1` is a TypeError in BOTH modes: `GetValue` on the + // base runs before `PutValue` ever consults `Throw`. + emit_nullish_write_guard(ctx, &obj_bits, property, "pset"); let key_box = ctx.block().load(DOUBLE, &key_handle_global); + emit_typed_feedback_property_set_observation(ctx, property, &obj_bits, |ctx| { + // The key is an interned heap string, so its `StringHeader*` is + // a mask, never an allocation. + let key_bits = ctx.block().bitcast_double_to_i64(&key_box); + ctx.block().and(I64, &key_bits, POINTER_MASK_I64) + }); + let strict_flag = if assignment_strict { "1" } else { "0" }; let _ = ctx.block().call( DOUBLE, "js_put_value_set", @@ -637,7 +258,7 @@ fn lower_sloppy_property_set_by_name( (DOUBLE, &key_box), (DOUBLE, &val_double), (DOUBLE, &obj_box), - (I32, "0"), + (I32, strict_flag), ], ); Ok(val_double) @@ -645,6 +266,50 @@ fn lower_sloppy_property_set_by_name( ) } +/// #9495: the typed-feedback `PropertySet` observation for a by-name store +/// whose tail is `js_put_value_set`. +/// +/// The old strict tail folded the observation into a DISPATCHING wrapper +/// (`js_typed_feedback_object_set_field_by_name_fast`), emitted in every build +/// because it also chose between the shape-transition fast path and the +/// by-name setter. The receiver-aware `[[Set]]` makes that choice inside +/// `js_put_value_set`, so nothing is left for a wrapper to decide and the +/// observation stands alone -- a pure-recording helper, compile-gated on +/// `PERRY_TYPED_FEEDBACK` / `_TRACE` like every other one since #7480 step 4. A +/// default build emits the bare `js_put_value_set` call and nothing else. +/// +/// The site is always REGISTERED (a no-op call-free counter bump in a default +/// build) so that `ic_site_counter` -- and with it every later inline-cache +/// global name in the function -- is numbered exactly as it was when the +/// dispatching wrapper stood here. +/// +/// `key_raw` derives the bare `StringHeader*` the observer hashes, and is only +/// run in an emitting build. It runs BEFORE the observe call and after nothing +/// else, so a derivation that can allocate (an SSO key materialising onto the +/// heap -- #7640 section D) precedes no raw receiver handle: `obj_bits` is the +/// NaN-boxed receiver the rooting group re-read. +pub(super) fn emit_typed_feedback_property_set_observation( + ctx: &mut FnCtx<'_>, + operation: &str, + obj_bits: &str, + key_raw: impl FnOnce(&mut FnCtx<'_>) -> String, +) { + let site_id = emit_typed_feedback_register_site( + ctx, + TypedFeedbackKind::PropertySet, + operation, + TypedFeedbackContract::put_value_set(), + ); + if !typed_feedback_emission_enabled() { + return; + } + let key_raw = key_raw(ctx); + ctx.block().call_void( + "js_typed_feedback_observe_property_set", + &[(I64, &site_id), (I64, obj_bits), (I64, &key_raw)], + ); +} + fn lower_value_for_dynamic_property_set( ctx: &mut FnCtx<'_>, value: &Expr, @@ -719,6 +384,10 @@ pub(crate) fn emit_nullish_write_guard( /// every arm below whose runtime entry rejects by THROWING is strict-only; the /// sloppy twin of each is the strictness-aware `js_put_value_set(..., 0)` that /// the surrounding `PutValueSet` lowering already uses for `o.x = v`. +/// +/// #9495: the generic by-name tail is that same receiver-aware `[[Set]]` in BOTH +/// modes (`lower_put_value_property_set_by_name`). The strict tail used to be an +/// own-property store that skipped the prototype walk; see the helper. pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) -> Result { match expr { Expr::PropertySet { @@ -1129,7 +798,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - return Ok(result); } } - return lower_sloppy_property_set_by_name(ctx, object, property, value); + return lower_put_value_property_set_by_name( + ctx, object, property, value, false, + ); } // Fast path: known class instance + plain instance field. // The runtime guard checks the receiver's class/shape and @@ -1852,18 +1523,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - } } } - // #9459: the generic SLOPPY tail. The strict tail below ends in - // `js_typed_feedback_object_set_field_by_name_fast`, whose underlying - // `js_object_set_field_by_name` has no `strict` parameter and rejects - // by throwing; sloppy `PutValue` must discard the rejection instead. + // #9459 / #9495: the generic by-name tail is the receiver-aware + // `[[Set]]` with the assignment's own `Throw` flag, in BOTH modes. // // `caller`/`arguments` are excluded in BOTH modes: those two names are // routed here from `PutValueSet` specifically to reach // `js_object_set_field_by_name`'s poisoned-accessor handling, which is - // not a `Throw`-flag decision, and diverting them would change - // behaviour this issue is not about. - if !assignment_strict && !matches!(property.as_str(), "caller" | "arguments") { - return lower_sloppy_property_set_by_name(ctx, object, property, value); + // neither a `Throw`-flag nor a prototype-walk decision, and diverting + // them would change behaviour neither issue is about. + if !matches!(property.as_str(), "caller" | "arguments") { + return lower_put_value_property_set_by_name( + ctx, + object, + property, + value, + assignment_strict, + ); } // #7154: the value expression can collect, and an evacuating minor // inside it relocates the receiver out from under `obj_box` -- @@ -1906,27 +1581,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - let key_box = ctx.block().load(DOUBLE, &key_handle_global); let key_bits = ctx.block().bitcast_double_to_i64(&key_box); let key_raw = ctx.block().and(I64, &key_bits, POINTER_MASK_I64); - if matches!(property.as_str(), "caller" | "arguments") { - ctx.block().call_void( - "js_object_set_field_by_name", - &[(I64, &obj_bits), (I64, &key_raw), (DOUBLE, &val_double)], - ); - return Ok(val_double); - } - let site_id = emit_typed_feedback_register_site( - ctx, - TypedFeedbackKind::PropertySet, - property, - TypedFeedbackContract::object_set_by_name(), - ); ctx.block().call_void( - "js_typed_feedback_object_set_field_by_name_fast", - &[ - (I64, &site_id), - (I64, &obj_bits), - (I64, &key_raw), - (DOUBLE, &val_double), - ], + "js_object_set_field_by_name", + &[(I64, &obj_bits), (I64, &key_raw), (DOUBLE, &val_double)], ); Ok(val_double) }, diff --git a/crates/perry-codegen/src/expr/property_set/sloppy_class_field.rs b/crates/perry-codegen/src/expr/property_set/sloppy_class_field.rs new file mode 100644 index 0000000000..f942638e3f --- /dev/null +++ b/crates/perry-codegen/src/expr/property_set/sloppy_class_field.rs @@ -0,0 +1,410 @@ +//! The sloppy-mode class-field store fast paths, split from +//! `property_set.rs` to keep it under the 2000-line file cap. + +use super::*; + +pub(crate) fn try_lower_sloppy_class_field_store( + ctx: &mut FnCtx<'_>, + object: &Expr, + property: &str, + value: &Expr, +) -> Result> { + // Oversized modules full-outline the whole IC diamond into one call + // (#5334 lever B); that outlined runtime has no sloppy variant, so leave + // those modules on the unchanged path. + if crate::codegen::full_outline_ic_enabled() { + return Ok(None); + } + let Some(class_name) = receiver_class_name(ctx, object) + .or_else(|| guarded_declared_class_store_candidate(ctx, object)) + else { + return Ok(None); + }; + if class_has_computed_runtime_members(ctx, &class_name) { + return Ok(None); + } + // A compiled setter owns the name; never store into the slot behind it. + // (`class_field_global_index` also rejects accessors anywhere in the + // chain — this is the same check the strict arm makes first, kept so the + // two arms agree on which shapes are eligible.) + if ctx + .methods + .contains_key(&(class_name.clone(), format!("__set_{}", property))) + { + return Ok(None); + } + let Some(field_index) = + crate::type_analysis::class_field_global_index(ctx, &class_name, property) + else { + return Ok(None); + }; + let (Some(&expected_class_id), Some(keys_global_name)) = ( + ctx.class_ids.get(&class_name), + ctx.class_keys_globals.get(&class_name).cloned(), + ) else { + return Ok(None); + }; + let requires_raw_f64 = + crate::type_analysis::class_field_declared_type(ctx, &class_name, property) + .as_ref() + .is_some_and(crate::typed_shape::type_is_raw_f64_candidate); + if !requires_raw_f64 { + return try_lower_sloppy_class_field_boxed_store( + ctx, + object, + property, + value, + field_index, + expected_class_id, + &keys_global_name, + &class_name, + ); + } + + // Operand order mirrors the strict class-field arm below verbatim: the + // assignment reference is evaluated before the RHS. + // + // #7640 section C: this used to claim the receiver's relocation across an + // allocating RHS was "handled by the same statepoint re-read that arm + // relies on". That mechanism doesn't exist — RS4GC only relocates a value + // that is still `ptr addrspace(1)`-typed and live across the safepoint, + // and `recv_box` crosses it as a plain `double` (a `bitcast`/`ptrtoint` + // chain, dead before the call, per `function/precise_roots.rs`). The + // repair is deliberately split by receiver shape: + // + // * `object` a bare `Expr::LocalGet`/`Expr::This` — its value IS a load + // out of a shadow slot, and `root_reload.rs` (#7280) re-materialises + // that load (plus any pure `bitcast`/`ptrtoint`/`and`/… derived from + // it) below any collection point it doesn't dominate. Unconditional + // on RS4GC — it runs before either root lowering sees the IR, so it + // protects shadow (`PERRY_RS4GC=0`) and native (`=1`, default) + // identically. Verified: `scripts/gc_root_dominance_check.py + // --stale-registers`/`--statepoints`, both lowerings, on + // `test-files/test_gap_gc_class_field_receiver_rooting.ts`'s + // `setRawF64`/`setBoxed`/`setViaSetter` — zero hazards. + // * `object` anything else — e.g. `this.target.x = allocPoint(n).x`, + // where the receiver is itself a class-field READ — cannot use that + // repair: the receiver + // is a `phi` over two field-get paths, not a direct shadow-slot load, + // so `root_reload` has no root to re-derive from, and + // `--stale-registers`' pattern match only anchors on a direct + // `load double, ptr ` source. Confirmed by hand on this exact + // shape (`Holder.setOnThis` in the test above): the field-get result + // register is reused, unreloaded, after `allocPoint`'s call in the + // emitted IR. `with_class_store_operands` closes exactly this residual + // with an explicit operand group, while routing bare locals / `this` + // through the unchanged direct path. Its own collection predicate keeps + // a compound receiver with an inert RHS byte-identical too. + with_class_store_operands(ctx, object, value, |ctx, recv_box, val_double| { + // #7287: inside the fast clone of a #5093 class-field versioned loop, this + // store is covered by the preheader's hoisted shape check — emit the same + // inline plain-finite check + bare slot store the STRICT arm emits (see + // `lower`'s class-field arm), instead of the per-access diamond. + // + // Sound in sloppy mode for the same reason #7423 made the fast arm + // mode-independent: the preheader proved not-frozen, no per-receiver + // descriptors, matching class id and keys token, and an intact typed + // layout, and the loop's body is call-free so none of that can change while + // the clone runs. A store that reaches the raw slot could not have been + // *rejected* in either mode, so there is no sloppy/strict divergence to + // preserve. Everything else — a non-finite or NaN-boxed value — side-exits + // to the slow clone BEFORE storing, and the slow clone re-executes the whole + // iteration through this unchanged sloppy lowering. + if let Expr::LocalGet(recv_id) = object { + if let Some((fact, _)) = crate::expr::class_field_loop_fact_lookup( + &ctx.class_field_loop_facts, + *recv_id, + &class_name, + property, + ) + .filter(|(_, loop_idx)| *loop_idx == field_index) + { + let obj_ptr = fact.obj_ptr.clone(); + let side_exit_label = fact.side_exit_label.clone(); + let store_idx = ctx.new_block("class_field_loop_store.sloppy_fast"); + let store_label = ctx.block_label(store_idx); + { + let blk = ctx.block(); + let val_bits = blk.bitcast_double_to_i64(&val_double); + let finite = + crate::expr::class_field_inline_guard::emit_plain_finite_number_check( + blk, &val_bits, + ); + blk.cond_br(&finite, &store_label, &side_exit_label); + } + ctx.current_block = store_idx; + { + let header_skip = + crate::target_layout::object_header_size_bytes(ctx.target_triple) + .to_string(); + let blk = ctx.block(); + let fields_base = blk.gep(I8, &obj_ptr, &[(I64, &header_skip)]); + let field_ptr = + blk.gep(DOUBLE, &fields_base, &[(I64, &field_index.to_string())]); + // No `js_array_numeric_value_to_raw_f64` canonicalization is + // needed: INT32-boxed and NaN values — the only inputs it + // rewrites — cannot pass the finite check above. + // + // GC_STORE_AUDIT(POINTER_FREE): the finite check proved + // `val_double` is a genuine unboxed double, never a heap + // pointer — no edge, no write barrier. + blk.store(DOUBLE, &val_double, &field_ptr); + } + return Ok(Some(val_double)); + } + } + + let key_idx = ctx.strings.intern(property); + let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); + let field_idx_str = field_index.to_string(); + let expected_class_id_str = expected_class_id.to_string(); + let expected_shape_id = + crate::typed_shape::load_class_shape_id(ctx, &class_name, &keys_global_name); + + let (obj_bits, obj_handle, key_box, val_bits) = { + let blk = ctx.block(); + let obj_bits = blk.bitcast_double_to_i64(&recv_box); + let obj_handle = blk.and(I64, &obj_bits, POINTER_MASK_I64); + let key_box = blk.load(DOUBLE, &key_handle_global); + let val_bits = blk.bitcast_double_to_i64(&val_double); + (obj_bits, obj_handle, key_box, val_bits) + }; + + let fast_idx = ctx.new_block("class_field_sloppy_set.fast"); + let merge_idx = ctx.new_block("class_field_sloppy_set.merge"); + let fast_label = ctx.block_label(fast_idx); + let merge_label = ctx.block_label(merge_idx); + + // Emits the shape/flags/value precheck and branches to `fast_label` on a + // hit; leaves `ctx.current_block` on the freshly created miss block. + let subclass_arms = crate::expr::class_field_inline_guard::class_field_subclass_arms( + ctx, + &class_name, + property, + field_index, + true, + ); + let _miss_label = crate::expr::class_field_inline_guard::emit_class_field_inline_precheck( + ctx, + &obj_bits, + &obj_handle, + &expected_class_id_str, + &expected_shape_id, + true, + Some(&val_bits), + &fast_label, + &subclass_arms, + ); + + // Miss: the strict-aware runtime with `strict = 0`, so a rejected write + // stays a silent no-op exactly as sloppy `PutValue` requires. + { + let blk = ctx.block(); + let _ = blk.call( + DOUBLE, + "js_put_value_set", + &[ + (DOUBLE, &recv_box), + (DOUBLE, &key_box), + (DOUBLE, &val_double), + (DOUBLE, &recv_box), + (I32, "0"), + ], + ); + blk.br(&merge_label); + } + + ctx.current_block = fast_idx; + { + // arm64_32 watchOS: the fields region starts at `size_of::()` + // past the user pointer (16 on LP64 and ILP32 since #8047) — + // same derivation as the strict arm and the runtime setter. + let header_skip = + crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string(); + let blk = ctx.block(); + let obj_ptr = blk.inttoptr(I64, &obj_handle); + let fields_base = blk.gep(I8, &obj_ptr, &[(I64, &header_skip)]); + let field_ptr = blk.gep(DOUBLE, &fields_base, &[(I64, &field_idx_str)]); + // GC_STORE_AUDIT(POINTER_FREE): a guarded raw-f64 class slot holds + // numbers only, and the precheck rejected every value that is not a + // plain finite double, so no write barrier and no layout note are due. + let numeric_value = canonicalize_raw_f64_numeric_store_value(blk, &val_double); + blk.store(DOUBLE, &numeric_value, &field_ptr); + blk.br(&merge_label); + } + + ctx.current_block = merge_idx; + Ok(Some(val_double)) + }) +} + +/// The boxed-slot half of [`try_lower_sloppy_class_field_store`] — P1 (#5094). +/// +/// Same shape as the raw-f64 half: the #5093 inline precheck decides, a hit +/// stores straight into the packed slot, a miss goes to `js_put_value_set(..., +/// strict = 0)` so a rejected sloppy write stays a silent no-op. +/// +/// # Why the precheck alone licenses a guard-free boxed store +/// +/// `emit_class_field_inline_precheck` is a strict subset of the runtime's +/// `class_field_fast_contract`: on a hit, the guard call would have answered +/// "fast" too. For a SET it additionally proves the receiver is not frozen and +/// carries no per-object descriptors, and the process-global latch it reads +/// first is flipped by any prototype-level descriptor or accessor install. Add +/// the `__set_` refusal the caller already made, and every way a +/// `[[Set]]` could be *rejected* or *diverted* is excluded — which is the only +/// thing sloppy and strict `PutValue` disagree about. The value plays no part: +/// unlike the raw-f64 arm, a boxed slot accepts any `JSValue`, so this arm +/// passes `require_raw_f64 = false` and the plain-finite test is not emitted. +/// +/// # GC obligations +/// +/// All three are discharged by [`emit_jsvalue_slot_store_pointer_tested`], with +/// the same value-side predicates the strict guarded arm computes — the write +/// barrier (`expr_produces_non_pointer_bits_by_construction`), the layout note +/// (`class_field_store_needs_layout_note`) and the string demote +/// (`class_field_store_needs_string_addref`). Whatever survives those static +/// proofs is decided by ONE live test of the stored bits (#7511), so a genuine +/// pointer store still reaches the remembered set. Nothing here is keyed on +/// strictness, so this arm's GC behaviour is byte-identical to the strict one. +#[allow(clippy::too_many_arguments)] +fn try_lower_sloppy_class_field_boxed_store( + ctx: &mut FnCtx<'_>, + object: &Expr, + property: &str, + value: &Expr, + field_index: u32, + expected_class_id: u32, + keys_global_name: &str, + class_name: &str, +) -> Result> { + // The direct local/`this` path keeps the existing root-reload repair; the + // compound path gets the explicit operand root the #7640 note above says it + // lacked. + with_class_store_operands(ctx, object, value, |ctx, recv_box, val_double| { + // Computed before the block builder is borrowed below. + let barrier_needed = !expr_produces_non_pointer_bits_by_construction(ctx, value); + let layout_note_needed = class_field_store_needs_layout_note(ctx, value); + let string_addref_needed = class_field_store_needs_string_addref(ctx, value); + + let key_idx = ctx.strings.intern(property); + let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); + let field_idx_str = field_index.to_string(); + let expected_class_id_str = expected_class_id.to_string(); + let expected_shape_id = + crate::typed_shape::load_class_shape_id(ctx, class_name, keys_global_name); + + let (obj_bits, obj_handle, key_box, val_bits) = { + let blk = ctx.block(); + let obj_bits = blk.bitcast_double_to_i64(&recv_box); + let obj_handle = blk.and(I64, &obj_bits, POINTER_MASK_I64); + let key_box = blk.load(DOUBLE, &key_handle_global); + let val_bits = blk.bitcast_double_to_i64(&val_double); + (obj_bits, obj_handle, key_box, val_bits) + }; + + let fast_idx = ctx.new_block("class_field_sloppy_set.boxed_fast"); + let merge_idx = ctx.new_block("class_field_sloppy_set.boxed_merge"); + let fast_label = ctx.block_label(fast_idx); + let merge_label = ctx.block_label(merge_idx); + + // `set_value_bits` is `Some` so the not-frozen check is emitted; + // `require_raw_f64` is false, so the plain-finite value check is not. + let subclass_arms = crate::expr::class_field_inline_guard::class_field_subclass_arms( + ctx, + class_name, + property, + field_index, + false, + ); + let _miss_label = crate::expr::class_field_inline_guard::emit_class_field_inline_precheck( + ctx, + &obj_bits, + &obj_handle, + &expected_class_id_str, + &expected_shape_id, + false, + Some(&val_bits), + &fast_label, + &subclass_arms, + ); + + { + let blk = ctx.block(); + let _ = blk.call( + DOUBLE, + "js_put_value_set", + &[ + (DOUBLE, &recv_box), + (DOUBLE, &key_box), + (DOUBLE, &val_double), + (DOUBLE, &recv_box), + (I32, "0"), + ], + ); + blk.br(&merge_label); + } + + ctx.current_block = fast_idx; + { + // arm64_32 watchOS: the fields region starts at + // `size_of::()` past the user pointer — same derivation + // as every sibling arm and the runtime setter. + let header_skip = + crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string(); + let (field_ptr, field_addr) = { + let blk = ctx.block(); + let obj_ptr = blk.inttoptr(I64, &obj_handle); + let fields_base = blk.gep(I8, &obj_ptr, &[(I64, &header_skip)]); + let field_ptr = blk.gep(DOUBLE, &fields_base, &[(I64, &field_idx_str)]); + let field_addr = blk.ptrtoint(&field_ptr, I64); + (field_ptr, field_addr) + }; + emit_jsvalue_slot_store_pointer_tested( + ctx, + &field_ptr, + &val_double, + &obj_handle, + &field_idx_str, + string_addref_needed, + layout_note_needed, + &obj_bits, + &field_addr, + barrier_needed, + class_field_store_layout_note_is_conforming(ctx, class_name, field_index), + "class_field_set", + ); + ctx.block().br(&merge_label); + } + + ctx.current_block = merge_idx; + let stored = LoweredValue { + semantic: SemanticKind::JsValue, + rep: NativeRep::JsValue, + llvm_ty: DOUBLE, + value: val_double.clone(), + }; + ctx.record_lowered_value_with_access_mode( + "ClassFieldSet", + None, + "class_field_set.sloppy_boxed_store", + &stored, + Some(BoundsState::Guarded { + guard_id: "class_field_inline_precheck".to_string(), + }), + None, + Some(BufferAccessMode::CheckedNative), + None, + false, + false, + vec![ + format!("field={}", property), + format!("field_index={}", field_idx_str), + "receiver_proof=inline_precheck_exact_class".to_string(), + "field_layout_raw_f64=false".to_string(), + "store_guard_failure=js_put_value_set_sloppy".to_string(), + ], + ); + Ok(Some(val_double)) + }) +} diff --git a/crates/perry-codegen/src/expr/this_super_call.rs b/crates/perry-codegen/src/expr/this_super_call.rs index 724cd2a0ef..852a9d6ada 100644 --- a/crates/perry-codegen/src/expr/this_super_call.rs +++ b/crates/perry-codegen/src/expr/this_super_call.rs @@ -601,8 +601,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // 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 @@ -1169,8 +1169,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } // 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!( @@ -1274,6 +1274,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { 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); @@ -1295,58 +1306,27 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)], ); } - // this.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); @@ -1591,11 +1571,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } 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 @@ -1633,6 +1613,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { 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); @@ -1650,25 +1638,6 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(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 diff --git a/crates/perry-codegen/src/expr/typed_feedback.rs b/crates/perry-codegen/src/expr/typed_feedback.rs index 01e72cc09e..78aff0e2c6 100644 --- a/crates/perry-codegen/src/expr/typed_feedback.rs +++ b/crates/perry-codegen/src/expr/typed_feedback.rs @@ -42,6 +42,14 @@ impl TypedFeedbackContract { Self::new("object_set_by_name_guard", "js_object_set_field_by_name") } + /// #9495: the by-name store whose tail is the receiver-aware `[[Set]]` + /// (`js_put_value_set`) rather than the own-property setter. Same guard + /// label as `object_set_by_name` -- it is the same site kind, observed the + /// same way; only the entry that performs the store differs. + pub(crate) const fn put_value_set() -> Self { + Self::new("object_set_by_name_guard", "js_put_value_set") + } + pub(crate) const fn class_field_get() -> Self { Self::new("class_field_get_guard", "js_object_get_field_by_name_f64") } diff --git a/crates/perry-codegen/src/lower_call/new_error_init.rs b/crates/perry-codegen/src/lower_call/new_error_init.rs index 86fd0d208b..7f529c8e3a 100644 --- a/crates/perry-codegen/src/lower_call/new_error_init.rs +++ b/crates/perry-codegen/src/lower_call/new_error_init.rs @@ -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. @@ -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); @@ -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 diff --git a/crates/perry-codegen/src/lower_call/property_get/map_set.rs b/crates/perry-codegen/src/lower_call/property_get/map_set.rs index 68076e8e34..d3304ca3e1 100644 --- a/crates/perry-codegen/src/lower_call/property_get/map_set.rs +++ b/crates/perry-codegen/src/lower_call/property_get/map_set.rs @@ -26,7 +26,7 @@ use anyhow::Result; use perry_hir::Expr; -use crate::expr::{lower_expr, unbox_to_i64, FnCtx}; +use crate::expr::{lower_expr, nanbox_pointer_inline, unbox_to_i64, FnCtx}; use crate::nanbox::double_literal; use crate::rooting; use crate::type_analysis::{ @@ -88,11 +88,23 @@ pub(crate) fn try_lower_map_set_methods( (vals[0].clone(), vals[1].clone(), vals[2].clone()); let blk = ctx.block(); let m_handle = unbox_to_i64(blk, &m_box); - blk.call_void( + // #9523: `js_map_set` returns the RECEIVER as it stands + // after the insert. For a `class X extends Map` instance + // that receiver is a movable `ObjectHeader` the runtime + // roots across the grow (`map_op_returning_receiver`), so + // a moving minor inside `ensure_capacity` hands back a + // different address — and `m_box`, read from its slot + // BEFORE the call, is then a from-space pointer. The + // chained `.set(a, 1).set(b, 2)` consumes exactly that + // return value. `Expr::MapSet` already re-boxes the + // returned pointer; this arm called the helper as `void` + // and returned the pre-call box. + let receiver = blk.call( + I64, "js_map_set", &[(I64, &m_handle), (DOUBLE, &k_box), (DOUBLE, &v_box)], ); - Ok(Some(m_box)) + Ok(Some(nanbox_pointer_inline(blk, &receiver))) }, ); } diff --git a/crates/perry-codegen/src/rooting/mod.rs b/crates/perry-codegen/src/rooting/mod.rs index 1a16070e08..e791b8215b 100644 --- a/crates/perry-codegen/src/rooting/mod.rs +++ b/crates/perry-codegen/src/rooting/mod.rs @@ -1670,6 +1670,10 @@ const MIGRATED_MODULES: &[(&str, &str)] = &[ "crates/perry-codegen/src/expr/logical_collections.rs", include_str!("../expr/logical_collections.rs"), ), + ( + "crates/perry-codegen/src/expr/bigint_set.rs", + include_str!("../expr/bigint_set.rs"), + ), ( "crates/perry-codegen/src/lower_call/property_get/map_set.rs", include_str!("../lower_call/property_get/map_set.rs"), diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index e60394b4c0..03a72c3386 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -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, diff --git a/crates/perry-codegen/src/temp_root_coverage/mod.rs b/crates/perry-codegen/src/temp_root_coverage/mod.rs index 502a6f693b..d46ad2ce0a 100644 --- a/crates/perry-codegen/src/temp_root_coverage/mod.rs +++ b/crates/perry-codegen/src/temp_root_coverage/mod.rs @@ -47,6 +47,7 @@ mod builtin_ctor; mod call_callee; mod dispatch_receiver; mod operands; +mod set_receiver; pub(crate) fn entry_opts() -> CompileOptions { CompileOptions { diff --git a/crates/perry-codegen/src/temp_root_coverage/set_receiver.rs b/crates/perry-codegen/src/temp_root_coverage/set_receiver.rs new file mode 100644 index 0000000000..e8b6ae2560 --- /dev/null +++ b/crates/perry-codegen/src/temp_root_coverage/set_receiver.rs @@ -0,0 +1,204 @@ +//! #9523: the `Expr::SetHas` / `Expr::SetDelete` RECEIVER is a rooted +//! temporary, not an SSA register. +//! +//! `expr/bigint_set.rs` lowered the receiver, unboxed it to a raw `i64` handle, +//! THEN lowered the value expression — arbitrary user code that can allocate — +//! and consumed the handle after. That is the #6970 shape `MapGet` / `MapHas` +//! / `MapDelete` were fixed for (`math_simple.rs`, `logical_collections.rs`), +//! found live in these two twins by #9522's audit. An evacuating minor inside +//! the value's lowering moves the Set; the handle keeps the pre-move address, +//! and `js_set_has` reads a from-space header. `test-files/test_gap_9523_set_ +//! receiver_roots_across_value.ts` is the end-to-end half. +//! +//! # Non-vacuity +//! +//! The positive assertions name the VALUE — the register `js_set_alloc` +//! produced went into a rooted slot, and the consuming helper read its operand +//! back OUT of that slot — so a compiler that roots nothing cannot satisfy +//! them by emitting nothing. The receiver is deliberately a fresh `SetNew` +//! rather than a local read: a load out of a shadow slot is a re-readable +//! location that `root_reload` already re-derives below the collection point, +//! so a `LocalGet` receiver would pass against the unfixed compiler. (The +//! typed-arm test below uses a local on purpose and pins the *temp* slot the +//! fix adds, which `root_reload` never emits.) +//! +//! The negative controls hold the other side: a value that cannot collect +//! leaves no window, so the lowering must stay on its pre-#9523 IR with no +//! temp slot at all. +//! +//! Sabotage: reverting `bigint_set.rs`'s two arms to the eager +//! `unbox_to_i64(lower_expr(set))` fails the positive tests with "is never +//! stored into a rooted slot — it lives its whole life in an SSA register" and +//! the typed-arm test with a temp-slot count of 0. + +use super::{allocating, main_ir_for, under_both_lowerings}; +use crate::testing::temp_slots::{ + assert_no_temp_rooting, assert_rooted_across, first_call_result, temp_root_slots, +}; +use perry_hir::types::Type; +use perry_hir::{Expr, Stmt}; + +const STRING_SET_LOCAL: u32 = 910; + +fn set_has(set: Expr, value: Expr) -> Stmt { + Stmt::Expr(Expr::SetHas { + set: Box::new(set), + value: Box::new(value), + }) +} + +fn set_delete(set: Expr, value: Expr) -> Stmt { + Stmt::Expr(Expr::SetDelete { + set: Box::new(set), + value: Box::new(value), + }) +} + +/// `const s: Set = new Set()` — the receiver shape the frontend +/// produces (`Expr::SetHas { set: LocalGet(..) }`), typed so the string arm +/// (`js_set_has_string`) is selected. +fn string_set_local() -> Stmt { + Stmt::Let { + id: STRING_SET_LOCAL, + name: "s".to_string(), + ty: Type::Generic { + base: "Set".to_string(), + type_args: vec![Type::String], + }, + mutable: false, + init: Some(Expr::SetNew), + } +} + +/// `String({})` — runtime-guaranteed to be a string, so the typed string arm +/// is selected, and a collection point: `ToPrimitive` on an object can run user +/// code, and the object literal itself allocates. +fn allocating_string() -> Expr { + Expr::StringCoerce(Box::new(allocating())) +} + +/// THE GAP (#9523), `has`: the receiver is produced before the value and +/// consumed after it, so it must live in a rooted slot and `js_set_has` must +/// read it back out of that slot. +#[test] +fn a_set_has_receiver_is_rooted_across_an_allocating_value() { + under_both_lowerings(|lowering| { + let ir = main_ir_for( + "set_has_receiver_rooted.ts", + vec![set_has(Expr::SetNew, allocating())], + ); + let set = first_call_result(&ir, "js_set_alloc").unwrap_or_else(|| { + panic!( + "{lowering}: no call to `js_set_alloc` in `main` — this test has no subject:\n{ir}" + ) + }); + assert_rooted_across( + &ir, + &set, + "js_set_has", + &format!( + "{lowering}: #9523 — the Set receiver is live across the value, which allocates" + ), + ); + }); +} + +/// THE GAP (#9523), `delete`: same window, same contract, the other twin. +#[test] +fn a_set_delete_receiver_is_rooted_across_an_allocating_value() { + under_both_lowerings(|lowering| { + let ir = main_ir_for( + "set_delete_receiver_rooted.ts", + vec![set_delete(Expr::SetNew, allocating())], + ); + let set = first_call_result(&ir, "js_set_alloc").unwrap_or_else(|| { + panic!( + "{lowering}: no call to `js_set_alloc` in `main` — this test has no subject:\n{ir}" + ) + }); + assert_rooted_across( + &ir, + &set, + "js_set_delete", + &format!( + "{lowering}: #9523 — the Set receiver is live across the value, which allocates" + ), + ); + }); +} + +/// The control on the other side: a value that cannot collect leaves no +/// window, so the receiver must not pay a temp slot and the IR is exactly what +/// it was before #9523. Without this half, a lowering that roots every receiver +/// unconditionally would pass the tests above and pay for it on every `has`. +#[test] +fn a_set_has_with_a_non_allocating_value_pays_no_temp_slot() { + under_both_lowerings(|lowering| { + for (name, stmt) in [ + ("set_has_no_gc.ts", set_has(Expr::SetNew, Expr::Integer(7))), + ( + "set_delete_no_gc.ts", + set_delete(Expr::SetNew, Expr::Integer(7)), + ), + ] { + let ir = main_ir_for(name, vec![stmt]); + assert!( + ir.contains("@js_set_has(") || ir.contains("@js_set_delete("), + "{lowering}: {name} must reach the generic Set helper, or this proves nothing:\n{ir}" + ); + assert_no_temp_rooting( + &ir, + &format!("{lowering}: {name} — #9523 gate: nothing after the receiver can collect"), + ); + } + }); +} + +/// The typed arms take the same window. A `Set` local with a +/// string-guaranteed, allocating value selects `js_set_has_string`, and the +/// receiver — a plain local read — must be pushed into a TEMP slot for the +/// value's duration. `root_reload` would re-derive the shadow-slot load on its +/// own, but it never emits a temp slot, so the count below is the fix's own +/// signature: exactly one temp slot with the value, none without it. +#[test] +fn a_typed_string_set_receiver_is_temp_rooted_only_when_the_value_collects() { + under_both_lowerings(|lowering| { + let rooted = main_ir_for( + "set_has_string_arm_rooted.ts", + vec![ + string_set_local(), + set_has(Expr::LocalGet(STRING_SET_LOCAL), allocating_string()), + ], + ); + assert!( + rooted.contains("@js_set_has_string("), + "{lowering}: the fixture must select the string arm, or this proves nothing:\n{rooted}" + ); + let rooted_slots = temp_root_slots(&rooted); + assert_eq!( + rooted_slots.len(), + 1, + "{lowering}: #9523 — the string-arm receiver must be the ONE temp root across \ + the allocating value; got {rooted_slots:?}:\n{rooted}" + ); + + let unrooted = main_ir_for( + "set_has_string_arm_no_gc.ts", + vec![ + string_set_local(), + set_has( + Expr::LocalGet(STRING_SET_LOCAL), + Expr::String("k".to_string()), + ), + ], + ); + assert!( + unrooted.contains("@js_set_has_string("), + "{lowering}: the control must select the same arm:\n{unrooted}" + ); + assert_no_temp_rooting( + &unrooted, + &format!("{lowering}: a literal value cannot collect, so the receiver pays no slot"), + ); + }); +} diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index 5bb6d13a42..4644c57f92 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -6975,10 +6975,12 @@ fn artifact_records_dynamic_property_set_value_bits_before_helper() { /// #9459: the sloppy twin of the test above. /// -/// Same module, `is_strict: false`, so the store takes the sloppy tail. The -/// representation contract is identical -- the RHS is lowered by -/// `lower_value_for_dynamic_property_set` and stays `js_value_bits` until the -/// helper edge -- only the consumer/boxed-at labels and the runtime entry differ. +/// Same module, `is_strict: false`. The representation contract is identical -- +/// the RHS is lowered by `lower_value_for_dynamic_property_set` and stays +/// `js_value_bits` until the helper edge. #9495: the two modes share one tail +/// (`js_put_value_set` with the assignment's own `Throw` flag), so the labels +/// are the same too; the twin stays so that the invariant is asserted on both +/// values of the flag rather than on one. #[test] fn artifact_records_sloppy_dynamic_property_set_value_bits_before_helper() { let module = module_with_classes_and_params( @@ -7001,12 +7003,12 @@ fn artifact_records_sloppy_dynamic_property_set_value_bits_before_helper() { assert!( records.iter().any(|record| { record["expr_kind"] == "PropertySet" - && record["consumer"] == "property_set.sloppy_dynamic_value_bits" + && record["consumer"] == "property_set.dynamic_value_bits" && record["native_rep_name"] == "js_value_bits" && record["llvm_ty"] == "i64" && record["native_value_state"] == "region_local" && record["access_mode"].is_null() - && record_has_note(record, "boxed_at=sloppy_property_set_helper_edge") + && record_has_note(record, "boxed_at=dynamic_property_set_helper_edge") }), "expected sloppy dynamic property-set RHS to stay as js_value_bits before the helper edge:\n{artifact:#}" ); diff --git a/crates/perry-codegen/tests/typed_feedback.rs b/crates/perry-codegen/tests/typed_feedback.rs index c84fd1cd02..f2a19fb0b9 100644 --- a/crates/perry-codegen/tests/typed_feedback.rs +++ b/crates/perry-codegen/tests/typed_feedback.rs @@ -362,10 +362,11 @@ fn typed_feedback_instruments_property_and_method_boundaries() { // later tests in this binary never observe the changed environment. let _lock = env_lock(); let _env = EnvVarGuard::set("PERRY_TYPED_FEEDBACK", Some("1")); - // #9459: STRICT -- the `object_set_by_name_guard` site and the - // `js_typed_feedback_object_set_field_by_name_fast` dispatcher this asserts - // live on the strict store tail. See `strict_module`. The sloppy tail is - // asserted by `sloppy_property_set_uses_strictness_aware_put_value` below. + // #9459: STRICT -- see `strict_module`. #9495: the strict and sloppy tails + // are now ONE tail (`js_put_value_set` with the assignment's own `Throw` + // flag), so the `object_set_by_name_guard` site asserted here is registered + // in both modes; the sloppy twin below asserts the flag, not a different + // lane. let ir = ir_for(strict_module( "typed_feedback_property.ts", vec![param(1, "obj", Type::Any)], @@ -399,8 +400,26 @@ fn typed_feedback_instruments_property_and_method_boundaries() { assert!(ir.contains("object_set_by_name_guard")); assert!(ir.contains("object_get_by_name_guard")); assert!(ir.contains("method_call_guard")); - assert!(ir.contains("js_typed_feedback_object_set_field_by_name_fast")); - assert!(ir.contains("js_object_set_field_by_name")); + // #9495: the strict by-name store is the receiver-aware `[[Set]]` + // (`js_put_value_set(..., 1)`), observed by the pure-recording + // `js_typed_feedback_observe_property_set` in an emitting build -- not the + // own-property `js_typed_feedback_object_set_field_by_name_fast` -> + // `js_object_set_field_by_name` dispatcher, which skipped the prototype + // walk. Match CALLS: every runtime entry is `declare`d in every module. + assert!( + ir.contains("call void @js_typed_feedback_observe_property_set("), + "an emitting build observes the strict by-name store:\n{ir}" + ); + assert_eq!( + put_value_set_strict_flags(&ir), + vec!["1"], + "the strict store reaches `js_put_value_set` with Throw = 1:\n{ir}" + ); + assert!( + !ir.contains("call void @js_typed_feedback_object_set_field_by_name_fast("), + "the own-property store dispatcher must not be CALLED on a strict \ + by-name store -- it skips the prototype walk (#9495):\n{ir}" + ); assert!(ir.contains("js_object_get_field_by_name_f64")); assert!(ir.contains("call double @js_typed_feedback_native_call_method")); assert!(ir.contains("call void @js_typed_feedback_record_guard_pass")); @@ -409,16 +428,35 @@ fn typed_feedback_instruments_property_and_method_boundaries() { assert!(ir.contains("call void @js_typed_feedback_observe_property_get")); } +/// The `i32` `Throw` flag of every `js_put_value_set` CALL in `ir`, in emission +/// order. Matches the call, never the `declare` line, and reads the flag off +/// the call's last operand so a test can pin WHICH mode reached the entry. +fn put_value_set_strict_flags(ir: &str) -> Vec<&str> { + ir.lines() + .filter(|line| line.contains("call double @js_put_value_set(")) + .map(|line| { + let args = line.rsplit_once(')').map(|(head, _)| head).unwrap_or(line); + args.rsplit_once("i32 ") + .map(|(_, flag)| flag.trim()) + .expect("js_put_value_set call ends in its i32 Throw flag") + }) + .collect() +} + /// #9459: the sloppy twin of the boundaries test above. /// /// A sloppy `o.x = v` must not reach `js_object_set_field_by_name` -- it has no /// `strict` parameter and rejects a non-writable slot by throwing, where sloppy /// `PutValue` discards the rejection. It goes to `js_put_value_set(..., 0)`. /// +/// #9495: the strict tail is the same entry with `1`, so the two modes are now +/// distinguished by the flag alone; the typed-feedback `PropertySet` site is +/// registered in both (it describes the store, not its strictness), and the +/// old own-property dispatcher is called in neither. +/// /// The GET boundary is asserted too, and is deliberately unchanged: strictness /// is a property of `PutValue`, not of `GetValue`. That assertion is also what -/// keeps this test from passing on an empty program -- every other check here is -/// a negative. +/// keeps this test from passing on an empty program. #[test] fn sloppy_property_set_uses_strictness_aware_put_value() { let _lock = env_lock(); @@ -445,18 +483,22 @@ fn sloppy_property_set_uses_strictness_aware_put_value() { // every module, so `contains("js_put_value_set")` would pass on a module // that never stores and `contains("...set_field_by_name_fast")` would fail // on one that never does either. - assert!( - ir.contains("call double @js_put_value_set("), - "a sloppy property store should reach the strictness-aware [[Set]]:\n{ir}" + assert_eq!( + put_value_set_strict_flags(&ir), + vec!["0"], + "a sloppy property store should reach the strictness-aware [[Set]] with \ + Throw = 0:\n{ir}" ); assert!( !ir.contains("call void @js_typed_feedback_object_set_field_by_name_fast("), - "the strict typed-feedback store dispatcher must not be CALLED on a \ - sloppy store -- its underlying setter throws on a rejected write:\n{ir}" + "the own-property store dispatcher must not be CALLED on a sloppy \ + store -- its underlying setter throws on a rejected write:\n{ir}" ); assert!( - !ir.contains("object_set_by_name_guard"), - "no typed-feedback SET site is registered on the sloppy tail:\n{ir}" + ir.contains("object_set_by_name_guard") + && ir.contains("call void @js_typed_feedback_observe_property_set("), + "the typed-feedback SET site is registered and observed on the sloppy \ + tail too -- it is one tail since #9495:\n{ir}" ); // ANTI-VACUITY: the reads are untouched by strictness and must still be // instrumented, so the negatives above are about the store lane and not @@ -546,9 +588,29 @@ fn a_default_build_emits_no_typed_feedback_recording_calls() { // And the helpers that DECIDE something, rather than merely counting, are // not gated: this is the line between the two, asserted rather than // described. + // + // #9495: this used to name `js_typed_feedback_object_set_field_by_name_fast` + // on the `obj.x = 1` store. That dispatcher chose between the shape- + // transition fast path and the by-name setter; the store is now the + // receiver-aware `js_put_value_set`, which makes that choice itself, so no + // wrapper stands on the store and its observation is a gated recording + // helper like the rest (asserted absent above). The line is now drawn on + // the two dispatchers this same fixture still emits -- the property GET + // (the set dispatcher's twin) and the method call -- and as CALLS, since + // the old symbol match was satisfied by the `declare` line alone. + assert!( + ir.contains("call double @js_typed_feedback_object_get_field_by_name_f64("), + "dispatching feedback wrappers must still be emitted in a default build \ + (property get):\n{ir}" + ); + assert!( + ir.contains("call double @js_typed_feedback_native_call_method"), + "dispatching feedback wrappers must still be emitted in a default build \ + (method call):\n{ir}" + ); assert!( - ir.contains("js_typed_feedback_object_set_field_by_name_fast"), - "dispatching feedback wrappers must still be emitted in a default build" + ir.contains("call double @js_put_value_set("), + "the by-name store itself must still be lowered in a default build:\n{ir}" ); } diff --git a/crates/perry-codegen/tests/typed_shape_descriptors.rs b/crates/perry-codegen/tests/typed_shape_descriptors.rs index 7957589b65..38dd52b960 100644 --- a/crates/perry-codegen/tests/typed_shape_descriptors.rs +++ b/crates/perry-codegen/tests/typed_shape_descriptors.rs @@ -304,13 +304,18 @@ fn scalar_object_literal_keeps_initializers_read_by_update() { fn assert_typed_feedback_setter_after(ir: &str, start_pos: usize, context: &str) { let after_start = &ir[start_pos..]; + // #9459 / #9495: the dynamic by-name store is the receiver-aware `[[Set]]` + // (`js_put_value_set`) in both modes -- not the own-property + // `js_typed_feedback_object_set_field_by_name` wrapper, which had no + // `strict` parameter and no prototype walk. Match the CALL: every runtime + // entry is `declare`d in every module. assert!( - after_start.contains("call void @js_typed_feedback_object_set_field_by_name"), - "{context} should use the typed-feedback setter wrapper" + after_start.contains("call double @js_put_value_set("), + "{context} should reach the receiver-aware runtime setter" ); assert!( - ir.contains("js_object_set_field_by_name"), - "{context} should keep the safe runtime setter as the typed-feedback fallback" + !after_start.contains("call void @js_typed_feedback_object_set_field_by_name"), + "{context} must not take the own-property setter wrapper (#9495)" ); } diff --git a/crates/perry-dispatch/src/cjs_default_modules.rs b/crates/perry-dispatch/src/cjs_default_modules.rs new file mode 100644 index 0000000000..798f436d19 --- /dev/null +++ b/crates/perry-dispatch/src/cjs_default_modules.rs @@ -0,0 +1,151 @@ +//! The one table of Node builtins whose CommonJS `module.exports` is a +//! namespace object distinct from the ESM namespace — the modules for which +//! `require('')`, `import x from ''` and +//! `process.getBuiltinModule('')` hand out a `.default` namespace +//! whose method calls and property reads must reach the base module. +//! +//! #9500 (from #9485 / #9498): this knowledge used to live in FOUR +//! hand-maintained copies — the runtime's `cjs_default_base_module` and +//! `cjs_default_namespace_name` tables, the `cjs_default_export_value` match +//! arm, the method-call router's own `.default → base` list, and the +//! HIR's `is_cjs_style_native_default_import` (itself duplicated in two files +//! that had already drifted apart: one lacked `ffi`, `inspector`, +//! `inspector/promises` and `wasi`). The router's copy drifted far enough that +//! `require('child_process').spawn(...)` dispatched under a name with no +//! bucket and returned `undefined` WITHOUT SPAWNING, which is why claude-code's +//! MCP stdio client reported `Failed to connect` (#9485). Every consumer now +//! derives from this table: adding a module here is the whole edit. +//! +//! Base names are the runtime's canonical spellings (`path.posix`, not +//! `path/posix`; `util`, not `sys`) — the alias folding happens in +//! `normalize_native_module_name` before any lookup here. + +/// Builds the `(base, ".default")` pairs from one literal per module, so +/// the two spellings cannot disagree. +macro_rules! cjs_default_namespace_modules { + ($($base:literal),+ $(,)?) => { + /// `(base module, ".default")` for every Node builtin with a + /// distinct CommonJS default namespace. Sorted by base name. + pub const CJS_DEFAULT_NAMESPACE_MODULES: &[(&str, &str)] = + &[$(($base, concat!($base, ".default"))),+]; + }; +} + +cjs_default_namespace_modules!( + "async_hooks", + "child_process", + "cluster", + "constants", + "dns", + "dns/promises", + "ffi", + "inspector", + "inspector/promises", + "module", + "node-pty", + "os", + "path", + "path.posix", + "path.win32", + "process", + "punycode", + "querystring", + "repl", + "sea", + "url", + "util", + "wasi", +); + +/// Whether `base` (canonical spelling) has a distinct `.default` +/// CommonJS namespace. +pub fn has_cjs_default_namespace(base: &str) -> bool { + cjs_default_namespace_name(base).is_some() +} + +/// `base` → `".default"`, the name the CJS default namespace object is +/// created under. +pub fn cjs_default_namespace_name(base: &str) -> Option<&'static str> { + CJS_DEFAULT_NAMESPACE_MODULES + .iter() + .find(|(b, _)| *b == base) + .map(|(_, name)| *name) +} + +/// `".default"` → `base`: the module a CJS default namespace's method +/// calls and property reads dispatch against. +pub fn cjs_default_base_module(namespace_name: &str) -> Option<&'static str> { + CJS_DEFAULT_NAMESPACE_MODULES + .iter() + .find(|(_, name)| *name == namespace_name) + .map(|(base, _)| *base) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_row_round_trips() { + for (base, name) in CJS_DEFAULT_NAMESPACE_MODULES { + assert_eq!(*name, format!("{base}.default")); + assert_eq!(cjs_default_namespace_name(base), Some(*name)); + assert_eq!(cjs_default_base_module(name), Some(*base)); + assert!(has_cjs_default_namespace(base)); + } + } + + #[test] + fn rows_are_unique_and_sorted() { + let bases: Vec<&str> = CJS_DEFAULT_NAMESPACE_MODULES + .iter() + .map(|(b, _)| *b) + .collect(); + let mut sorted = bases.clone(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!(bases, sorted, "keep the table sorted and duplicate-free"); + } + + #[test] + fn base_names_are_canonical_spellings() { + for (base, _) in CJS_DEFAULT_NAMESPACE_MODULES { + assert!(!base.starts_with("node:"), "{base}: strip the node: scheme"); + assert!( + !matches!(*base, "sys" | "path/posix" | "path/win32"), + "{base}: an alias, not a canonical module name" + ); + } + } + + #[test] + fn modules_without_a_cjs_default_namespace_are_absent() { + for base in [ + "fs", + "crypto", + "events", + "http", + "stream", + "test", + "child_process.default", + ] { + assert!(!has_cjs_default_namespace(base), "{base}"); + assert_eq!(cjs_default_namespace_name(base), None, "{base}"); + } + assert_eq!(cjs_default_base_module("child_process"), None); + assert_eq!(cjs_default_base_module("fs.default"), None); + } + + /// The #9485 regression, pinned at the source of truth. + #[test] + fn child_process_default_maps_to_child_process() { + assert_eq!( + cjs_default_base_module("child_process.default"), + Some("child_process") + ); + assert_eq!( + cjs_default_namespace_name("child_process"), + Some("child_process.default") + ); + } +} diff --git a/crates/perry-dispatch/src/lib.rs b/crates/perry-dispatch/src/lib.rs index 7026ca6e7e..be1e330537 100644 --- a/crates/perry-dispatch/src/lib.rs +++ b/crates/perry-dispatch/src/lib.rs @@ -96,6 +96,7 @@ pub struct MethodRow { // re-exported below so consumers keep using `perry_dispatch::PERRY_*`. mod audio_table; mod background_table; +mod cjs_default_modules; mod i18n_table; mod ios_table; mod media_table; @@ -106,6 +107,10 @@ mod updater_table; pub use audio_table::PERRY_AUDIO_TABLE; pub use background_table::PERRY_BACKGROUND_TABLE; +pub use cjs_default_modules::{ + cjs_default_base_module, cjs_default_namespace_name, has_cjs_default_namespace, + CJS_DEFAULT_NAMESPACE_MODULES, +}; pub use i18n_table::PERRY_I18N_TABLE; pub use ios_table::PERRY_IOS_TABLE; pub use media_table::PERRY_MEDIA_TABLE; diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 532ead4347..4c045db809 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/crates/perry-hir/src/lower/context.rs @@ -466,20 +466,53 @@ impl LoweringContext { pub(crate) fn resolve_class_name(&self, name: &str) -> String { self.class_renames .get(name) - .cloned() + .map(|(registration_key, _)| registration_key.clone()) .unwrap_or_else(|| name.to_string()) } - /// Register a scope-local rename for `class X` when an outer/prior `class X` - /// is already registered (a distinct class that the name-keyed dedup would - /// otherwise skip). Returns immediately if no collision or already aliased. - /// Call from each body's Phase-1.5 class scan. - pub(crate) fn maybe_rename_colliding_class(&mut self, name: &str) { - if self.lookup_class(name).is_some() && !self.class_renames.contains_key(name) { - let unique = format!("{}${}", name, self.next_class_rename_id); - self.next_class_rename_id += 1; - self.class_renames.insert(name.to_string(), unique); + /// Mint a scope-local rename for `class X` when an outer/prior `class X` is + /// already registered (a distinct class that the name-keyed dedup would + /// otherwise skip). Returns `Some(displaced entry)` when it minted — so the + /// caller can restore exactly what it replaced — and `None` when no rename + /// was needed. + /// + /// #9466: the "already renamed" guard is per SCOPE, not per name. It used + /// to be `!class_renames.contains_key(name)`, but `class_renames` inherits + /// every enclosing body's aliases, so a NESTED body declaring the same name + /// took that branch and registered its class under the OUTER body's key. + /// The two source classes then shared one ClassId, whichever body lowered + /// first won, and the other's members silently vanished — no diagnostic. + /// Keying on `scope_key` (the declaring scope's source span) keeps the + /// idempotence the old guard existed for — one alias per scope, so a scope + /// scanned twice (a function body: Phase-1.5, then `lower_block_stmt`) + /// mints exactly once — while letting each nested scope mint its own. + pub(crate) fn mint_class_rename( + &mut self, + name: &str, + scope_key: u32, + ) -> Option> { + if self.lookup_class(name).is_none() { + return None; + } + if self + .class_renames + .get(name) + .is_some_and(|(_, key)| *key == scope_key) + { + return None; } + let unique = format!("{}${}", name, self.next_class_rename_id); + self.next_class_rename_id += 1; + Some( + self.class_renames + .insert(name.to_string(), (unique, scope_key)), + ) + } + + /// Single-name entry point for the function-body Phase-1.5 class scans, + /// which snapshot and restore the whole `class_renames` map themselves. + pub(crate) fn maybe_rename_colliding_class(&mut self, name: &str, scope_key: u32) { + let _ = self.mint_class_rename(name, scope_key); } /// Is `name` a user-declared `interface`? Interfaces are not classes, so diff --git a/crates/perry-hir/src/lower/expr_function.rs b/crates/perry-hir/src/lower/expr_function.rs index cbbbf6953e..16e1c3d0c5 100644 --- a/crates/perry-hir/src/lower/expr_function.rs +++ b/crates/perry-hir/src/lower/expr_function.rs @@ -1151,7 +1151,7 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul // shape `(function(e){…class s{…}…})(t)` declares superstruct's // `Struct` = `class s`, which collided with other `class s` in // the bundle and was dedup-skipped). See `class_renames`. - ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str()); + ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str(), block.span.lo.0); let cname = class_decl.ident.sym.to_string(); ctx.forward_class_decl_depth .entry(cname.clone()) diff --git a/crates/perry-hir/src/lower/lower_expr/helpers.rs b/crates/perry-hir/src/lower/lower_expr/helpers.rs index db8b60f754..45679dda18 100644 --- a/crates/perry-hir/src/lower/lower_expr/helpers.rs +++ b/crates/perry-hir/src/lower/lower_expr/helpers.rs @@ -11,6 +11,10 @@ use anyhow::Result; use swc_ecma_ast as ast; use crate::lower_types::extract_ts_type_with_ctx; +// #9500: one CJS-default-import predicate, derived from the shared table — +// this file used to carry its own copy, which had drifted (no `ffi`, +// `inspector`, `inspector/promises`, `wasi`). +use crate::lower::module_decl::native_default_import::is_cjs_style_native_default_import; /// Whether `PERRY_GLOBAL_SCRIPT_THIS` is set — compile the program as a /// *global script* rather than a CJS module, so module top-level `this` @@ -185,29 +189,6 @@ pub(crate) fn is_fetch_global_value_name(name: &str) -> bool { ) } -pub(crate) fn is_cjs_style_native_default_import(module_name: &str) -> bool { - matches!( - module_name, - "async_hooks" - | "child_process" - | "cluster" - | "constants" - | "dns" - | "dns/promises" - | "events" - | "module" - | "os" - | "path" - | "path/posix" - | "path/win32" - | "punycode" - | "querystring" - | "sys" - | "url" - | "util" - ) -} - pub(crate) fn wrap_with_gets(property: &str, fallback: Expr, envs: Vec) -> Expr { envs.into_iter() .rev() diff --git a/crates/perry-hir/src/lower/lowering_context.rs b/crates/perry-hir/src/lower/lowering_context.rs index f744350041..ef37621c37 100644 --- a/crates/perry-hir/src/lower/lowering_context.rs +++ b/crates/perry-hir/src/lower/lowering_context.rs @@ -820,9 +820,21 @@ pub struct LoweringContext { /// name-keyed: `Expr::New { class_name }` / `ClassRef(name)`). When a body /// declares `class X` while an outer/prior `class X` is already registered, /// the body's X is renamed `X$` and `X -> X$` recorded so every - /// reference in that body binds to the lexically-correct class. Saved/ - /// restored per body in both `lower_fn_body_block_stmt` and `lower_fn_expr`. - pub(crate) class_renames: std::collections::HashMap, + /// reference in that body binds to the lexically-correct class. + /// + /// The value is `(registration key, scope key)`, where the scope key is the + /// source span of the LEXICAL SCOPE that minted the alias. #9466: without + /// it there was one alias per NAME — a nested body inherited the enclosing + /// body's alias, saw "already renamed", and registered its own `class X` + /// under the OUTER body's key, so the two classes shared one ClassId and + /// the inner body was silently dropped. With it there is one alias per + /// (name, scope), which is what a lexical declaration actually is. + /// + /// Bracketed at every scope that can declare a class: the function-body + /// Phase-1.5 scans in `lower_fn_body_block_stmt` / `lower_fn_expr` + /// (whole-map snapshot + restore), and `enter_class_rename_scope` / + /// `exit_class_rename_scope` for `{ … }`-shaped block scopes. + pub(crate) class_renames: std::collections::HashMap, /// Monotonic suffix source for `class_renames` unique names. pub(crate) next_class_rename_id: u32, /// Names of TOP-LEVEL `class X { … }` declarations in the module being diff --git a/crates/perry-hir/src/lower/module_decl.rs b/crates/perry-hir/src/lower/module_decl.rs index fc06854594..3f63f68aa7 100644 --- a/crates/perry-hir/src/lower/module_decl.rs +++ b/crates/perry-hir/src/lower/module_decl.rs @@ -10,7 +10,7 @@ use super::*; use crate::ir::*; mod namespace; -mod native_default_import; +pub(super) mod native_default_import; pub(super) mod native_profile_import; mod object_literal; mod static_import_bindings; diff --git a/crates/perry-hir/src/lower/module_decl/native_default_import.rs b/crates/perry-hir/src/lower/module_decl/native_default_import.rs index b509d628d3..1250026bad 100644 --- a/crates/perry-hir/src/lower/module_decl/native_default_import.rs +++ b/crates/perry-hir/src/lower/module_decl/native_default_import.rs @@ -10,31 +10,39 @@ pub(crate) fn canonicalize_native_import_source(raw_source: &str) -> String { } } +/// Whether a native module's default import binds its CommonJS +/// `module.exports` — a `default` property read plus a builtin-module alias +/// for member calls — rather than the historical namespace object. +/// +/// #9500: derived from the ONE shared table +/// (`perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES`) that the runtime's +/// property-read and method-call paths also consume, so the three cannot +/// drift apart again (the HIR alone used to carry two hand-written copies of +/// this list, and one had already lost `ffi`, `inspector`, +/// `inspector/promises` and `wasi`). The arms below are the deliberate +/// differences between "has a `.default` namespace at runtime" and +/// "lowers as a CJS-style default import", each spelled out so a new table +/// row is classified by this function automatically and only an exception +/// needs a line here. pub(crate) fn is_cjs_style_native_default_import(module_name: &str) -> bool { - matches!( - module_name, - "async_hooks" - | "child_process" - | "cluster" - | "constants" - | "dns" - | "dns/promises" - | "events" - | "ffi" - | "inspector" - | "inspector/promises" - | "module" - | "os" - | "path" - | "path/posix" - | "path/win32" - | "punycode" - | "querystring" - | "sys" - | "url" - | "util" - | "wasi" - ) + match module_name { + // `events`' CommonJS export is the `EventEmitter` class itself + // (`cjs_default_export_value("events")`), not a `.default` + // namespace, but the default import is still CJS-shaped. + "events" => true, + // Aliases the runtime folds before any table lookup + // (`normalize_native_module_name`: `sys` → `util`, `path/posix` → + // `path.posix`, `path/win32` → `path.win32`); the HIR sees the + // import's own spelling. + "sys" | "path/posix" | "path/win32" => true, + // Table rows whose default import the HIR keeps on the namespace + // object: `process` has its own lowering (the `source == "process"` + // arms in `module_decl.rs`); `node-pty`, `repl` and `sea` never took + // the CJS-style path — flipping them is a lowering change, not a + // dedup, and is left for a follow-up. + "node-pty" | "process" | "repl" | "sea" => false, + other => perry_dispatch::has_cjs_default_namespace(other), + } } pub(crate) fn node_submodule_default_export_key(module_name: &str) -> Option<&'static str> { @@ -43,3 +51,71 @@ pub(crate) fn node_submodule_default_export_key(module_name: &str) -> Option<&'s _ => None, } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Shared-table rows the HIR deliberately keeps on the namespace-object + /// default. Adding a row to the table classifies it CJS-style unless it + /// is listed here — so this list, not the table, is what a lowering + /// decision edits. + const NAMESPACE_OBJECT_DEFAULT_ROWS: &[&str] = &["node-pty", "process", "repl", "sea"]; + + #[test] + fn every_shared_table_row_is_classified() { + for (base, _) in perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES { + let expected = !NAMESPACE_OBJECT_DEFAULT_ROWS.contains(base); + assert_eq!( + is_cjs_style_native_default_import(base), + expected, + "`{base}`: shared-table row classified unexpectedly" + ); + } + for base in NAMESPACE_OBJECT_DEFAULT_ROWS { + assert!( + perry_dispatch::has_cjs_default_namespace(base), + "`{base}` is listed as an exclusion but is not a shared-table row" + ); + } + } + + /// The spellings only the HIR sees (aliases + the callable-default + /// `events`) stay CJS-style. + #[test] + fn hir_only_spellings_are_cjs_style() { + for module in ["events", "sys", "path/posix", "path/win32"] { + assert!(is_cjs_style_native_default_import(module), "{module}"); + } + } + + /// #9485 / #9500: the rows one of the two former copies had lost, plus + /// the module the regression was found on. + #[test] + fn formerly_drifted_rows_are_cjs_style() { + for module in [ + "child_process", + "ffi", + "inspector", + "inspector/promises", + "wasi", + ] { + assert!(is_cjs_style_native_default_import(module), "{module}"); + } + } + + #[test] + fn plain_esm_shaped_builtins_are_not() { + for module in [ + "fs", + "fs/promises", + "crypto", + "http", + "stream", + "test", + "buffer", + ] { + assert!(!is_cjs_style_native_default_import(module), "{module}"); + } + } +} diff --git a/crates/perry-hir/src/lower/stmt.rs b/crates/perry-hir/src/lower/stmt.rs index e0a0802be2..5acbea4c5a 100644 --- a/crates/perry-hir/src/lower/stmt.rs +++ b/crates/perry-hir/src/lower/stmt.rs @@ -1822,9 +1822,19 @@ pub(crate) fn lower_stmt( // Case statement-lists share the switch's block scope without // being a `BlockStmt`, so they don't pass through // `lower_block_stmt` — re-bind their pre-registered - // forward-captured lets here (all cases up front: one scope). + // forward-captured lets here (all cases up front: one scope), and + // (#9466) disambiguate the `class` declarations they hold for the + // same reason. Every case shares ONE lexical scope, so they take + // one shared scope key: a second case re-declaring the name is a + // redeclaration, not a shadow. + let mut saved_class_renames = Vec::new(); for case in &switch_stmt.cases { rebind_nested_forward_scope_lets(ctx, &case.cons); + saved_class_renames.extend(enter_class_rename_scope( + ctx, + switch_stmt.span.lo.0, + &case.cons, + )); } for case in &switch_stmt.cases { @@ -1838,6 +1848,7 @@ pub(crate) fn lower_stmt( cases.push(SwitchCase { test, body }); } + exit_class_rename_scope(ctx, saved_class_renames); ctx.pop_block_scope(switch_scope_mark); module.init.push(Stmt::Switch { diff --git a/crates/perry-hir/src/lower_decl/block.rs b/crates/perry-hir/src/lower_decl/block.rs index f9cae5ccc9..c2315806a2 100644 --- a/crates/perry-hir/src/lower_decl/block.rs +++ b/crates/perry-hir/src/lower_decl/block.rs @@ -20,7 +20,22 @@ pub(crate) use var_names::{ pub fn lower_block_stmt(ctx: &mut LoweringContext, block: &ast::BlockStmt) -> Result> { rebind_nested_forward_scope_lets(ctx, &block.stmts); - lower_stmts_using_aware(ctx, &block.stmts) + // #9466: `class` is block-scoped, so a `class X` here is a DISTINCT class + // from any enclosing/prior `class X` and needs its own registration key. + // This is the funnel every `{}`-shaped scope shares — bare block, `if` / + // `else` branch, loop body, `try` / `catch` / `finally` — the same set + // `rebind_nested_forward_scope_lets` documents. Bracketed so the alias dies + // with the block. + // + // Keyed on the block's span: a FUNCTION body arrives here after + // `lower_fn_body_block_stmt`'s Phase-1.5 scan already aliased this same + // block, and the matching key makes this call a no-op rather than a second + // alias — which would strand that function's end-of-body capture + // re-registration on the now-stale key. + let saved_class_renames = enter_class_rename_scope(ctx, block.span.lo.0, &block.stmts); + let lowered = lower_stmts_using_aware(ctx, &block.stmts); + exit_class_rename_scope(ctx, saved_class_renames); + lowered } /// Make the forward-captured `let`/`const` bindings that @@ -524,7 +539,7 @@ pub fn lower_fn_body_block_stmt( // Disambiguate a distinct same-named class declared in this body so // its references don't bind to a colliding `class X` elsewhere in // the bundled module (see `class_renames`). - ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str()); + ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str(), block.span.lo.0); let cname = class_decl.ident.sym.to_string(); // Record the (shallowest) scope depth this class is declared at so a // later bare-ident reference can compare it against a same-named @@ -1025,13 +1040,22 @@ pub fn lower_block_stmt_scoped( block: &ast::BlockStmt, ) -> Result> { let mark = ctx.push_block_scope(); + // #9466: the strict-mode branch does NOT route through `lower_block_stmt`, + // so the block-scoped class disambiguation is bracketed here, around both + // branches. On the non-strict path `lower_block_stmt`'s own bracket sees + // this same span key and is a no-op. + let saved_class_renames = enter_class_rename_scope(ctx, block.span.lo.0, &block.stmts); // Via `lower_block_stmt` so this scope's pre-registered forward-captured // lets are re-bound at entry (`rebind_nested_forward_scope_lets`). let stmts = if ctx.current_strict { - lower_strict_block_fn_decls(ctx, block)? + lower_strict_block_fn_decls(ctx, block) } else { - lower_block_stmt(ctx, block)? + lower_block_stmt(ctx, block) }; + exit_class_rename_scope(ctx, saved_class_renames); + // `?` deliberately AFTER the rename restore but BEFORE `pop_block_scope`, + // preserving this function's original error control flow exactly. + let stmts = stmts?; ctx.pop_block_scope(mark); Ok(stmts) } @@ -1160,6 +1184,58 @@ fn register_block_forward_lexicals(ctx: &mut LoweringContext, stmts: &[ast::Stmt newly } +/// What one [`enter_class_rename_scope`] bracket displaced, keyed by source +/// name: `None` = the name had no active alias, `Some(entry)` = the enclosing +/// scope's alias to put back. See `LoweringContext::class_renames`. +pub(crate) type ClassRenameScopeSave = Vec<(String, Option<(String, u32)>)>; + +/// #9466: scope-entry hook for a `{ … }`-shaped lexical scope — disambiguate +/// every `class X` declared DIRECTLY in `stmts` and return what to hand to +/// [`exit_class_rename_scope`] on the way out. +/// +/// A `class` declaration is block-scoped, so a bare block, an `if`/`else` +/// branch, a loop body, a `try` / `catch` / `finally` block and a `switch` body +/// each shadow an enclosing same-named class exactly as a `let` does. Before +/// #9466 only FUNCTION bodies ran the disambiguation scan, so two sibling +/// `{ class X { … } }` blocks registered one ClassId between them and the +/// second block silently ran the first's body. +/// +/// Deliberately mirrors [`register_block_forward_lexicals`] (#6062), which +/// brackets this same boundary for TDZ names: record only what this call +/// changed and undo exactly that, so an alias owned by an enclosing scope +/// survives the block. +pub(crate) fn enter_class_rename_scope( + ctx: &mut LoweringContext, + scope_key: u32, + stmts: &[ast::Stmt], +) -> ClassRenameScopeSave { + let mut saved = ClassRenameScopeSave::new(); + for stmt in stmts { + let ast::Stmt::Decl(ast::Decl::Class(class_decl)) = stmt else { + continue; + }; + let name = class_decl.ident.sym.as_str(); + if let Some(displaced) = ctx.mint_class_rename(name, scope_key) { + saved.push((name.to_string(), displaced)); + } + } + saved +} + +/// Undo an [`enter_class_rename_scope`] bracket, innermost mint first. +pub(crate) fn exit_class_rename_scope(ctx: &mut LoweringContext, saved: ClassRenameScopeSave) { + for (name, displaced) in saved.into_iter().rev() { + match displaced { + Some(entry) => { + ctx.class_renames.insert(name, entry); + } + None => { + ctx.class_renames.remove(&name); + } + } + } +} + pub fn lower_stmts_using_aware( ctx: &mut LoweringContext, stmts: &[ast::Stmt], diff --git a/crates/perry-hir/src/lower_decl/body_stmt.rs b/crates/perry-hir/src/lower_decl/body_stmt.rs index 59c2b347b5..f9be1c6e73 100644 --- a/crates/perry-hir/src/lower_decl/body_stmt.rs +++ b/crates/perry-hir/src/lower_decl/body_stmt.rs @@ -1025,9 +1025,19 @@ fn lower_body_stmt_impl(ctx: &mut LoweringContext, stmt: &ast::Stmt) -> Result Result.default` <-> base) is shared +# with perry-hir through perry-dispatch, so the runtime's property-read and +# method-call paths and the HIR's import lowering cannot drift apart. +perry-dispatch.workspace = true thiserror.workspace = true anyhow.workspace = true libc.workspace = true diff --git a/crates/perry-runtime/src/builtins/formatting.rs b/crates/perry-runtime/src/builtins/formatting.rs index c585ffc889..876e6ebd2e 100644 --- a/crates/perry-runtime/src/builtins/formatting.rs +++ b/crates/perry-runtime/src/builtins/formatting.rs @@ -12,6 +12,7 @@ use super::*; mod array_buffer; mod boxed_primitives; mod collection_equality; +mod errors; pub(crate) use boxed_primitives::{ boxed_primitive_json_value, boxed_primitive_payload, boxed_primitive_to_string_tag, prune_dead_boxed_primitive_payload_owners, @@ -701,117 +702,6 @@ impl Drop for InspectCompactGuard { } } -unsafe fn string_header_to_string(ptr: *mut StringHeader, fallback: &str) -> String { - if ptr.is_null() { - return fallback.to_string(); - } - let len = (*ptr).byte_len as usize; - let data = (ptr as *const u8).add(std::mem::size_of::()); - let bytes = std::slice::from_raw_parts(data, len); - std::str::from_utf8(bytes).unwrap_or(fallback).to_string() -} - -unsafe fn format_error_headline(error_ptr: *const crate::error::ErrorHeader) -> String { - let name_str = string_header_to_string((*error_ptr).name, "Error"); - let message_str = string_header_to_string((*error_ptr).message, ""); - if message_str.is_empty() { - name_str - } else { - format!("{}: {}", name_str, message_str) - } -} - -/// The one stack line `util.inspect` shows under an error's headline. -/// -/// #9486: through the accessor, never off the field — `alloc_error` leaves -/// `stack` null and the first read materialises it, so a direct field read -/// here made `console.log(err)` print no frame at all. It is called from -/// `format_error_value` as the LAST use of `error_ptr` on purpose: the -/// accessor allocates, and a moving scavenge during that allocation would -/// leave any later read of `error_ptr` pointing at from-space. -unsafe fn format_error_stack_frame(error_ptr: *mut crate::error::ErrorHeader) -> Option { - let stack = string_header_to_string(crate::error::js_error_get_stack(error_ptr), ""); - stack - .lines() - .skip(1) - .find(|line| !line.trim().is_empty()) - .map(str::to_string) -} - -unsafe fn format_error_array(arr_ptr: *const crate::array::ArrayHeader, depth: usize) -> String { - if arr_ptr.is_null() { - return "[]".to_string(); - } - let length = (*arr_ptr).length as usize; - if length == 0 { - return "[]".to_string(); - } - let data_ptr = - (arr_ptr as *const u8).add(std::mem::size_of::()) as *const f64; - let mut out = String::from("["); - for i in 0..length { - out.push('\n'); - out.push_str(" "); - out.push_str(&format_jsvalue_for_json(*data_ptr.add(i), depth + 1)); - } - out.push('\n'); - out.push_str(" ]"); - out -} - -unsafe fn format_error_value(error_ptr: *const crate::error::ErrorHeader, depth: usize) -> String { - let headline = format_error_headline(error_ptr); - let mut entries: Vec<(String, String)> = - crate::node_submodules::error_user_props(error_ptr as usize) - .into_iter() - .filter(|(key, _)| key != "cause" && key != "errors") - .map(|(key, value)| (key, format_jsvalue_for_json(value, depth + 1))) - .collect(); - - let cause = (*error_ptr).cause; - if !crate::value::JSValue::from_bits(cause.to_bits()).is_undefined() { - entries.push(( - "[cause]".to_string(), - format_jsvalue_for_json(cause, depth + 1), - )); - } - - if !(*error_ptr).errors.is_null() { - entries.push(( - "[errors]".to_string(), - format_error_array((*error_ptr).errors, depth + 1), - )); - } - - if entries.is_empty() { - return headline; - } - - let mut out = headline; - if let Some(frame) = format_error_stack_frame(error_ptr as *mut _) { - out.push('\n'); - out.push_str(&frame); - out.push_str(" {"); - } else { - out.push_str("\n{"); - } - - let last = entries.len().saturating_sub(1); - for (idx, (label, value)) in entries.into_iter().enumerate() { - out.push('\n'); - out.push_str(" "); - out.push_str(&label); - out.push_str(": "); - out.push_str(&value); - if idx != last { - out.push(','); - } - } - out.push('\n'); - out.push('}'); - out -} - /// #2089: a Date's `util.inspect` rendering — ISO string (unquoted) or "Invalid Date". DateCell pointer only (gated by callers). unsafe fn date_inspect_string(value: f64) -> String { let s_ptr = crate::date::js_date_to_iso_string(value); @@ -981,7 +871,7 @@ pub(crate) fn format_jsvalue(value: f64, depth: usize) -> String { if gc_type == crate::gc::GC_TYPE_ERROR { let error_ptr = ptr as *const crate::error::ErrorHeader; - format_error_value(error_ptr, depth) + errors::format_error_value(error_ptr, depth) } else if gc_type == crate::gc::GC_TYPE_ARRAY { // Array — format as [ elem1, elem2, ... ] matching Node.js util.inspect. // Cycle check FIRST so back-edges win over depth truncation @@ -1248,14 +1138,14 @@ fn format_weak_wrapper( /// crash safety net for cyclic structures; the Node-style `[Object]` truncation /// at depth > 2 is enforced by `format_jsvalue_for_json` on the way in. unsafe fn format_object_as_json( - obj_ptr: *const crate::object::ObjectHeader, + mut obj_ptr: *const crate::object::ObjectHeader, depth: usize, ) -> String { if depth > 10 { return "{...}".to_string(); } - let obj_addr = obj_ptr as usize; + let mut obj_addr = obj_ptr as usize; // `[util.inspect.custom]` hook: when the object carries a symbol-keyed // entry for `Symbol.for("nodejs.util.inspect.custom")` and the @@ -1366,6 +1256,18 @@ unsafe fn format_object_as_json( crate::object::class_name_for_id(class_id).filter(|name| !name.is_empty()) } }; + let error_headline = if crate::object::extends_builtin_error((*obj_ptr).class_id) { + let (headline, refreshed_obj_ptr) = errors::format_error_subclass_headline( + obj_ptr, + (*obj_ptr).class_id, + class_name.as_deref().unwrap_or("Error"), + ); + obj_ptr = refreshed_obj_ptr; + obj_addr = obj_ptr as usize; + Some(headline) + } else { + None + }; let has_class_name = class_name.is_some(); let class_name_ref = if deep_equal_skip_prototype_format_enabled() { None @@ -1385,14 +1287,21 @@ unsafe fn format_object_as_json( // null-proto plain object, otherwise nothing. (Distinct from // `class_name_ref`/`has_class_name`, which drive the private-field skip // and must reflect only a genuine class.) - let name_prefix: Option = match class_name_ref { - Some(name) => Some(name.to_string()), - None if boxed_base.is_none() && is_null_proto => { - Some("[Object: null prototype]".to_string()) + let name_prefix: Option = if let Some(headline) = error_headline.as_ref() { + Some(headline.clone()) + } else { + match class_name_ref { + Some(name) => Some(name.to_string()), + None if boxed_base.is_none() && is_null_proto => { + Some("[Object: null prototype]".to_string()) + } + None => None, } - None => None, }; let empty_object = || { + if let Some(headline) = error_headline.as_deref() { + return headline.to_string(); + } if let Some(base) = boxed_base.as_deref() { return base.to_string(); } @@ -1445,6 +1354,13 @@ unsafe fn format_object_as_json( continue; } + // Error inspection consumes an own `name` into the headline. Node + // does not print it again as an enumerable body property unless + // showHidden asks for the complete reflective surface. + if error_headline.is_some() && !show_hidden && key_str == "name" { + continue; + } + // Hide a boxed String's character index properties (`"0".."len-1"`): // they are rendered by the `[String: '…']` base, not the body. if let Some(char_count) = boxed_string_char_count { @@ -1701,7 +1617,7 @@ fn format_jsvalue_for_json(value: f64, depth: usize) -> String { if gc_type == crate::gc::GC_TYPE_ERROR { let error_ptr = ptr as *const crate::error::ErrorHeader; - format_error_value(error_ptr, depth) + errors::format_error_value(error_ptr, depth) } else if gc_type == crate::gc::GC_TYPE_ARRAY { // Cycle check FIRST so back-edges always print as // `[Circular *N]` regardless of depth (#1204). The @@ -1846,8 +1762,35 @@ fn escape_string(s: &str) -> String { /// whitespace, numeric-leading, and non-ASCII names are quoted. `$` is a /// valid JavaScript identifier character but Node deliberately quotes it in /// inspected object keys. -mod inspect_property_key; -use inspect_property_key::format_inspect_property_key; +fn format_inspect_property_key(key: &str) -> String { + let mut chars = key.chars(); + let is_bare = chars + .next() + .is_some_and(|first| first.is_ascii_alphabetic() || first == '_') + && chars.all(|c| c.is_ascii_alphanumeric() || c == '_'); + if is_bare { + return key.to_string(); + } + + // Node prefers the delimiter that avoids an escape when exactly one kind + // of quote occurs in the key. + if key.contains('\'') && !key.contains('"') { + let escaped = key + .chars() + .flat_map(|c| match c { + '\\' => "\\\\".chars().collect::>(), + '"' => "\\\"".chars().collect(), + '\n' => "\\n".chars().collect(), + '\r' => "\\r".chars().collect(), + '\t' => "\\t".chars().collect(), + _ => vec![c], + }) + .collect::(); + format!("\"{}\"", escaped) + } else { + format!("'{}'", escape_string(key)) + } +} #[cfg(test)] mod inspect_property_key_tests; diff --git a/crates/perry-runtime/src/builtins/formatting/errors.rs b/crates/perry-runtime/src/builtins/formatting/errors.rs new file mode 100644 index 0000000000..2bc86f7e96 --- /dev/null +++ b/crates/perry-runtime/src/builtins/formatting/errors.rs @@ -0,0 +1,234 @@ +//! `util.inspect` formatting for native Errors and ordinary-layout Error +//! subclasses. + +use super::*; + +unsafe fn string_header_to_string(ptr: *mut StringHeader, fallback: &str) -> String { + if ptr.is_null() { + return fallback.to_string(); + } + let len = (*ptr).byte_len as usize; + let data = (ptr as *const u8).add(std::mem::size_of::()); + let bytes = std::slice::from_raw_parts(data, len); + std::str::from_utf8(bytes).unwrap_or(fallback).to_string() +} + +unsafe fn format_error_headline(error_ptr: *const crate::error::ErrorHeader) -> String { + let scope = crate::gc::RuntimeHandleScope::new(); + let error_h = scope.root_raw_const_ptr(error_ptr); + let own_name_h = error_h + .with_const_ptr::(|error_ptr| { + crate::node_submodules::error_user_prop(error_ptr as usize, "name") + }) + .map(|value| scope.root_nanbox_f64(value)); + let (own_message_h, error_ptr) = error_h.across_const::(|| { + error_h + .with_const_ptr::(|error_ptr| { + crate::node_submodules::error_user_prop(error_ptr as usize, "message") + }) + .map(|value| scope.root_nanbox_f64(value)) + }); + let display_part = |value: Option<&crate::gc::RuntimeHandle<'_>>, + header: *mut StringHeader, + fallback: &str| { + value + .and_then(|handle| jsvalue_string_content(handle.get_nanbox_f64())) + .unwrap_or_else(|| string_header_to_string(header, fallback)) + }; + // `ErrorHeader.name` is internal backing storage for the inherited + // Error-family prototype value. An explicit `error.name = ...` is an own + // expando and must drive inspection without being redundantly printed as + // a body property (#9440). + let name_str = display_part(own_name_h.as_ref(), (*error_ptr).name, "Error"); + let message_str = display_part(own_message_h.as_ref(), (*error_ptr).message, ""); + if message_str.is_empty() { + name_str + } else { + format!("{}: {}", name_str, message_str) + } +} + +unsafe fn format_error_stack_frame(error_ptr: *const crate::error::ErrorHeader) -> Option { + let stack = string_header_to_string((*error_ptr).stack, ""); + stack + .lines() + .skip(1) + .find(|line| !line.trim().is_empty()) + .map(str::to_string) +} + +unsafe fn format_error_array(arr_ptr: *const crate::array::ArrayHeader, depth: usize) -> String { + if arr_ptr.is_null() { + return "[]".to_string(); + } + let length = (*arr_ptr).length as usize; + if length == 0 { + return "[]".to_string(); + } + let data_ptr = + (arr_ptr as *const u8).add(std::mem::size_of::()) as *const f64; + let mut out = String::from("["); + for i in 0..length { + out.push('\n'); + out.push_str(" "); + out.push_str(&format_jsvalue_for_json(*data_ptr.add(i), depth + 1)); + } + out.push('\n'); + out.push_str(" ]"); + out +} + +pub(super) unsafe fn format_error_value( + error_ptr: *const crate::error::ErrorHeader, + depth: usize, +) -> String { + // Headline lookup consults the ordinary expando bag and may allocate. + // Keep the native Error live and re-read its address for every later slot. + let scope = crate::gc::RuntimeHandleScope::new(); + let error_h = scope.root_raw_const_ptr(error_ptr); + let headline = error_h.with_const_ptr::(|error_ptr| { + format_error_headline(error_ptr) + }); + let mut entries: Vec<(String, String)> = error_h + .with_const_ptr::(|error_ptr| { + crate::node_submodules::error_user_props(error_ptr as usize) + }) + .into_iter() + .filter(|(key, _)| key != "cause" && key != "errors" && key != "name") + .map(|(key, value)| (key, format_jsvalue_for_json(value, depth + 1))) + .collect(); + + let cause = + error_h.with_const_ptr::(|error_ptr| (*error_ptr).cause); + if !crate::value::JSValue::from_bits(cause.to_bits()).is_undefined() { + entries.push(( + "[cause]".to_string(), + format_jsvalue_for_json(cause, depth + 1), + )); + } + + let errors = + error_h.with_const_ptr::(|error_ptr| (*error_ptr).errors); + if !errors.is_null() { + entries.push(( + "[errors]".to_string(), + format_error_array(errors, depth + 1), + )); + } + + if entries.is_empty() { + return headline; + } + + let mut out = headline; + if let Some(frame) = error_h.with_const_ptr::(|error_ptr| { + format_error_stack_frame(error_ptr) + }) { + out.push('\n'); + out.push_str(&frame); + out.push_str(" {"); + } else { + out.push_str("\n{"); + } + + let last = entries.len().saturating_sub(1); + for (idx, (label, value)) in entries.into_iter().enumerate() { + out.push('\n'); + out.push_str(" "); + out.push_str(&label); + out.push_str(": "); + out.push_str(&value); + if idx != last { + out.push(','); + } + } + out.push('\n'); + out.push('}'); + out +} + +/// Build Node's Error headline for a user class whose instances use the +/// ordinary object layout. The class registry supplies the Error-family +/// prototype name; own `message` and explicitly assigned `name` values come +/// from the instance slots (#9440). +pub(super) unsafe fn format_error_subclass_headline( + obj_ptr: *const crate::object::ObjectHeader, + class_id: u32, + class_name: &str, +) -> (String, *const crate::object::ObjectHeader) { + // String coercion below can collect. Keep both the receiver and the two + // values which can participate in the headline live across either + // conversion, and return the receiver's refreshed address to the caller. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_raw_const_ptr(obj_ptr); + let keys = obj_h.with_const_ptr::(|obj_ptr| { + crate::object::object_keys_array(obj_ptr) + }); + let mut own_name: Option = None; + let mut own_message: Option = None; + if !keys.is_null() { + let len = crate::array::js_array_length(keys); + for index in 0..len { + let key = crate::array::js_array_get(keys, index); + if !key.is_string() { + continue; + } + let key_ptr = key.as_string_ptr(); + if key_ptr.is_null() { + continue; + } + let key_len = (*key_ptr).byte_len as usize; + let key_data = (key_ptr as *const u8).add(std::mem::size_of::()); + let key_bytes = std::slice::from_raw_parts(key_data, key_len); + if key_bytes == b"name" { + own_name = Some(obj_h.with_const_ptr::( + |obj_ptr| crate::object::js_object_get_field_f64(obj_ptr, index), + )); + } else if key_bytes == b"message" { + own_message = Some(obj_h.with_const_ptr::( + |obj_ptr| crate::object::js_object_get_field_f64(obj_ptr, index), + )); + } + } + } + + let own_name_h = own_name.map(|value| scope.root_nanbox_f64(value)); + let own_message_h = own_message.map(|value| scope.root_nanbox_f64(value)); + // The string coercions below can collect; compute the headline inside + // `across_const` so the receiver address handed back is re-read afterwards. + let (headline, obj_ptr) = obj_h.across_const::(|| { + let value_string = |value: &crate::gc::RuntimeHandle<'_>, fallback: &str| { + let value = value.get_nanbox_f64(); + let js = JSValue::from_bits(value.to_bits()); + if js.is_undefined() { + return fallback.to_string(); + } + jsvalue_string_content(value).unwrap_or_else(|| { + let string = crate::value::js_jsvalue_to_string(value); + string_header_to_string(string, fallback) + }) + }; + let prototype_name = crate::object::builtin_error_prototype_name(class_id); + let name = own_name_h + .as_ref() + .map(|value| value_string(value, prototype_name)) + .unwrap_or_else(|| prototype_name.to_string()); + let message = own_message_h + .as_ref() + .map(|value| value_string(value, "")) + .unwrap_or_default(); + let display_name = if own_name_h.is_none() && class_name != name { + format!("{class_name} [{name}]") + } else { + name + }; + if display_name.is_empty() { + message + } else if message.is_empty() { + display_name + } else { + format!("{display_name}: {message}") + } + }); + (headline, obj_ptr) +} diff --git a/crates/perry-runtime/src/date/parse.rs b/crates/perry-runtime/src/date/parse.rs index 22bda19b31..fcec1bbcb3 100644 --- a/crates/perry-runtime/src/date/parse.rs +++ b/crates/perry-runtime/src/date/parse.rs @@ -82,9 +82,216 @@ fn parse_tz_offset(rest: &str) -> Option { }; let h: i64 = hh.parse().ok()?; let m: i64 = mm.parse().ok()?; + if h > 23 || m > 59 { + return None; + } Some(sign * (h * 60 + m)) } +/// V8's fixed legacy timezone-name table. These abbreviations deliberately do +/// not consult the host timezone database: `EST` always means UTC-05:00, even +/// for a date on which a particular location observes daylight time. +fn named_tz_offset(token: &str) -> Option { + let lower = token.to_ascii_lowercase(); + let fixed = match lower.as_str() { + "ut" | "utc" | "gmt" | "z" => Some(0), + "edt" => Some(-4 * 60), + "est" | "cdt" => Some(-5 * 60), + "cst" | "mdt" => Some(-6 * 60), + "mst" | "pdt" => Some(-7 * 60), + "pst" => Some(-8 * 60), + _ => None, + }; + if fixed.is_some() { + return fixed; + } + + // A GMT-family word may carry an attached numeric offset. V8 accepts the + // same spelling after a date-only form and after a clock. + for prefix in ["utc", "gmt", "ut", "z"] { + if lower.starts_with(prefix) && lower.len() > prefix.len() { + let rest = &token[prefix.len()..]; + if rest.starts_with('+') || rest.starts_with('-') { + return parse_tz_offset(rest).filter(|offset| *offset != i64::MAX); + } + } + } + None +} + +#[derive(Clone, Copy)] +struct ParsedClock { + hour: i64, + minute: i64, + second: i64, + millis: i64, + attached_tz: Option, +} + +fn parse_clock_digits(input: &str, index: &mut usize, min: usize, max: usize) -> Option { + let start = *index; + while *index < input.len() && *index - start < max && input.as_bytes()[*index].is_ascii_digit() + { + *index += 1; + } + if *index - start < min { + return None; + } + input[start..*index].parse().ok() +} + +/// Parse a complete clock token and return any numeric/Z designator attached +/// directly to it. In the ISO `T` spelling hour/minute/second fields are two +/// digits; the whitespace-separated legacy spelling also accepts one digit. +/// Alphabetic words are intentionally not accepted as an attached suffix — +/// `10:30 GMT` is valid while `10:30GMT` is not. +fn parse_clock_token(token: &str, strict_iso: bool) -> Option { + let mut index = 0usize; + let field_min = if strict_iso { 2 } else { 1 }; + let hour = parse_clock_digits(token, &mut index, field_min, 2)?; + if token.as_bytes().get(index) != Some(&b':') { + return None; + } + index += 1; + let minute = parse_clock_digits(token, &mut index, field_min, 2)?; + let mut second = 0i64; + let mut millis = 0i64; + + if token.as_bytes().get(index) == Some(&b':') { + index += 1; + // The legacy grammar accepts a trailing colon as an omitted seconds + // field (`10:30:`); ISO requires the two digits. + if index < token.len() && token.as_bytes()[index].is_ascii_digit() { + second = parse_clock_digits(token, &mut index, field_min, 2)?; + } else if strict_iso || index != token.len() { + return None; + } + if token.as_bytes().get(index) == Some(&b'.') { + index += 1; + let fraction_start = index; + while index < token.len() && token.as_bytes()[index].is_ascii_digit() { + index += 1; + } + if fraction_start == index { + return None; + } + millis = normalize_millis(&token[fraction_start..index]); + } + } + + if minute > 59 || second > 59 { + return None; + } + if hour > 24 || (hour == 24 && (minute != 0 || second != 0 || millis != 0)) { + return None; + } + + let attached_tz = if index == token.len() { + None + } else { + let rest = &token[index..]; + if rest.eq_ignore_ascii_case("z") || rest.starts_with('+') || rest.starts_with('-') { + Some(parse_tz_offset(rest).filter(|offset| *offset != i64::MAX)?) + } else { + return None; + } + }; + Some(ParsedClock { + hour, + minute, + second, + millis, + attached_tz, + }) +} + +/// Parse the implementation-defined tail after an ISO-shaped date when it is +/// not the strict `T` clock. Tokens are order-independent like V8's legacy +/// DateParser: a clock, AM/PM and a named/numeric zone may be combined, with a +/// later zone token winning. Parenthesized comments are explicitly consumed; +/// every other word must be recognized or the whole parse fails. +fn parse_legacy_iso_tail(tail: &str) -> Option<(Option, Option)> { + let mut clock: Option = None; + let mut meridiem: Option = None; // true => PM + let mut tz_minutes_east: Option = None; + let mut in_comment = false; + + for token in tail.split_whitespace() { + if in_comment { + if token.ends_with(')') { + in_comment = false; + } + continue; + } + if token.starts_with('(') { + if token.ends_with(')') { + continue; + } + if token.contains(')') { + return None; + } + in_comment = true; + continue; + } + if token.contains(['(', ')']) { + return None; + } + + let lower = token.to_ascii_lowercase(); + if lower == "am" || lower == "pm" { + meridiem = Some(lower == "pm"); + continue; + } + if let Some(offset) = named_tz_offset(token) { + tz_minutes_east = Some(offset); + continue; + } + if (token.starts_with('+') || token.starts_with('-')) && clock.is_some() { + let offset = parse_tz_offset(token)?; + if offset == i64::MAX { + return None; + } + tz_minutes_east = Some(offset); + continue; + } + if let Some(parsed) = parse_clock_token(token, false) { + if clock.is_some() { + return None; + } + if let Some(offset) = parsed.attached_tz { + tz_minutes_east = Some(offset); + } + clock = Some(parsed); + continue; + } + return None; + } + if in_comment { + return None; + } + + if let Some(is_pm) = meridiem { + let parsed = clock.as_mut()?; + // V8 accepts 00:xx AM as midnight, but rejects an hour above 12 when a + // meridiem is present. + if parsed.hour > 12 { + return None; + } + parsed.hour = if is_pm { + if parsed.hour == 12 { + 12 + } else { + parsed.hour + 12 + } + } else if parsed.hour == 12 { + 0 + } else { + parsed.hour + }; + } + Some((clock, tz_minutes_east)) +} + /// Reinterpret an instant that was composed with `make_utc_ms` from /// wall-clock components as LOCAL time: subtract the host's UTC offset in /// effect at that instant. Shared by every grammar here that yields @@ -131,113 +338,61 @@ fn parse_iso8601(s: &str) -> Option { let mut second: i64 = 0; let mut millis: i64 = 0; - // Year only ("YYYY" / "±YYYYYY"). - if s.len() == year_end { - return Some(make_utc_ms( - year, - month1 as i64 - 1, - day, - hour, - minute, - second, - millis, - )); - } - // Require a '-' for month. - if b.get(year_end) != Some(&b'-') { - return None; - } - if b.len() < year_end + 3 { - return None; - } - month1 = s[year_end + 1..year_end + 3].parse().ok()?; - if !(1..=12).contains(&month1) { - return None; - } - let mut idx = year_end + 3; - let mut has_day = false; + let mut idx = year_end; if b.get(idx) == Some(&b'-') { if b.len() < idx + 3 { return None; } - day = s[idx + 1..idx + 3].parse().ok()?; - if !(1..=31).contains(&day) { + month1 = s[idx + 1..idx + 3].parse().ok()?; + if !(1..=12).contains(&month1) { return None; } idx += 3; - has_day = true; + if b.get(idx) == Some(&b'-') { + if b.len() < idx + 3 { + return None; + } + day = s[idx + 1..idx + 3].parse().ok()?; + if !(1..=31).contains(&day) { + return None; + } + idx += 3; + } } - // Time part (after 'T' or ' '). - let mut tz_minutes_east: Option = None; // None => "no offset present" - // #9449: the presence of a time component — not the presence of a zone — - // is what decides the default interpretation below. + // #9509: parsing the tail is explicit and exhaustive. A strict `T` clock + // may carry only its attached numeric/Z designator. The legacy tail used + // by whitespace-separated clocks and date-only zone words is tokenized; + // every token must be recognized. + let mut tz_minutes_east: Option = None; let mut has_time = false; if idx < s.len() { - let sep = b[idx]; - if sep != b'T' && sep != b' ' { - return None; - } - // Month-only "YYYY-MM" cannot carry a time component. - if !has_day { - return None; - } - let time_str = &s[idx + 1..]; - // Split off a trailing zone designator. Scan for the first of - // 'Z', '+', '-' after the HH:MM[:SS[.sss]] body. - let zone_pos = time_str - .char_indices() - .find(|(i, c)| *i > 0 && (*c == 'Z' || *c == '+' || *c == '-')) - .map(|(i, _)| i); - let (clock, zone) = match zone_pos { - Some(p) => (&time_str[..p], &time_str[p..]), - None => (time_str, ""), - }; - // #9449: node also accepts the designator as a trailing, whitespace- - // separated WORD in the space-separated spelling — - // `new Date("2026-09-01 10:30 GMT")` is 10:30 UTC, and so are the - // `UTC` / `UT` / `Z` spellings in either case. The scan above only - // finds `Z`, `+` and `-`, so the word used to be ignored outright; - // that was invisible while every offsetless form was read as UTC and - // becomes a wrong instant the moment they are read as local. A - // parenthesised trailing comment is NOT a designator and stays local. - let (clock, utc_word) = match clock.rsplit_once(char::is_whitespace) { - Some((head, tail)) - if !head.trim().is_empty() - && matches!( - tail.to_ascii_lowercase().as_str(), - "gmt" | "utc" | "ut" | "z" - ) => + if b[idx] == b'T' { + let parsed = parse_clock_token(&s[idx + 1..], true)?; + hour = parsed.hour; + minute = parsed.minute; + second = parsed.second; + millis = parsed.millis; + tz_minutes_east = parsed.attached_tz; + has_time = true; + } else { + let tail = &s[idx..]; + // A clock or token that follows the numeric date must either be + // whitespace-separated or be a directly-attached zone word. + if !tail.as_bytes()[0].is_ascii_whitespace() + && !tail.as_bytes()[0].is_ascii_alphabetic() { - (head.trim_end(), true) - } - _ => (clock, false), - }; - let cb = clock.as_bytes(); - if clock.len() < 5 || cb[2] != b':' { - return None; - } - hour = clock[0..2].parse().ok()?; - minute = clock[3..5].parse().ok()?; - has_time = true; - if clock.len() >= 8 && cb[5] == b':' { - second = clock[6..8].parse().ok()?; - if clock.len() > 9 && cb[8] == b'.' { - let frac = &clock[9..]; - let frac_digits: String = frac.chars().take_while(|c| c.is_ascii_digit()).collect(); - if !frac_digits.is_empty() { - millis = normalize_millis(&frac_digits); - } + return None; } - } - if !zone.is_empty() { - match parse_tz_offset(zone) { - Some(v) if v == i64::MAX => {} - Some(v) => tz_minutes_east = Some(v), - None => return None, + let (parsed_clock, parsed_tz) = parse_legacy_iso_tail(tail)?; + if let Some(parsed) = parsed_clock { + hour = parsed.hour; + minute = parsed.minute; + second = parsed.second; + millis = parsed.millis; + has_time = true; } - } else if utc_word { - tz_minutes_east = Some(0); + tz_minutes_east = parsed_tz; } } let base = make_utc_ms(year, month1 as i64 - 1, day, hour, minute, second, millis); @@ -251,7 +406,6 @@ fn parse_iso8601(s: &str) -> Option { // deliberate asymmetry. This half was already right; it must stay. None => base, }; - let _ = idx; Some(adjusted) } diff --git a/crates/perry-runtime/src/date/tests.rs b/crates/perry-runtime/src/date/tests.rs index 580b12d572..d51e34c4b3 100644 --- a/crates/perry-runtime/src/date/tests.rs +++ b/crates/perry-runtime/src/date/tests.rs @@ -207,6 +207,59 @@ fn test_date_parse_iso_offsetless_datetime_is_local() { assert_eq!(wall("2026-09-01 10:30 (comment)"), (2026, 9, 1, 10, 30, 0)); } +/// #9509: the ISO/space parser must consume its complete tail. V8 accepts a +/// fixed set of zone and meridiem tokens; an unknown or unseparated word is +/// Invalid Date rather than ignored. +#[test] +fn test_date_parse_iso_tail_tokens_are_consumed() { + let wall = |s: &str| { + let ts = parse_date_string(s); + assert!(!ts.is_nan(), "expected a valid date for {s:?}"); + let (y, mo, d, h, mi, sec, _) = timestamp_to_local_components((ts as i64).div_euclid(1000)); + (y, mo, d, h, mi, sec) + }; + + assert_eq!(wall("2026-09 10:30"), (2026, 9, 1, 10, 30, 0)); + assert_eq!(wall("2026-09-01 10:30"), (2026, 9, 1, 10, 30, 0)); + assert_eq!(wall("2026-09-01 10:30 PM"), (2026, 9, 1, 22, 30, 0)); + assert_eq!(wall("2026-09-01 12:30 AM"), (2026, 9, 1, 0, 30, 0)); + assert_eq!(wall("2026-09-01 12:30 PM"), (2026, 9, 1, 12, 30, 0)); + + let midnight = 1_788_220_800_000.0; + for s in ["2026-09-01 GMT", "2026-09-01 Z", "2026-09-01Z"] { + assert_eq!(parse_date_string(s), midnight, "{s:?}"); + } + assert_eq!( + parse_date_string("2026-09-01 EST"), + midnight + 5.0 * 3_600_000.0 + ); + assert_eq!( + parse_date_string("2026-09-01 PDT"), + midnight + 7.0 * 3_600_000.0 + ); + assert_eq!( + parse_date_string("2026-09-01 10:30 PM EST"), + midnight + 27.5 * 3_600_000.0 + ); + assert_eq!( + parse_date_string("2026-09-01 12:30 AM PST"), + midnight + 8.5 * 3_600_000.0 + ); + + for bad in [ + "2026-09-01 10:30GMT", + "2026-09-01 10:30EST", + "2026-09-01 10:30PM", + "2026-09-01 10:30 XYZ", + "2026-09-01 10:30:45oops", + ] { + assert!( + parse_date_string(bad).is_nan(), + "expected Invalid Date for {bad:?}" + ); + } +} + /// #9414: the numeric slash grammar node accepts as its /// implementation-defined format. Measured against /// `node --experimental-strip-types`, not derived from the spec (which diff --git a/crates/perry-runtime/src/object/class_constructors.rs b/crates/perry-runtime/src/object/class_constructors.rs index 37b2b33ea2..878542fba5 100644 --- a/crates/perry-runtime/src/object/class_constructors.rs +++ b/crates/perry-runtime/src/object/class_constructors.rs @@ -1054,15 +1054,24 @@ unsafe fn default_error_init_for_implicit_chain( if !crate::object::extends_builtin_error(class_cid) { return; } + let scope = crate::gc::RuntimeHandleScope::new(); + let this_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(inst as i64)); + // Read and root the forwarded value before stack capture can collect; the + // caller-owned argument slice itself is not a runtime handle. + let msg_h = if args_ptr.is_null() || args_len == 0 { + None + } else { + Some(scope.root_nanbox_f64(*args_ptr)) + }; // #9410: the dynamic replay path is a construction site like any other, // so the instance gets its own lazily-formatted `stack` here — before the // message guard below, which returns early for `new X()` with no argument // and would otherwise leave exactly those instances trace-less. - crate::error::js_error_subclass_capture_stack(crate::value::js_nanbox_pointer(inst as i64)); - if args_ptr.is_null() || args_len == 0 { + crate::error::js_error_subclass_capture_stack(this_h.get_nanbox_f64()); + let Some(msg_h) = msg_h else { return; - } - let msg = *args_ptr; + }; + let msg = msg_h.get_nanbox_f64(); if msg.to_bits() == crate::value::TAG_UNDEFINED { return; } @@ -1070,10 +1079,20 @@ unsafe fn default_error_init_for_implicit_chain( if msg_str.is_null() { return; } - let boxed = - f64::from_bits(crate::value::STRING_TAG | (msg_str as u64 & crate::value::POINTER_MASK)); - let key = crate::string::js_string_from_bytes(b"message".as_ptr(), b"message".len() as u32); - crate::object::js_object_set_field_by_name(inst, key, boxed); + let msg_str_h = scope.root_string_ptr(msg_str); + let key_h = scope.root_string_ptr(crate::string::js_string_from_bytes( + b"message".as_ptr(), + b"message".len() as u32, + )); + let inst = crate::value::js_nanbox_get_pointer(this_h.get_nanbox_f64()) as *mut ObjectHeader; + msg_str_h.with_const_ptr::(|msg_str| { + let boxed = f64::from_bits( + crate::value::STRING_TAG | (msg_str as u64 & crate::value::POINTER_MASK), + ); + key_h.with_const_ptr::(|key| { + crate::object::js_object_set_field_by_name(inst, key, boxed); + }); + }); } /// #6469: spec default Error-init, called from the SYNTHESIZED standalone @@ -1089,53 +1108,50 @@ unsafe fn default_error_init_for_implicit_chain( /// standalone ctor's forwarding params are padded with undefined for missing /// call args, and setting an OWN undefined `message` would shadow /// `Error.prototype.message` (""). Set non-enumerable, matching the built-in -/// (test262 NativeError/*-message). `name` mirrors the static arm: the -/// terminating Error-family kind as an own property. +/// (test262 NativeError/*-message). `name` remains inherited from the +/// terminating Error-family prototype (#9440). #[no_mangle] -pub unsafe extern "C" fn js_error_subclass_default_init( - this_val: f64, - msg: f64, - name_ptr: *const crate::StringHeader, -) { +pub unsafe extern "C" fn js_error_subclass_default_init(this_val: f64, msg: f64) { let bits = this_val.to_bits(); let raw = (bits & crate::value::POINTER_MASK) as usize; if raw < 0x10000 { return; } - let inst = raw as *mut ObjectHeader; + let scope = crate::gc::RuntimeHandleScope::new(); + let this_h = scope.root_nanbox_f64(this_val); + let msg_h = scope.root_nanbox_f64(msg); + // Capture first: V8 exposes `stack` before `message` from + // `getOwnPropertyNames`, and the lazy getter still observes the later + // message or an explicit user-assigned `name`. + crate::error::js_error_subclass_capture_stack(this_h.get_nanbox_f64()); + let msg = msg_h.get_nanbox_f64(); if msg.to_bits() != crate::value::TAG_UNDEFINED { let msg_str = crate::value::js_jsvalue_to_string(msg); if !msg_str.is_null() { - let boxed = f64::from_bits( - crate::value::STRING_TAG | (msg_str as u64 & crate::value::POINTER_MASK), - ); - let key = - crate::string::js_string_from_bytes(b"message".as_ptr(), b"message".len() as u32); - crate::object::js_object_set_field_by_name_nonenum(inst, key, boxed); + let msg_str_h = scope.root_string_ptr(msg_str); + let key_h = scope.root_string_ptr(crate::string::js_string_from_bytes( + b"message".as_ptr(), + b"message".len() as u32, + )); + let inst = + crate::value::js_nanbox_get_pointer(this_h.get_nanbox_f64()) as *mut ObjectHeader; + msg_str_h.with_const_ptr::(|msg_str| { + let boxed = f64::from_bits( + crate::value::STRING_TAG | (msg_str as u64 & crate::value::POINTER_MASK), + ); + key_h.with_const_ptr::(|key| { + crate::object::js_object_set_field_by_name_nonenum(inst, key, boxed); + }); + }); } } - if !name_ptr.is_null() { - let name_boxed = f64::from_bits( - crate::value::STRING_TAG | (name_ptr as u64 & crate::value::POINTER_MASK), - ); - let key = crate::string::js_string_from_bytes(b"name".as_ptr(), b"name".len() as u32); - crate::object::js_object_set_field_by_name(inst, key, name_boxed); - } - // #9410: `stack`. The synthesized standalone ctor stamps `message` and - // `name` but installed nothing for `stack`, so `new X("m").stack` was - // `undefined` for every `class X extends Error {}` with no own - // constructor. Last, so the getter's head sees the `name` just written. - crate::error::js_error_subclass_capture_stack(this_val); } /// Keepalive: generated code is the only caller (#6469). #[cfg(feature = "keepalive-anchors")] #[used] -static KEEP_JS_ERROR_SUBCLASS_DEFAULT_INIT: unsafe extern "C" fn( - f64, - f64, - *const crate::StringHeader, -) = js_error_subclass_default_init; +static KEEP_JS_ERROR_SUBCLASS_DEFAULT_INIT: unsafe extern "C" fn(f64, f64) = + js_error_subclass_default_init; /// Find the per-evaluation class object that owns `target_cid` while walking a /// fresh derived class's pinned parent chain. The template class-id registry diff --git a/crates/perry-runtime/src/object/class_meta_registry.rs b/crates/perry-runtime/src/object/class_meta_registry.rs index bd8580677f..83c2693d91 100644 --- a/crates/perry-runtime/src/object/class_meta_registry.rs +++ b/crates/perry-runtime/src/object/class_meta_registry.rs @@ -329,6 +329,31 @@ fn extends_builtin_error_slow(class_id: u32) -> bool { false } +/// Resolve the Error-family prototype at the bottom of a registered class +/// chain. Callers first establish [`extends_builtin_error`]; returning +/// `"Error"` on an incomplete/cyclic chain is the same fallback used by +/// ordinary prototype-property lookup. +pub(crate) fn builtin_error_prototype_name(class_id: u32) -> &'static str { + let mut current = class_id; + for _ in 0..32 { + match current { + crate::error::CLASS_ID_TYPE_ERROR => return "TypeError", + crate::error::CLASS_ID_RANGE_ERROR => return "RangeError", + crate::error::CLASS_ID_REFERENCE_ERROR => return "ReferenceError", + crate::error::CLASS_ID_SYNTAX_ERROR => return "SyntaxError", + crate::error::CLASS_ID_EVAL_ERROR => return "EvalError", + crate::error::CLASS_ID_URI_ERROR => return "URIError", + crate::error::CLASS_ID_AGGREGATE_ERROR => return "AggregateError", + crate::error::CLASS_ID_ERROR => return "Error", + _ => match get_parent_class_id(current) { + Some(parent) if parent != 0 && parent != current => current = parent, + _ => break, + }, + } + } + "Error" +} + #[cfg(test)] mod dense_parent_tests { use super::*; diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index ac983a3733..f3bb2f62cc 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1351,7 +1351,18 @@ fn js_object_get_own_property_names_shape(obj_value: f64) -> f64 { use super::exotic_expando::ExoticKind; let mut names = match kind { ExoticKind::RegExp => vec!["lastIndex".to_string()], - ExoticKind::Error => vec!["message".to_string(), "stack".to_string()], + ExoticKind::Error => { + // V8 creates the lazy own `stack` before Error's optional + // `message`. The header keeps both payload slots, but only + // `message` values supplied to the constructor are own + // properties; `name` is always inherited until assigned. + let mut builtin = vec!["stack".to_string()]; + let error = addr as *mut crate::error::ErrorHeader; + if crate::error::js_error_has_own_property(error, "message") { + builtin.push("message".to_string()); + } + builtin + } ExoticKind::Date | ExoticKind::Temporal | ExoticKind::Promise diff --git a/crates/perry-runtime/src/object/field_get_set/accessors.rs b/crates/perry-runtime/src/object/field_get_set/accessors.rs index 589bae723f..6e8d91ed66 100644 --- a/crates/perry-runtime/src/object/field_get_set/accessors.rs +++ b/crates/perry-runtime/src/object/field_get_set/accessors.rs @@ -316,45 +316,7 @@ pub(crate) unsafe fn ordinary_object_prototype_property_value( scope.root_nanbox_f64(crate::value::js_nanbox_pointer(obj as usize as i64)); let key_h = scope.root_nanbox_f64(crate::value::nanbox_string_key(key)); let _guard = object_prototype_lookup_guard()?; - let mut current = class_id; - let mut prototype_name = "Error"; - for _ in 0..32 { - match current { - crate::error::CLASS_ID_TYPE_ERROR => { - prototype_name = "TypeError"; - break; - } - crate::error::CLASS_ID_RANGE_ERROR => { - prototype_name = "RangeError"; - break; - } - crate::error::CLASS_ID_REFERENCE_ERROR => { - prototype_name = "ReferenceError"; - break; - } - crate::error::CLASS_ID_SYNTAX_ERROR => { - prototype_name = "SyntaxError"; - break; - } - crate::error::CLASS_ID_EVAL_ERROR => { - prototype_name = "EvalError"; - break; - } - crate::error::CLASS_ID_URI_ERROR => { - prototype_name = "URIError"; - break; - } - crate::error::CLASS_ID_AGGREGATE_ERROR => { - prototype_name = "AggregateError"; - break; - } - crate::error::CLASS_ID_ERROR => break, - _ => match super::super::get_parent_class_id(current) { - Some(parent) if parent != 0 && parent != current => current = parent, - _ => break, - }, - } - } + let prototype_name = super::super::builtin_error_prototype_name(class_id); let prototype = super::super::builtin_prototype_value(prototype_name); let prototype_value = JSValue::from_bits(prototype.to_bits()); if prototype_value.is_pointer() { diff --git a/crates/perry-runtime/src/object/global_this/fetch_globals.rs b/crates/perry-runtime/src/object/global_this/fetch_globals.rs index 8252c3c739..5d0e3f6f62 100644 --- a/crates/perry-runtime/src/object/global_this/fetch_globals.rs +++ b/crates/perry-runtime/src/object/global_this/fetch_globals.rs @@ -720,7 +720,7 @@ pub unsafe extern "C" fn js_fetch_or_value_super( // value routes through the dynamic-parent registry): the parent value is // the global Error-family constructor. The ordinary value-super dispatch // below invokes it as a plain call, which builds a FRESH error cell and - // drops it — `this` never receives `message`/`name`, so every subclass + // drops it — `this` never receives `message`, so every subclass // instance constructed through this path printed "An error has occurred". // Apply the spec default Error-init directly on `this`, mirroring the // static-`new` arm (#573, `lower_call/new.rs`) and the standalone-ctor arm @@ -736,28 +736,28 @@ pub unsafe extern "C" fn js_fetch_or_value_super( let stash = crate::object::class_registry::js_get_dynamic_parent_value(cid); super::super::class_registry::identify_global_builtin_constructor(stash) }); - if let Some(kind) = err_parent.filter(|k| { - matches!( - *k, - "Error" - | "TypeError" - | "RangeError" - | "ReferenceError" - | "SyntaxError" - | "URIError" - | "EvalError" - | "AggregateError" - ) - }) { + if err_parent + .filter(|k| { + matches!( + *k, + "Error" + | "TypeError" + | "RangeError" + | "ReferenceError" + | "SyntaxError" + | "URIError" + | "EvalError" + | "AggregateError" + ) + }) + .is_some() + { let msg = if !args_ptr.is_null() && args_len >= 1 { *args_ptr } else { undef }; - let name_str = crate::string::js_string_from_bytes(kind.as_ptr(), kind.len() as u32); - crate::object::class_constructors::js_error_subclass_default_init( - this_box, msg, name_str, - ); + crate::object::class_constructors::js_error_subclass_default_init(this_box, msg); return undef; } } diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 302b3fe0fa..4fd9536efc 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -236,8 +236,9 @@ pub use with_env::*; // named re-exports keep existing `crate::object::X` / bare-name call sites in // the object submodules resolving unchanged. pub(crate) use class_meta_registry::{ - class_generic_origin, extends_builtin_error, fetch_parent_kind, lookup_has_instance_hook, - lookup_to_string_tag_hook, register_fetch_parent_kind, CLASS_REGISTRY, + builtin_error_prototype_name, class_generic_origin, extends_builtin_error, fetch_parent_kind, + lookup_has_instance_hook, lookup_to_string_tag_hook, register_fetch_parent_kind, + CLASS_REGISTRY, }; pub use class_meta_registry::{ js_register_class_extends_error, js_register_class_generic_origin, diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index 014de86012..768a9fa811 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -670,62 +670,16 @@ pub(crate) fn subtle_crypto_namespace() -> f64 { js_create_native_module_namespace(b"crypto.subtle".as_ptr(), "crypto.subtle".len()) } +/// `".default"` → `mod`. #9500: a thin view over the ONE shared table +/// (`perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES`); the hand-maintained copy +/// that used to live here is what the method-call router drifted from (#9485). pub(crate) fn cjs_default_base_module(module_name: &str) -> Option<&'static str> { - match module_name { - "async_hooks.default" => Some("async_hooks"), - "child_process.default" => Some("child_process"), - "cluster.default" => Some("cluster"), - "constants.default" => Some("constants"), - "dns.default" => Some("dns"), - "dns/promises.default" => Some("dns/promises"), - "ffi.default" => Some("ffi"), - "inspector.default" => Some("inspector"), - "inspector/promises.default" => Some("inspector/promises"), - "module.default" => Some("module"), - "node-pty.default" => Some("node-pty"), - "os.default" => Some("os"), - "path.default" => Some("path"), - "path.posix.default" => Some("path.posix"), - "path.win32.default" => Some("path.win32"), - "process.default" => Some("process"), - "punycode.default" => Some("punycode"), - "querystring.default" => Some("querystring"), - "repl.default" => Some("repl"), - "sea.default" => Some("sea"), - "url.default" => Some("url"), - "util.default" => Some("util"), - "wasi.default" => Some("wasi"), - _ => None, - } + perry_dispatch::cjs_default_base_module(module_name) } +/// `mod` → `".default"`, from the same shared table (#9500). fn cjs_default_namespace_name(module_name: &str) -> Option<&'static str> { - match module_name { - "async_hooks" => Some("async_hooks.default"), - "child_process" => Some("child_process.default"), - "cluster" => Some("cluster.default"), - "constants" => Some("constants.default"), - "dns" => Some("dns.default"), - "dns/promises" => Some("dns/promises.default"), - "ffi" => Some("ffi.default"), - "inspector" => Some("inspector.default"), - "inspector/promises" => Some("inspector/promises.default"), - "module" => Some("module.default"), - "node-pty" => Some("node-pty.default"), - "os" => Some("os.default"), - "path" => Some("path.default"), - "path.posix" => Some("path.posix.default"), - "path.win32" => Some("path.win32.default"), - "process" => Some("process.default"), - "punycode" => Some("punycode.default"), - "querystring" => Some("querystring.default"), - "repl" => Some("repl.default"), - "sea" => Some("sea.default"), - "url" => Some("url.default"), - "util" => Some("util.default"), - "wasi" => Some("wasi.default"), - _ => None, - } + perry_dispatch::cjs_default_namespace_name(module_name) } fn create_cjs_default_namespace(module_name: &str) -> Option { @@ -764,10 +718,12 @@ pub(crate) fn cjs_default_export_value(module_name: &str) -> Option { b"wasi.default".as_ptr(), "wasi.default".len(), )), - "async_hooks" | "child_process" | "constants" | "dns" | "dns/promises" | "ffi" - | "node-pty" | "os" | "path" | "path.posix" | "path.win32" | "punycode" | "querystring" - | "repl" | "sea" | "url" | "util" | "inspector" | "inspector/promises" => { - create_cjs_default_namespace(module_name) + // #9500: every remaining row of the shared CJS-default table gets its + // `.default` namespace — the arms above are the modules whose + // default export is NOT that namespace (a callable, or the plain + // namespace itself) and must keep winning. + other if perry_dispatch::has_cjs_default_namespace(other) => { + create_cjs_default_namespace(other) } _ => None, } diff --git a/crates/perry-runtime/src/object/native_module_dispatch.rs b/crates/perry-runtime/src/object/native_module_dispatch.rs index 2fba11baaf..ded6e1039e 100644 --- a/crates/perry-runtime/src/object/native_module_dispatch.rs +++ b/crates/perry-runtime/src/object/native_module_dispatch.rs @@ -456,6 +456,25 @@ mod cjs_default_dispatch_tests { ); } + /// #9500: the spelled-out list above IS the shared table + /// (`perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES`), in both directions — + /// so a row added there is exercised by the two tests above, and a row + /// listed here that the table dropped is a failure rather than a stale name. + #[test] + fn spelled_out_list_matches_the_shared_table() { + let mut listed: Vec<&str> = CJS_DEFAULT_NAMESPACES.to_vec(); + let mut table: Vec<&str> = perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES + .iter() + .map(|(_, name)| *name) + .collect(); + listed.sort_unstable(); + table.sort_unstable(); + assert_eq!( + listed, table, + "CJS_DEFAULT_NAMESPACES and perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES differ" + ); + } + /// The exact regression, pinned by name. #[test] fn child_process_default_dispatches_as_child_process() { diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index c78cfe5106..8e2466bd6c 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -1616,21 +1616,7 @@ pub extern "C" fn js_string_split_regex_n( let parts: Vec> = if let Some(repeat_matcher) = lookup_repeat_matcher(re) { repeat_matcher.split(&str_data, limit) } else if let Some(fre) = lookup_fancy_regex(re) { - // Fancy-regex fallback (lookbehind/backreferences): `fancy_regex` has - // no `split`, so walk non-overlapping matches and slice between them. - // (Captured-group splicing is not reproduced for this engine.) - let mut v: Vec> = Vec::new(); - let mut last = 0usize; - let mut iter = fre.find_iter(&str_data); - while let Some(Ok(m)) = iter.next() { - v.push(Some(str_data[last..m.start()].to_string())); - last = m.end(); - } - v.push(Some(str_data[last..].to_string())); - if limit > 0 && (v.len() as i64) > (limit as i64) { - v.truncate(limit as usize); - } - v + crate::string::spec_fancy_regex_split(&fre, &str_data, limit) } else { // Standard engine: the JS `RegExp.prototype[Symbol.split]` algorithm // (21.2.5.11). The `regex` crate's own `split` diverges from JS for diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index af40de4e86..e44cb45fa7 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -219,15 +219,40 @@ fn fancy_lookbehind_search() { #[test] fn fancy_lookbehind_split() { - // Zero-width lookbehind split: "a1b2c3" → ["a1","b2","c3",""]. + // RegExp.prototype[@@split] never visits q == size, so a zero-width match + // at the end does not open a trailing empty chunk. let re = js_regexp_new(make_string(r"(?<=\d)"), make_string("")); let arr = js_string_split_regex(make_string("a1b2c3"), re); unsafe { - assert_eq!((*arr).length, 4); - let first = crate::array::js_array_get_f64(arr, 0); - let sp = crate::value::js_get_string_pointer_unified(first) as *const StringHeader; - assert_eq!(string_as_str(sp), "a1"); + assert_eq!((*arr).length, 3); } + assert_eq!( + (0..3) + .map(|index| match_capture_text(arr, index)) + .collect::>(), + vec![ + Some("a1".to_string()), + Some("b2".to_string()), + Some("c3".to_string()), + ] + ); + + // Separator captures are interleaved into the result. + let re = js_regexp_new(make_string(r"((?<=a)X)"), make_string("")); + let arr = js_string_split_regex(make_string("aXbXc"), re); + unsafe { + assert_eq!((*arr).length, 3); + } + assert_eq!( + (0..3) + .map(|index| match_capture_text(arr, index)) + .collect::>(), + vec![ + Some("a".to_string()), + Some("X".to_string()), + Some("bXc".to_string()), + ] + ); } #[test] diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index 21e7520372..d3ca201fad 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -118,7 +118,7 @@ mod raw; mod slice_ops; mod split; #[cfg(feature = "regex-engine")] -pub(crate) use split::spec_regex_split; +pub(crate) use split::{spec_fancy_regex_split, spec_regex_split}; #[cfg(test)] mod tests; diff --git a/crates/perry-runtime/src/string/split.rs b/crates/perry-runtime/src/string/split.rs index 007879f216..addeb94e55 100644 --- a/crates/perry-runtime/src/string/split.rs +++ b/crates/perry-runtime/src/string/split.rs @@ -100,6 +100,81 @@ pub(crate) fn spec_regex_split(regex: ®ex::Regex, s: &str, limit: i32) -> Vec out } +/// Fancy-regex implementation of the same `RegExp.prototype[Symbol.split]` +/// cursor algorithm as [`spec_regex_split`]. A plain `find_iter` is not enough: +/// the spec performs a sticky probe at each `q`, never probes `q == size`, and +/// splices every separator capture into the result. `captures_from_pos` keeps +/// the complete haystack visible to lookbehind while starting the search at +/// the spec cursor (#9429/#9438). +#[cfg(feature = "regex-engine")] +pub(crate) fn spec_fancy_regex_split( + regex: &fancy_regex::Regex, + s: &str, + limit: i32, +) -> Vec> { + let mut out: Vec> = Vec::new(); + let unbounded = limit < 0; + let push = |out: &mut Vec>, value: Option| -> bool { + out.push(value); + !unbounded && out.len() as i32 >= limit + }; + if limit == 0 { + return out; + } + + let size = s.len(); + if size == 0 { + // Empty subject: `[""]` unless the pattern itself matches empty. + if !matches!(regex.captures_from_pos(s, 0), Ok(Some(_))) { + out.push(Some(String::new())); + } + return out; + } + + let mut p = 0usize; + let mut q = 0usize; + while q < size { + let captures = match regex.captures_from_pos(s, q) { + Ok(Some(captures)) => captures, + Ok(None) | Err(_) => break, + }; + let Some(full) = captures.get(0) else { + break; + }; + if full.start() != q { + // Sticky probing found the next possible match to the right. No + // match exists between q and that position, so jump to it. + q = full.start(); + continue; + } + + let e = full.end().min(size); + if e == p { + // A zero-width match at the pending segment's start is skipped. + q = next_char_boundary(s, q); + continue; + } + if push(&mut out, Some(s[p..q].to_string())) { + return out; + } + for index in 1..captures.len() { + let group = captures + .get(index) + .map(|matched| matched.as_str().to_string()); + if push(&mut out, group) { + return out; + } + } + p = e; + q = p; + } + + if unbounded || (out.len() as i32) < limit { + out.push(Some(s[p..size].to_string())); + } + out +} + /// Split a string by a delimiter /// Returns an array of string pointers (stored as f64 bit patterns) #[no_mangle] diff --git a/test-files/test_gap_9410_error_subclass_stack.ts b/test-files/test_gap_9410_error_subclass_stack.ts index 57b8ea9477..bbc2939c1a 100644 --- a/test-files/test_gap_9410_error_subclass_stack.ts +++ b/test-files/test_gap_9410_error_subclass_stack.ts @@ -76,12 +76,9 @@ describe("factory-subclass", make("factory-msg"), "Error", "factory-msg"); // `stack` is not (node installs `stack` as a non-enumerable own property). const withField = new WithField("field-msg"); console.log("field value: " + withField.code); -// The subclass's own field enumerates; `stack` must not. NOT asserted here: -// the full `Object.keys` list, because perry additionally stamps an own -// ENUMERABLE `name` onto an Error-subclass instance where node leaves `name` -// on `Error.prototype` — a separate, pre-existing divergence (perry -// `["code","name"]` vs node `["code"]`) with its own fix, and asserting the -// whole list here would tie this fixture to that one. +// The subclass's own field enumerates; inherited `name` and own `stack` do +// not. The complete reflection/serialization contract is covered by #9440's +// dedicated fixture. console.log("field key enumerates: " + Object.keys(withField).includes("code")); console.log("stack key enumerates: " + Object.keys(withField).includes("stack")); console.log( diff --git a/test-files/test_gap_9438_fancy_regex_split.ts b/test-files/test_gap_9438_fancy_regex_split.ts new file mode 100644 index 0000000000..ce824b8160 --- /dev/null +++ b/test-files/test_gap_9438_fancy_regex_split.ts @@ -0,0 +1,31 @@ +// #9438: regex patterns that require fancy-regex used a separate split +// fallback which sliced between find_iter matches. That is not +// RegExp.prototype[@@split]: it emitted a trailing empty string for a match at +// the end and discarded every separator capture. + +function row(name: string, value: string[]): void { + console.log(name, JSON.stringify(value)); +} + +// Lookbehind and lookahead, with and without separator captures. +row("lookbehind/end", "a,b,".split(/(?<=,)/)); +row("lookbehind/capture", "aXbXc".split(/((?<=a)X)/)); +row("lookahead/middle", "abc".split(/(?=b)/)); +row("lookahead/capture", "aXbXc".split(/(X(?=b))/)); + +// A zero-width match at either boundary must not open an empty chunk. +row("start", "abc".split(/(?=a)/)); +row("end", "abc".split(/(?<=c)/)); + +// The empty-subject special case distinguishes a matching separator from a +// non-matching one. +row("empty/no-match", "".split(/(?<=a)/)); +row("empty/match", "".split(/(?=)/)); + +// Captures count toward limit just like ordinary chunks. +row("limit", "aXbXc".split(/((?<=a)X)/, 2)); + +// #9427 rewrites multiline anchors to lookaround-bearing patterns, so these +// ordinary /m spellings also exercise the fancy lane. +row("multiline-start", "a\r\nb".split(/^/gm)); +row("multiline-end", "a\r\nb".split(/$/gm)); diff --git a/test-files/test_gap_9440_error_name_ownership.ts b/test-files/test_gap_9440_error_name_ownership.ts new file mode 100644 index 0000000000..e3b608f332 --- /dev/null +++ b/test-files/test_gap_9440_error_name_ownership.ts @@ -0,0 +1,91 @@ +// #9440 — an Error subclass inherited `.name` from the Error-family +// prototype in Node, but Perry stamped it onto every instance as an enumerable +// own property. That leaked `name` through every own-key consumer. +// +// Keep all output portable and byte-comparable with +// `node --experimental-strip-types`: stack frames contain host paths, so the +// util.inspect check removes only `at ...` lines while retaining the Error +// headline and any rendered properties. + +import { inspect } from "node:util"; + +class Implicit extends Error {} + +class Explicit extends Error { + constructor(message: string) { + super(message); + } +} + +class Deep extends Implicit { + constructor(message: string) { + super(message); + } +} + +class TypeSubclass extends TypeError {} + +function forInKeys(value: object): string[] { + const keys: string[] = []; + for (const key in value) { + keys.push(key); + } + return keys; +} + +function stableInspect(value: unknown): string { + return inspect(value, { breakLength: Infinity }).replace( + /\n\s+at [^\n]*/g, + "", + ); +} + +function describe(label: string, error: Error): void { + console.log( + label + + " reflection: " + + JSON.stringify({ + name: error.name, + ownName: Object.prototype.hasOwnProperty.call(error, "name"), + descriptor: Object.getOwnPropertyDescriptor(error, "name"), + json: JSON.stringify(error), + ownNames: Object.getOwnPropertyNames(error), + keys: Object.keys(error), + forIn: forInKeys(error), + spread: { ...error }, + }), + ); + console.log(label + " inspect: " + JSON.stringify(stableInspect(error))); +} + +describe("base", new Error("base")); +describe("base-empty", new Error()); +describe("implicit", new Implicit("implicit")); +describe("implicit-empty", new Implicit()); +describe("explicit", new Explicit("explicit")); +describe("deep", new Deep("deep")); +describe("type-subclass", new TypeSubclass("typed")); + +// Exercise dynamic construction, which runs the synthesized standalone +// constructor rather than the direct-new initialization path. +const Dynamic: typeof Implicit = Implicit; +describe("dynamic", new Dynamic("dynamic")); + +function makeEscapedSubclass(): typeof Error { + return class Escaped extends Error {}; +} + +// Force the runtime constructor-replay path: the concrete subclass is created +// inside a function and only reaches this construction site as a value. +const Escaped = makeEscapedSubclass(); +describe("escaped-dynamic", new Escaped("escaped-dynamic")); + +// An explicit assignment must still create an ordinary own enumerable +// property, just as it does for any inherited writable data property. +const custom = new Implicit("custom"); +custom.name = "Custom"; +describe("assigned", custom); + +const customBase = new Error("custom-base"); +customBase.name = "CustomBase"; +describe("assigned-base", customBase); diff --git a/test-files/test_gap_9459_property_set_strictness.cts b/test-files/test_gap_9459_property_set_strictness.cts index 3729dfc0fc..f2462efbe3 100644 --- a/test-files/test_gap_9459_property_set_strictness.cts +++ b/test-files/test_gap_9459_property_set_strictness.cts @@ -29,7 +29,9 @@ // helper would test one mode twice. Only the mode prefix and the directive // differ. // -// Companions: test_gap_9422_strict_object_store_strictness.cts (the `=` lane), +// Companions: test_gap_9495_strict_inherited_property_set.cts (the prototype +// walk these spellings skipped in strict code), +// test_gap_9422_strict_object_store_strictness.cts (the `=` lane), // test_gap_9394_array_element_store_strictness.cts (the array element lane), // test_gap_9423_module_init_strictness.ts (the ESM always-strict half). @@ -336,29 +338,10 @@ function sloppyArm(): void { // ---- INHERITED rejecting receivers ---- // - // `+=` against a receiver whose rejection lives on the PROTOTYPE is asserted - // in the sloppy arm only, and the strict twin is spelled `=` instead. That - // asymmetry is deliberate and is NOT what this issue is about: - // - // Perry's `Expr::PropertySet` tail (`js_object_set_field_by_name`) performs - // an OWN-property store. It never runs `OrdinarySetWithOwnDescriptor`'s - // prototype walk, so an inherited non-writable data property, an inherited - // getter-only accessor, and an inherited SETTER are all mishandled the same - // way: the setter never fires and an own property is created instead. That - // is wrong in BOTH modes, it is wrong on unfixed `main` in both modes, and - // it is a missing prototype walk rather than a missing `Throw` flag. - // - // #9459 routes the SLOPPY tail to `js_put_value_set(..., 0)` -- the - // receiver-aware `[[Set]]` that sloppy `o.x = v` has always used -- so the - // sloppy arm below becomes correct as a side effect of getting the - // strictness right. The strict tail keeps its typed-feedback store site - // (`js_typed_feedback_object_set_field_by_name_fast`, a #7480/#5093 gate - // with its own IR tests), so fixing the strict half means retargeting that - // lane, which is a separate change. Filed as #9495; the strict `=` - // twins below pin the shapes so a future fix has a baseline here. - // - // Asserting the sloppy arm alone would be the #9394 mistake, which is why the - // strict side is still exercised -- on the lane that is already correct. + // The rejection lives on the PROTOTYPE, so these exercise the walk of + // ES2024 SS10.1.9.2 as well as the `Throw` flag. Both modes are asserted + // here; the walk itself, across every spelling and lane, is the subject of + // test_gap_9495_strict_inherited_property_set.cts. const inheritedNonWritable: any = Object.create(nonWritableProto()); threw = false; try { @@ -770,18 +753,22 @@ function strictArm(): void { } report("strict non-writable own +=:", threw, nonWritable.x); - // See the note in `sloppyArm`: the `+=` spelling on an inherited receiver is - // a separate, mode-independent defect. `=` is the same three receiver shapes - // on the lane that already walks the prototype chain. + // ---- INHERITED rejecting receivers ---- + // + // The rejection lives on the PROTOTYPE, so these exercise the walk of + // ES2024 SS10.1.9.2 as well as the `Throw` flag. Until #9495 the strict tail + // was an own-property store that skipped the walk, so these three were + // spelled `=` (the lane that already walked) as a baseline; they are the + // `+=` twins of the sloppy arm now. const inheritedNonWritable: any = Object.create(nonWritableProto()); threw = false; try { - inheritedNonWritable.x = 11; + inheritedNonWritable.x += 1; } catch { threw = true; } report( - "strict non-writable inherited =:", + "strict non-writable inherited +=:", threw, hasOwn(inheritedNonWritable, "x"), inheritedNonWritable.x, @@ -799,12 +786,12 @@ function strictArm(): void { const inheritedGetterOnly: any = Object.create(getterOnlyProto()); threw = false; try { - inheritedGetterOnly.x = 21; + inheritedGetterOnly.x += 1; } catch { threw = true; } report( - "strict getter-only inherited =:", + "strict getter-only inherited +=:", threw, hasOwn(inheritedGetterOnly, "x"), inheritedGetterOnly.x, @@ -818,11 +805,11 @@ function strictArm(): void { const withSetter: any = Object.create(setterProto(calls)); threw = false; try { - withSetter.x = 31; + withSetter.x += 1; } catch { threw = true; } - report("strict inherited setter =:", threw, calls.join(","), hasOwn(withSetter, "x")); + report("strict inherited setter +=:", threw, calls.join(","), hasOwn(withSetter, "x")); const noExtend: any = { x: 1 }; Object.preventExtensions(noExtend); diff --git a/test-files/test_gap_9466_shadowed_class_identity.ts b/test-files/test_gap_9466_shadowed_class_identity.ts new file mode 100644 index 0000000000..c675a0cf77 --- /dev/null +++ b/test-files/test_gap_9466_shadowed_class_identity.ts @@ -0,0 +1,257 @@ +// #9466: same-name class declarations at different lexical depths are DISTINCT +// classes. Perry disambiguates them with a compiler-internal registration key +// (`M$0` — see `maybe_rename_colliding_class`), but that key was minted once +// per NAME instead of once per SCOPE: a nested body inherited the enclosing +// body's alias and its `class M` aliased onto the SAME ClassId, so the third +// and every later same-name class silently ran an earlier one's body. Bare +// blocks never ran the disambiguation scan at all, so sibling `{ class X }` +// blocks collided too. +// +// Every arm distinguishes IDENTITY, not just dispatch: the `instanceof` arms +// are the ones that catch a fix that re-splits names without re-splitting +// class ids. + +// --- 1. three depths, three bodies --------------------------------------- +class M { v() { return "top"; } } +function h() { + class M { v() { return "outer"; } } + function h2() { + class M { v() { return "inner"; } } + return new M().v(); + } + return [new M().v(), h2()].join(","); +} +console.log("depths:", new M().v(), h()); + +// --- 1b. three depths with NO top-level declaration of the name ----------- +// The outermost declarer keeps the raw registration key; every nested one +// must still mint its own. +function deepA() { + class D { v() { return "d1"; } } + function deepB() { + class D { v() { return "d2"; } } + function deepC() { + class D { v() { return "d3"; } } + return new D().v(); + } + return [new D().v(), deepC()].join(","); + } + return [new D().v(), deepB()].join(","); +} +console.log("no-top:", deepA()); + +// --- 2a. sibling blocks at module top level ------------------------------- +{ class Blk { v() { return "b1"; } } console.log("blk1:", new Blk().v()); } +{ class Blk { v() { return "b2"; } } console.log("blk2:", new Blk().v()); } +{ class Blk { v() { return "b3"; } } console.log("blk3:", new Blk().v()); } + +// --- 2b. sibling blocks inside a function --------------------------------- +function blocksInFn() { + const out: string[] = []; + { class Q { v() { return "q1"; } } out.push(new Q().v()); } + { class Q { v() { return "q2"; } } out.push(new Q().v()); } + { class Q { v() { return "q3"; } } out.push(new Q().v()); } + return out.join(","); +} +console.log("fn-blocks:", blocksInFn()); + +// --- 2c. same-name classes in sibling functions --------------------------- +function sibA() { class S { v() { return "sA"; } } return new S().v(); } +function sibB() { class S { v() { return "sB"; } } return new S().v(); } +function sibC() { class S { v() { return "sC"; } } return new S().v(); } +console.log("siblings:", sibA(), sibB(), sibC()); + +// --- 2d. if/else branches and try/catch/finally are lexical scopes too ---- +class If1 { v() { return "if-top"; } } +function branches(flag: boolean) { + if (flag) { + class If1 { v() { return "then"; } } + return new If1().v(); + } else { + class If1 { v() { return "else"; } } + return new If1().v(); + } +} +console.log("branches:", branches(true), branches(false), new If1().v()); + +class T1 { v() { return "t-top"; } } +function tryCatchFinally() { + const out: string[] = []; + try { + class T1 { v() { return "try"; } } + out.push(new T1().v()); + throw new Error("x"); + } catch { + class T1 { v() { return "catch"; } } + out.push(new T1().v()); + } finally { + class T1 { v() { return "finally"; } } + out.push(new T1().v()); + } + out.push(new T1().v()); + return out.join(","); +} +console.log("try:", tryCatchFinally()); + +class L { v() { return "L-top"; } } +function loopBody() { + const acc: string[] = []; + for (let i = 0; i < 2; i++) { + class L { v() { return "L-body"; } } + acc.push(new L().v()); + } + acc.push(new L().v()); + return acc.join(","); +} +console.log("loop:", loopBody()); + +// --- 3. shadowed classes captured in closures, called after the block exits +const closures: Array<() => string> = []; +{ + class Cap { v() { return "cap1"; } } + closures.push(() => new Cap().v()); +} +{ + class Cap { v() { return "cap2"; } } + closures.push(() => new Cap().v()); +} +function capFn() { + class Cap { v() { return "cap-fn"; } } + return () => new Cap().v(); +} +closures.push(capFn()); +class Cap { v() { return "cap-top"; } } +closures.push(() => new Cap().v()); +console.log("closures:", closures.map((f) => f()).join(",")); + +// --- 4. instanceof across the shadowing boundary -------------------------- +class P { tag() { return "P-top"; } } +const topP = new P(); +function innerP() { + class P { tag() { return "P-inner"; } } + const p = new P(); + return { + inst: p, + ownIsInner: p instanceof P, + topIsInner: topP instanceof P, + cls: P as any, + }; +} +const r = innerP(); +console.log("io own-inner:", r.ownIsInner); +console.log("io top-is-inner:", r.topIsInner); +console.log("io inner-is-top:", r.inst instanceof P); +console.log("io top-is-top:", topP instanceof P); +console.log("io ctor-identity:", r.cls === P); +console.log("io proto-identity:", Object.getPrototypeOf(r.inst) === P.prototype); +console.log("io tags:", topP.tag(), r.inst.tag()); + +// --- 5. `.name` stays the SOURCE name for every one of them (#9413) ------- +function nameA() { class N {} return N.name; } +function nameB() { class N {} return N.name; } +function nameC() { function d() { class N {} return N.name; } return d(); } +class N {} +console.log("names:", N.name, nameA(), nameB(), nameC()); +console.log("ctor-names:", new M().constructor.name, r.inst.constructor.name); + +// --- 6. subclassing a shadowed class inside the inner scope --------------- +class B { who() { return "B-top"; } } +class SubTop extends B {} +function innerSub() { + class B { who() { return "B-inner"; } } + class Sub extends B { both() { return this.who() + "/sub"; } } + const s = new Sub(); + return [ + s.who(), + s.both(), + String(s instanceof B), + String(s instanceof Sub), + String(s instanceof SubTop), + ].join(","); +} +console.log("sub-top:", new SubTop().who(), new SubTop() instanceof B); +console.log("sub-inner:", innerSub()); + +// --- 7. switch: a bare case statement-list shares ONE switch block scope --- +class Sw { v() { return "sw-top"; } } +function switchBare(k: number) { + switch (k) { + case 1: + class Sw { v() { return "sw-case"; } } + return new Sw().v(); + default: + return "none"; + } +} +console.log("switch-bare:", switchBare(1), switchBare(2), new Sw().v()); + +// A braced case is its own block scope on top of the switch's. +class Sw2 { v() { return "sw2-top"; } } +function switchBraced(k: number) { + switch (k) { + case 1: { class Sw2 { v() { return "c1"; } } return new Sw2().v(); } + case 2: { class Sw2 { v() { return "c2"; } } return new Sw2().v(); } + default: return new Sw2().v(); + } +} +console.log("switch-braced:", switchBraced(1), switchBraced(2), switchBraced(3)); + +// --- 8. loop body: ONE declaration site, so ONE class for every iteration --- +// (the disambiguation is keyed on the declaration's source span, and every +// iteration shares that span). The closures must still hold the inner class +// after the loop exits, and the post-loop `new Lp()` must get the OUTER one. +class Lp { v() { return "lp-top"; } } +function loopSameClass() { + const fs: Array<() => string> = []; + for (let i = 0; i < 3; i++) { + class Lp { v() { return "lp-body"; } } + fs.push(() => new Lp().v()); + } + return fs.map((f) => f()).join(",") + "|" + new Lp().v(); +} +console.log("loop-same-class:", loopSameClass()); + +// NOT covered here: the same loop body where the class CAPTURES the loop +// variable (`class Cp { v() { return "cp" + i; } }`). Node gives one class +// with three environments (`cp0,cp1,cp2`); perry gives `cp3,cp3,cp3` because +// a class carries ONE `RegisterClassCaptures` snapshot, refreshed at +// assignments and returns — neither of which a loop body has. That is the +// class-capture mechanism, not class identity: it reproduces with NO name +// shadowing anywhere (`class Uniq` declared in a loop body, nothing else +// named Uniq in the program) and is byte-identical before and after this fix. +// Reported separately so this fixture keeps discriminating exactly one thing. + +// --- 9. instanceof across a BLOCK boundary, and at the THIRD depth --------- +// Arm 4's instanceof rows sit at two-scope depth, which the name-keyed +// disambiguation already handled; these two are where identity actually broke. +class Ib { tag() { return "ib-top"; } } +const ibTop = new Ib(); +let ibInnerInst: any = null; +let ibInnerCls: any = null; +{ + class Ib { tag() { return "ib-block"; } } + ibInnerInst = new Ib(); + ibInnerCls = Ib; +} +console.log("blk-io same-class:", ibInnerCls === Ib); +console.log("blk-io inner-is-top:", ibInnerInst instanceof Ib); +console.log("blk-io top-is-inner:", ibTop instanceof ibInnerCls); +console.log("blk-io proto:", Object.getPrototypeOf(ibInnerInst) === Ib.prototype); +console.log("blk-io tags:", ibTop.tag(), ibInnerInst.tag()); + +function ioDepth() { + class Id { tag() { return "id-1"; } } + function inner() { + class Id { tag() { return "id-2"; } } + return { inst: new Id(), cls: Id as any }; + } + const deep = inner(); + return [ + deep.inst.tag(), + String(deep.cls === Id), + String(deep.inst instanceof Id), + String(new Id() instanceof deep.cls), + ].join(","); +} +class Id { tag() { return "id-top"; } } +console.log("depth-io:", ioDepth(), new Id().tag()); diff --git a/test-files/test_gap_9495_strict_inherited_property_set.cts b/test-files/test_gap_9495_strict_inherited_property_set.cts new file mode 100644 index 0000000000..a5e6bdb3dc --- /dev/null +++ b/test-files/test_gap_9495_strict_inherited_property_set.cts @@ -0,0 +1,716 @@ +// #9495: strict `o.x += 1` against an INHERITED non-writable data property, +// getter-only accessor or setter skipped the prototype walk entirely. +// +// ES2024 SS10.1.9.2 (OrdinarySetWithOwnDescriptor): when the receiver has no +// own property, `[[Set]]` walks to the parent and lets THE PARENT'S descriptor +// decide -- a non-writable data property rejects, a getter-only accessor +// rejects, a setter RUNS with the original receiver, and only a writable data +// property (or the end of the chain) creates a new own property on the +// receiver. `PutValue` then turns a rejection into a TypeError iff the +// reference is strict. +// +// Perry's strict `Expr::PropertySet` tail (`js_typed_feedback_object_set_ +// field_by_name_fast` -> `js_object_set_field_by_name`) performed an +// OWN-property store: no walk, so no throw, no setter call, and an own +// property materialised where the spec creates none. The strict object-by-name +// arms of `Expr::IndexSet` (`o["x"] += 1`, `o[k] += 1`) had the same tail. +// `o.x = v` was already right (`Expr::PutValueSet` -> `js_put_value_set`), +// and #9459 made the SLOPPY half of these spellings right as a side effect of +// routing them to that same entry. +// +// This is a missing prototype walk, not a missing `Throw` flag -- it is the +// opposite direction from #9422 and a different defect from #9459 -- so it is +// wrong in BOTH modes on unfixed main, and BOTH ARMS ARE ASSERTED here. The +// sloppy arm distinguishes "silent because sloppy" from "silent because the +// walk never ran": an inherited setter must be CALLED in both modes, and no +// own property may appear in either. +// +// `.cts` so that `sloppyArm` is sloppy and `strictArm` opts in with its own +// directive prologue. The two arms are textual duplicates on purpose: a +// function inherits the strictness of the code it is DEFINED in, never its +// caller's, so a shared helper would test one mode twice. +// +// Companions: test_gap_9459_property_set_strictness.cts (the `Throw` flag on +// these spellings, own-property receivers) and +// test_gap_9422_strict_object_store_strictness.cts (the `=` lane). + +function report(name: string, threw: boolean, ...rest: unknown[]): void { + console.log(name, threw ? "TypeError" : "silent", ...rest); +} + +function hasOwn(value: any, key: PropertyKey): boolean { + return Object.prototype.hasOwnProperty.call(value, key); +} + +function nonWritableProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + enumerable: true, + value: 10, + writable: false, + }); + return proto; +} + +function getterOnlyProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + get() { + return 20; + }, + }); + return proto; +} + +function setterProto(calls: any[]): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + get() { + return 30; + }, + set(value: any) { + calls.push(value); + }, + }); + return proto; +} + +class Acc { + calls: any[]; + constructor(calls: any[]) { + this.calls = calls; + } + get x(): number { + return 40; + } + set x(value: number) { + this.calls.push(value); + } +} + +// An INT32-tagged class ref as the receiver (`Klass.prop`), without a declared +// static field: the store lands in the class's dynamic-property bag. (A +// DECLARED `static n` loses `K.n += 1` in both modes on main -- a static-slot +// lane defect, not a prototype walk -- and is #9526.) +class Bag {} + +function sloppyArm(): void { + + let threw = false; + const calls: any[] = []; + + // ---- `o.x += 1` (Expr::PropertySet) against each inherited receiver ---- + const nw: any = Object.create(nonWritableProto()); + threw = false; + try { + nw.x += 1; + } catch { + threw = true; + } + report("sloppy non-writable inherited +=:", threw, hasOwn(nw, "x"), nw.x); + + const go: any = Object.create(getterOnlyProto()); + threw = false; + try { + go.x += 1; + } catch { + threw = true; + } + report("sloppy getter-only inherited +=:", threw, hasOwn(go, "x"), go.x); + + calls.length = 0; + const st: any = Object.create(setterProto(calls)); + threw = false; + try { + st.x += 1; + } catch { + threw = true; + } + report("sloppy inherited setter +=:", threw, calls.join(","), hasOwn(st, "x")); + + // Two levels up: the walk must continue past an empty intermediate object. + calls.length = 0; + const deep: any = Object.create(Object.create(setterProto(calls))); + threw = false; + try { + deep.x += 1; + } catch { + threw = true; + } + report("sloppy inherited setter two levels +=:", threw, calls.join(","), hasOwn(deep, "x")); + + // A class accessor on the prototype chain (compiled setter, not a descriptor). + calls.length = 0; + const viaClass: any = Object.create(new Acc(calls)); + threw = false; + try { + viaClass.x += 1; + } catch { + threw = true; + } + report("sloppy class accessor inherited +=:", threw, calls.join(","), hasOwn(viaClass, "x")); + + // A Proxy on the prototype chain: `OrdinarySetWithOwnDescriptor` forwards to + // `parent.[[Set]](P, V, Receiver)` with the ORIGINAL receiver. + const trapLog: any[] = []; + const viaProxy: any = Object.create( + new Proxy( + { x: 50 }, + { + set(t: any, k: any, v: any, r: any) { + trapLog.push(String(k) + "=" + String(v) + ":" + String(r === viaProxy)); + return true; + }, + }, + ), + ); + threw = false; + try { + viaProxy.x += 1; + } catch { + threw = true; + } + report("sloppy proxy inherited +=:", threw, trapLog.join(","), hasOwn(viaProxy, "x")); + + // ---- logical assignment: the branch that stores ---- + calls.length = 0; + const andAnd: any = Object.create(setterProto(calls)); + threw = false; + try { + andAnd.x &&= 5; + } catch { + threw = true; + } + report("sloppy inherited setter &&=:", threw, calls.join(","), hasOwn(andAnd, "x")); + + const goOr: any = Object.create(getterOnlyProto()); + threw = false; + try { + goOr.x ??= 5; + } catch { + threw = true; + } + // getter returns 20, so `??=` never stores: a control that must be silent. + report("sloppy getter-only inherited ??= (no store):", threw, hasOwn(goOr, "x"), goOr.x); + + // ---- for-of head ---- + calls.length = 0; + const forHead: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forHead.x of [7]) { + } + } catch { + threw = true; + } + report("sloppy inherited setter for-of head:", threw, calls.join(","), hasOwn(forHead, "x")); + + const forNw: any = Object.create(nonWritableProto()); + threw = false; + try { + for (forNw.x of [7]) { + } + } catch { + threw = true; + } + report("sloppy non-writable inherited for-of head:", threw, hasOwn(forNw, "x"), forNw.x); + + // ---- destructuring targets, statement and expression position ---- + calls.length = 0; + const destr: any = Object.create(setterProto(calls)); + threw = false; + try { + [destr.x] = [7]; + } catch { + threw = true; + } + report("sloppy inherited setter [o.x] = arr:", threw, calls.join(","), hasOwn(destr, "x")); + + calls.length = 0; + const destrExpr: any = Object.create(setterProto(calls)); + threw = false; + try { + const seen = ([destrExpr.x] = [7]); + void seen; + } catch { + threw = true; + } + report("sloppy inherited setter ([o.x] = arr) expr:", threw, calls.join(","), hasOwn(destrExpr, "x")); + + calls.length = 0; + const destrObj: any = Object.create(setterProto(calls)); + threw = false; + try { + ({ a: destrObj.x } = { a: 7 }); + } catch { + threw = true; + } + report("sloppy inherited setter ({a: o.x}) = obj:", threw, calls.join(","), hasOwn(destrObj, "x")); + + const destrGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + [destrGo.x] = [7]; + } catch { + threw = true; + } + report("sloppy getter-only inherited [o.x] = arr:", threw, hasOwn(destrGo, "x"), destrGo.x); + + // ---- computed keys (Expr::IndexSet object-by-name lanes) ---- + calls.length = 0; + const lit: any = Object.create(setterProto(calls)); + threw = false; + try { + lit["x"] += 1; + } catch { + threw = true; + } + report("sloppy inherited setter o[\"x\"] +=:", threw, calls.join(","), hasOwn(lit, "x")); + + const litNw: any = Object.create(nonWritableProto()); + threw = false; + try { + litNw["x"] += 1; + } catch { + threw = true; + } + report("sloppy non-writable inherited o[\"x\"] +=:", threw, hasOwn(litNw, "x"), litNw.x); + + calls.length = 0; + const keyed: any = Object.create(setterProto(calls)); + const key = "x"; + threw = false; + try { + keyed[key] += 1; + } catch { + threw = true; + } + report("sloppy inherited setter o[k] +=:", threw, calls.join(","), hasOwn(keyed, "x")); + + const keyedGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + keyedGo[key] += 1; + } catch { + threw = true; + } + report("sloppy getter-only inherited o[k] +=:", threw, hasOwn(keyedGo, "x"), keyedGo.x); + + calls.length = 0; + const anyKeyed: any = Object.create(setterProto(calls)); + const anyKey: any = "x"; + threw = false; + try { + anyKeyed[anyKey] += 1; + } catch { + threw = true; + } + report("sloppy inherited setter o[anyKey] +=:", threw, calls.join(","), hasOwn(anyKeyed, "x")); + + const anyKeyedNw: any = Object.create(nonWritableProto()); + threw = false; + try { + anyKeyedNw[anyKey] += 1; + } catch { + threw = true; + } + report("sloppy non-writable inherited o[anyKey] +=:", threw, hasOwn(anyKeyedNw, "x"), anyKeyedNw.x); + + calls.length = 0; + const forKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forKeyed[key] of [7]) { + } + } catch { + threw = true; + } + report("sloppy inherited setter for-of head computed:", threw, calls.join(","), hasOwn(forKeyed, "x")); + + calls.length = 0; + const destrKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + [destrKeyed[key]] = [7]; + } catch { + threw = true; + } + report("sloppy inherited setter [o[k]] = arr:", threw, calls.join(","), hasOwn(destrKeyed, "x")); + + // ---- accepted stores: the tail must still STORE, and still create own + // properties where the chain does not object ---- + const plain: any = Object.create({ x: 1 }); + threw = false; + try { + plain.x += 41; + } catch { + threw = true; + } + report("sloppy inherited writable data +=:", threw, hasOwn(plain, "x"), plain.x, Object.getPrototypeOf(plain).x); + + const fresh: any = Object.create(setterProto(calls)); + threw = false; + try { + fresh.y ??= 9; + } catch { + threw = true; + } + report("sloppy new key beside inherited accessor ??=:", threw, hasOwn(fresh, "y"), fresh.y); + + // A class ref as the receiver: the receiver-aware `[[Set]]` must keep + // routing an INT32-tagged class value to its dynamic-property bag. + const bag: any = Bag; + threw = false; + try { + bag.sloppy_n = 1; + bag.sloppy_n += 1; + bag["sloppy_n"] += 1; + } catch { + threw = true; + } + report("sloppy class ref +=:", threw, bag.sloppy_n, (Bag as any).sloppy_n); + + // ---- the lanes that were already right, as controls ---- + calls.length = 0; + const assign: any = Object.create(setterProto(calls)); + threw = false; + try { + assign.x = 31; + } catch { + threw = true; + } + report("sloppy inherited setter =:", threw, calls.join(","), hasOwn(assign, "x")); + + calls.length = 0; + const upd: any = Object.create(setterProto(calls)); + threw = false; + try { + upd.x++; + } catch { + threw = true; + } + report("sloppy inherited setter ++:", threw, calls.join(","), hasOwn(upd, "x")); + + const updNw: any = Object.create(nonWritableProto()); + threw = false; + try { + updNw.x++; + } catch { + threw = true; + } + report("sloppy non-writable inherited ++:", threw, hasOwn(updNw, "x"), updNw.x); +} + +function strictArm(): void { + "use strict"; + + let threw = false; + const calls: any[] = []; + + // ---- `o.x += 1` (Expr::PropertySet) against each inherited receiver ---- + const nw: any = Object.create(nonWritableProto()); + threw = false; + try { + nw.x += 1; + } catch { + threw = true; + } + report("strict non-writable inherited +=:", threw, hasOwn(nw, "x"), nw.x); + + const go: any = Object.create(getterOnlyProto()); + threw = false; + try { + go.x += 1; + } catch { + threw = true; + } + report("strict getter-only inherited +=:", threw, hasOwn(go, "x"), go.x); + + calls.length = 0; + const st: any = Object.create(setterProto(calls)); + threw = false; + try { + st.x += 1; + } catch { + threw = true; + } + report("strict inherited setter +=:", threw, calls.join(","), hasOwn(st, "x")); + + // Two levels up: the walk must continue past an empty intermediate object. + calls.length = 0; + const deep: any = Object.create(Object.create(setterProto(calls))); + threw = false; + try { + deep.x += 1; + } catch { + threw = true; + } + report("strict inherited setter two levels +=:", threw, calls.join(","), hasOwn(deep, "x")); + + // A class accessor on the prototype chain (compiled setter, not a descriptor). + calls.length = 0; + const viaClass: any = Object.create(new Acc(calls)); + threw = false; + try { + viaClass.x += 1; + } catch { + threw = true; + } + report("strict class accessor inherited +=:", threw, calls.join(","), hasOwn(viaClass, "x")); + + // A Proxy on the prototype chain: `OrdinarySetWithOwnDescriptor` forwards to + // `parent.[[Set]](P, V, Receiver)` with the ORIGINAL receiver. + const trapLog: any[] = []; + const viaProxy: any = Object.create( + new Proxy( + { x: 50 }, + { + set(t: any, k: any, v: any, r: any) { + trapLog.push(String(k) + "=" + String(v) + ":" + String(r === viaProxy)); + return true; + }, + }, + ), + ); + threw = false; + try { + viaProxy.x += 1; + } catch { + threw = true; + } + report("strict proxy inherited +=:", threw, trapLog.join(","), hasOwn(viaProxy, "x")); + + // ---- logical assignment: the branch that stores ---- + calls.length = 0; + const andAnd: any = Object.create(setterProto(calls)); + threw = false; + try { + andAnd.x &&= 5; + } catch { + threw = true; + } + report("strict inherited setter &&=:", threw, calls.join(","), hasOwn(andAnd, "x")); + + const goOr: any = Object.create(getterOnlyProto()); + threw = false; + try { + goOr.x ??= 5; + } catch { + threw = true; + } + // getter returns 20, so `??=` never stores: a control that must be silent. + report("strict getter-only inherited ??= (no store):", threw, hasOwn(goOr, "x"), goOr.x); + + // ---- for-of head ---- + calls.length = 0; + const forHead: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forHead.x of [7]) { + } + } catch { + threw = true; + } + report("strict inherited setter for-of head:", threw, calls.join(","), hasOwn(forHead, "x")); + + const forNw: any = Object.create(nonWritableProto()); + threw = false; + try { + for (forNw.x of [7]) { + } + } catch { + threw = true; + } + report("strict non-writable inherited for-of head:", threw, hasOwn(forNw, "x"), forNw.x); + + // ---- destructuring targets, statement and expression position ---- + calls.length = 0; + const destr: any = Object.create(setterProto(calls)); + threw = false; + try { + [destr.x] = [7]; + } catch { + threw = true; + } + report("strict inherited setter [o.x] = arr:", threw, calls.join(","), hasOwn(destr, "x")); + + calls.length = 0; + const destrExpr: any = Object.create(setterProto(calls)); + threw = false; + try { + const seen = ([destrExpr.x] = [7]); + void seen; + } catch { + threw = true; + } + report("strict inherited setter ([o.x] = arr) expr:", threw, calls.join(","), hasOwn(destrExpr, "x")); + + calls.length = 0; + const destrObj: any = Object.create(setterProto(calls)); + threw = false; + try { + ({ a: destrObj.x } = { a: 7 }); + } catch { + threw = true; + } + report("strict inherited setter ({a: o.x}) = obj:", threw, calls.join(","), hasOwn(destrObj, "x")); + + const destrGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + [destrGo.x] = [7]; + } catch { + threw = true; + } + report("strict getter-only inherited [o.x] = arr:", threw, hasOwn(destrGo, "x"), destrGo.x); + + // ---- computed keys (Expr::IndexSet object-by-name lanes) ---- + calls.length = 0; + const lit: any = Object.create(setterProto(calls)); + threw = false; + try { + lit["x"] += 1; + } catch { + threw = true; + } + report("strict inherited setter o[\"x\"] +=:", threw, calls.join(","), hasOwn(lit, "x")); + + const litNw: any = Object.create(nonWritableProto()); + threw = false; + try { + litNw["x"] += 1; + } catch { + threw = true; + } + report("strict non-writable inherited o[\"x\"] +=:", threw, hasOwn(litNw, "x"), litNw.x); + + calls.length = 0; + const keyed: any = Object.create(setterProto(calls)); + const key = "x"; + threw = false; + try { + keyed[key] += 1; + } catch { + threw = true; + } + report("strict inherited setter o[k] +=:", threw, calls.join(","), hasOwn(keyed, "x")); + + const keyedGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + keyedGo[key] += 1; + } catch { + threw = true; + } + report("strict getter-only inherited o[k] +=:", threw, hasOwn(keyedGo, "x"), keyedGo.x); + + calls.length = 0; + const anyKeyed: any = Object.create(setterProto(calls)); + const anyKey: any = "x"; + threw = false; + try { + anyKeyed[anyKey] += 1; + } catch { + threw = true; + } + report("strict inherited setter o[anyKey] +=:", threw, calls.join(","), hasOwn(anyKeyed, "x")); + + const anyKeyedNw: any = Object.create(nonWritableProto()); + threw = false; + try { + anyKeyedNw[anyKey] += 1; + } catch { + threw = true; + } + report("strict non-writable inherited o[anyKey] +=:", threw, hasOwn(anyKeyedNw, "x"), anyKeyedNw.x); + + calls.length = 0; + const forKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forKeyed[key] of [7]) { + } + } catch { + threw = true; + } + report("strict inherited setter for-of head computed:", threw, calls.join(","), hasOwn(forKeyed, "x")); + + calls.length = 0; + const destrKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + [destrKeyed[key]] = [7]; + } catch { + threw = true; + } + report("strict inherited setter [o[k]] = arr:", threw, calls.join(","), hasOwn(destrKeyed, "x")); + + // ---- accepted stores: the tail must still STORE, and still create own + // properties where the chain does not object ---- + const plain: any = Object.create({ x: 1 }); + threw = false; + try { + plain.x += 41; + } catch { + threw = true; + } + report("strict inherited writable data +=:", threw, hasOwn(plain, "x"), plain.x, Object.getPrototypeOf(plain).x); + + const fresh: any = Object.create(setterProto(calls)); + threw = false; + try { + fresh.y ??= 9; + } catch { + threw = true; + } + report("strict new key beside inherited accessor ??=:", threw, hasOwn(fresh, "y"), fresh.y); + + // A class ref as the receiver: the receiver-aware `[[Set]]` must keep + // routing an INT32-tagged class value to its dynamic-property bag. + const bag: any = Bag; + threw = false; + try { + bag.strict_n = 1; + bag.strict_n += 1; + bag["strict_n"] += 1; + } catch { + threw = true; + } + report("strict class ref +=:", threw, bag.strict_n, (Bag as any).strict_n); + + // ---- the lanes that were already right, as controls ---- + calls.length = 0; + const assign: any = Object.create(setterProto(calls)); + threw = false; + try { + assign.x = 31; + } catch { + threw = true; + } + report("strict inherited setter =:", threw, calls.join(","), hasOwn(assign, "x")); + + calls.length = 0; + const upd: any = Object.create(setterProto(calls)); + threw = false; + try { + upd.x++; + } catch { + threw = true; + } + report("strict inherited setter ++:", threw, calls.join(","), hasOwn(upd, "x")); + + const updNw: any = Object.create(nonWritableProto()); + threw = false; + try { + updNw.x++; + } catch { + threw = true; + } + report("strict non-writable inherited ++:", threw, hasOwn(updNw, "x"), updNw.x); +} + +sloppyArm(); +strictArm(); diff --git a/test-files/test_gap_9500_exec_callback_completion_order.ts b/test-files/test_gap_9500_exec_callback_completion_order.ts new file mode 100644 index 0000000000..d6941258a6 --- /dev/null +++ b/test-files/test_gap_9500_exec_callback_completion_order.ts @@ -0,0 +1,32 @@ +// #9500 (part 2): `cp.exec` / `cp.execFile` callbacks fire in COMPLETION order, +// not submission order — a child that finishes first calls back first +// regardless of which API launched it or which call came first. (When both +// children finish inside the same loop turn, node's order is a libuv +// batch-delivery artefact, not a rule — the issue's "exec→execFile vs +// execFile→exec" for two instant `echo`s — so only the order with a real +// completion gap is pinned here.) +import * as cp from "node:child_process"; + +function race(label: string, first: () => Promise, second: () => Promise) { + const order: string[] = []; + const tag = (name: string) => (p: Promise) => p.then((out) => { order.push(`${name}:${out}`); }); + return Promise.all([tag("A")(first()), tag("B")(second())]).then(() => { + console.log(label, "→", order.join(" ")); + }); +} +const exec = (cmd: string) => new Promise((res) => cp.exec(cmd, (_e, out) => res(String(out).trim()))); +const execFile = (file: string, args: string[]) => new Promise((res) => cp.execFile(file, args, (_e, out) => res(String(out).trim()))); + +// exec submitted first but slow; execFile submitted second and instant. +race("slow exec, instant execFile", () => exec("sleep 0.3; echo slow"), () => execFile("/bin/echo", ["fast"])) + // execFile submitted first but slow (via sh); exec second and instant. + .then(() => race("slow execFile, instant exec", () => execFile("/bin/sh", ["-c", "sleep 0.3; echo slow"]), () => exec("echo fast"))) + // three children with staggered durations, submitted longest-first. + .then(() => { + const order: string[] = []; + return Promise.all([ + exec("sleep 0.45; echo c").then((o) => { order.push(o); }), + execFile("/bin/sh", ["-c", "sleep 0.3; echo b"]).then((o) => { order.push(o); }), + exec("sleep 0.15; echo a").then((o) => { order.push(o); }), + ]).then(() => console.log("staggered → " + order.join(" "))); + }); diff --git a/test-files/test_gap_9500_mcp_debug_logger_shape.ts b/test-files/test_gap_9500_mcp_debug_logger_shape.ts new file mode 100644 index 0000000000..1161e35ca9 --- /dev/null +++ b/test-files/test_gap_9500_mcp_debug_logger_shape.ts @@ -0,0 +1,132 @@ +// #9500: claude-code's MCP debug logger wrote NOTHING under perry — not even +// the `~/.cache/claude-cli-nodejs//mcp-logs-/` tree — although +// the connect-failure path that feeds it demonstrably ran. This fixture is the +// bundle's exact write shape, de-minified: +// +// * every fs call goes through a wrapper compiled from a `using` declaration +// (esbuild's downlevel): the error is stashed by `var O=A,w=1` in the CATCH +// block and re-thrown from FINALLY by the dispose helper; +// * records go into a buffered writer flushed by a 1 s timer, a size cap, or +// `dispose()`; the logger registers `dispose` in a cleanup set that the +// graceful-shutdown path awaits (raced against a 2 s timer) before +// `process.exit`; +// * the flush's write function is `try { appendFileSync } catch { mkdirSync; +// appendFileSync }` — the ONLY code that ever creates the log directory +// tree, so it relies on the first append THROWING ENOENT. +// +// Under perry the append silently succeeded-without-writing (#9421, fixed for +// this surface by #9491), the recovery arm never ran, and the tree was never +// created. This pins the whole shape end to end: the throw, the `using` +// re-throw, the recursive mkdir recovery, the timer/dispose flush, and the +// exit sequencing. +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +// ── esbuild's `using` downlevel helpers, verbatim shape ────────────────────── +const SYM_DISPOSE = Symbol.dispose || Symbol.for("Symbol.dispose"); +const SYM_ASYNC_DISPOSE = Symbol.asyncDispose || Symbol.for("Symbol.asyncDispose"); +const rz = (q: any[], K: any, _: any) => { + if (K != null) { + if (typeof K !== "object" && typeof K !== "function") throw TypeError('Object expected to be assigned to "using" declaration'); + var z; + if (_) z = K[SYM_ASYNC_DISPOSE]; + if (z === void 0) z = K[SYM_DISPOSE]; + if (typeof z !== "function") throw TypeError("Object not disposable"); + q.push([_, z, K]); + } else if (_) q.push([_]); + return K; +}; +const oz = (q: any[], K: any, _: any) => { + var z = typeof (globalThis as any).SuppressedError === "function" + ? (globalThis as any).SuppressedError + : function (O: any, w: any, $: any, j?: any) { return (j = Error($)), (j.name = "SuppressedError"), (j.error = O), (j.suppressed = w), j; }, + Y = (O: any) => (K = _ ? new z(O, K, "An error was suppressed during disposal") : ((_ = !0), O)), + A: any = (O?: any) => { + while ((O = q.pop())) + try { + var w = O[1] && O[1].call(O[2]); + if (O[0]) return Promise.resolve(w).then(A, ($: any) => (Y($), A())); + } catch ($) { Y($); } + if (_) throw K; + }; + return A(); +}; +// Tracing is off in the shipped CLI: the span tag yields nothing disposable. +function Jw(_s: TemplateStringsArray, ..._v: any[]): any { return undefined; } + +// ── the fs wrapper (`V8()`), the two methods the logger uses ──────────────── +const V8 = { + appendFileSync(q: string, K: string) { let Y: any[] = []; try { const _ = rz(Y, Jw`fs.appendFileSync(${q})`, 0); fs.appendFileSync(q, K); } catch (A) { var O = A, w = 1; } finally { oz(Y, O, w); } }, + mkdirSync(q: string) { let Y: any[] = []; try { const _ = rz(Y, Jw`fs.mkdirSync(${q})`, 0); try { fs.mkdirSync(q, { recursive: true }); } catch ($: any) { if ($?.code !== "EEXIST") throw $; } } catch (A) { var O = A, w = 1; } finally { oz(Y, O, w); } }, +}; + +// ── the buffered writer (`bD6`) ───────────────────────────────────────────── +function bufferedWriter({ writeFn, flushIntervalMs = 1000, maxBufferSize = 100 }: { writeFn: (s: string) => void; flushIntervalMs?: number; maxBufferSize?: number }) { + let buf: string[] = [], timer: any = null, pending: string[] | null = null; + const clear = () => { if (timer) clearTimeout(timer), (timer = null); }; + const flush = () => { if (pending) writeFn(pending.join("")), (pending = null); if (buf.length === 0) return; writeFn(buf.join("")), (buf = []), clear(); }; + const arm = () => { if (!timer) timer = setTimeout(flush, flushIntervalMs); }; + const flushSoon = () => { if (pending) { pending.push(...buf), (buf = []), clear(); return; } const M = buf; buf = [], clear(), (pending = M), setImmediate(() => { const P = pending; if (((pending = null), P)) writeFn(P.join("")); }); }; + return { write(M: string) { buf.push(M), arm(), buf.length >= maxBufferSize && flushSoon(); }, flush, dispose() { flush(); } }; +} + +// ── the cleanup registry (`eq` / `_w8`) ───────────────────────────────────── +const cleanups = new Set<() => Promise>(); +function onCleanup(fn: () => Promise) { cleanups.add(fn); return () => cleanups.delete(fn); } +async function runCleanups() { await Promise.all(Array.from(cleanups).map((q) => q())); } + +// ── the per-file logger cache (`BJ7`) and the MCP sinks ───────────────────── +let recoveries = 0; +const loggers = new Map>(); +function loggerFor(file: string) { + let K = loggers.get(file); + if (!K) { + const dir = path.dirname(file); + const w = bufferedWriter({ + writeFn: (z) => { try { V8.appendFileSync(file, z); } catch { recoveries++; V8.mkdirSync(dir); V8.appendFileSync(file, z); } }, + flushIntervalMs: 1000, + maxBufferSize: 50, + }); + K = { write: (o: unknown) => w.write(JSON.stringify(o) + "\n"), flush: w.flush, dispose: w.dispose }; + loggers.set(file, K); + onCleanup(async () => K?.dispose()); + } + return K; +} +const home = fs.mkdtempSync(path.join(os.tmpdir(), "gap9500-")); +const cacheRoot = path.join(home, ".cache", "claude-cli-nodejs", "-cwd-key"); // nothing under `home` exists yet +const mcpLogPath = (server: string) => path.join(cacheRoot, `mcp-logs-${server}`, "session.jsonl"); +function logMCPDebug(server: string, msg: string) { loggerFor(mcpLogPath(server)).write({ debug: msg, timestamp: "T" }); } +function logMCPError(server: string, err: unknown) { loggerFor(mcpLogPath(server)).write({ error: err instanceof Error ? err.message : String(err), timestamp: "T" }); } + +// ── the connect-failure path, then graceful shutdown (`WK`) ───────────────── +logMCPDebug("alpha", "Connection failed: spawn /bin/echo ENOENT"); +logMCPError("alpha", new Error("Connection failed: spawn /bin/echo ENOENT")); +logMCPDebug("beta", "Connection failed: fetch failed"); +console.log("queued; tree exists before flush:", fs.existsSync(path.join(home, ".cache"))); + +function report() { + for (const server of ["alpha", "beta"]) { + const p = mcpLogPath(server); + const exists = fs.existsSync(p); + const records = exists ? fs.readFileSync(p, "utf8").trim().split("\n").map((l) => JSON.parse(l)) : []; + console.log(`${server}: exists=${exists} records=${records.length}`, records.map((r) => r.debug ?? `ERR ${r.error}`).join(" | ")); + } + console.log("recoveries:", recoveries); + console.log("tree:", fs.existsSync(cacheRoot) ? fs.readdirSync(cacheRoot).sort().join(",") : ""); + fs.rmSync(home, { recursive: true, force: true }); +} +async function gracefulShutdown(code: number) { + let timer: any; + try { + await Promise.race([ + (async () => { try { await runCleanups(); } catch {} })(), + new Promise((_resolve, reject) => { timer = setTimeout((rej: (e: Error) => void) => rej(new Error("cleanup timeout")), 2000, reject); }), + ]); + clearTimeout(timer); + } catch { clearTimeout(timer); } + report(); + process.exit(code); +} +void gracefulShutdown(0); diff --git a/test-files/test_gap_9523_map_set_chain_returns_receiver.ts b/test-files/test_gap_9523_map_set_chain_returns_receiver.ts new file mode 100644 index 0000000000..b2dab61399 --- /dev/null +++ b/test-files/test_gap_9523_map_set_chain_returns_receiver.ts @@ -0,0 +1,89 @@ +// #9523 (second item): `lower_call/property_get/map_set.rs`'s `"set"` arm — the +// path `this.field.set(k, v)` takes when the field is declared `Map` — +// called `js_map_set` as `void` and returned the receiver box it had read from +// its root slot BEFORE the call. `Expr::MapSet` (the `m.set(k, v)` shape on a +// plain local) re-boxes the pointer the helper RETURNS instead. +// +// `js_map_set` returns the receiver as it stands after the insert. When the +// receiver is a `class X extends Map` instance, the runtime resolves it to the +// hidden backing `MapHeader`, roots the receiver (a movable `ObjectHeader`) and +// runs the insert under that root (`map_op_returning_receiver`, #7570). A +// moving minor inside the grow (`ensure_capacity` notes an external side +// allocation, which can trigger one) relocates the receiver, and the helper +// hands back the NEW address. The pre-call box is then a from-space pointer — +// and a chained `.set(a, 1).set(b, 2)` is exactly the consumer of that value: +// the second call dispatches on whatever the first one returned. +// +// The fault needs the minor to fire INSIDE the first `set`'s grow, which is a +// pressure question rather than a deterministic one, so this fixture sweeps +// the nursery fill across rounds and reports how many rounds disagree with +// the specification. Node prints `bad=0` for every round. + +class Registry extends Map {} + +class Holder { + m: Map; + constructor() { + this.m = new Registry(); + } + // The chained shape. The FIRST `.set` is the `"set"` arm; the second one + // consumes its return value. + put(a: string, b: string): number { + this.m.set(a, 1).set(b, 2); + return this.m.size; + } + // The identity contract on its own: `Map.prototype.set` returns `this`. + same(a: string): boolean { + return this.m.set(a, 3) === this.m; + } +} + +// Allocates `n` escaping cells (kept alive in a bounded window) so the nursery +// is filled to a controlled level before the chained set runs. +function fill(n: number): any[] { + let keep: any[] = []; + for (let i = 0; i < n; i++) { + keep.push({ a: i, b: i + 1, c: i + 2, d: i + 3 }); + if (keep.length >= 2048) { + keep = []; + } + } + return keep; +} + +function main(): void { + let bad = 0; + let sameOk = 0; + const rounds = 24; + for (let r = 0; r < rounds; r++) { + const holder = new Holder(); + const reg = new Registry(); + // Fill the initial capacity so the chained set's first insert grows. + for (let i = 0; i < 8; i++) { + reg.set("p" + r + "_" + i, i); + } + holder.m = reg; + // Sweep the fill so successive rounds land the grow at different points of + // the nursery budget. + const keep = fill(120000 + r * 20000); + const size = holder.put("a" + r, "b" + r); + const ok = + size === 10 && + holder.m.get("a" + r) === 1 && + holder.m.get("b" + r) === 2 && + holder.m.get("p" + r + "_7") === 7; + if (!ok) { + bad++; + } + if (holder.same("s" + r)) { + sameOk++; + } + if (keep.length < 0) { + console.log("unreachable"); + } + } + console.log("map-set-chain bad=" + bad); + console.log("set returns receiver=" + sameOk + "/" + rounds); +} + +main(); diff --git a/test-files/test_gap_9523_set_receiver_roots_across_value.ts b/test-files/test_gap_9523_set_receiver_roots_across_value.ts new file mode 100644 index 0000000000..baf1ef4cdb --- /dev/null +++ b/test-files/test_gap_9523_set_receiver_roots_across_value.ts @@ -0,0 +1,134 @@ +// #9523: `Expr::SetHas` and `Expr::SetDelete` unboxed the receiver to a raw +// `i64` handle BEFORE lowering the value expression and consumed that handle +// after it — the #6970 shape their Map twins (`MapGet` / `MapHas` / +// `MapDelete`) were fixed for. +// +// `crates/perry-codegen/src/expr/bigint_set.rs` lowered `set`, masked the +// pointer out of the NaN-box into an SSA register, THEN lowered `value` — +// arbitrary user code that allocates — and only then called `js_set_has` / +// `js_set_delete` with the register. An evacuating young-gen minor inside the +// value's evaluation moves the Set; the register keeps the pre-move address, +// and the runtime helper reads a from-space header. Nothing faults at the +// move: the answer is simply wrong (`has` false for a member, `delete` false +// and the member still present) or the process dies on a recycled cell. +// +// TWO THINGS THIS FIXTURE NEEDS, AND BOTH ARE LOAD-BEARING: +// +// 1. The receiver must be a MODULE-LEVEL binding read from inside a function. +// A function-local Set lives in a shadow slot, and `root_reload.rs` already +// re-materialises a slot load — together with the `bitcast`/`and` unmask +// derived from it — below every collection point that can reach a use, so +// a plain local receiver passes on the unfixed compiler. A module-level +// binding is a `@perry_global_*` load, which that pass deliberately does +// NOT reload ("that population needs rooting, not reloading"), so the raw +// handle is the only copy the consuming call ever sees. +// 2. The value must allocate past the 16 MiB nursery cap +// (`SCAVENGE_NURSERY_CAP_DEFAULT_MB`) with cells that ESCAPE, the +// `test_gap_9417_dispatch_receiver_roots.ts` recipe. A scalar-replaced +// object literal never reaches the arena, and a churn that allocates +// nothing is a test that cannot fail. +// +// Each probe allocates a FRESH Set immediately before the call, so the Set is +// a nursery object when the value's churn runs and the minor that fires there +// is the one that evacuates it. + +let stringSet: Set = new Set(); +let numberSet: Set = new Set(); +let anySet: Set = new Set(); + +function churn(n: number): void { + let keep: any[] = []; + for (let i = 0; i < n; i++) { + const cell = { a: i, b: i + 1, c: i + 2, d: i + 3 }; + keep.push(cell); + if (keep.length >= 1024) { + keep = []; + } + } +} + +// The value expressions. Each one collects (churn) and then builds the key it +// returns, so the key itself is a post-collection object. +function stringKey(k: number): string { + churn(400000); + return "k" + k; +} +function numberKey(k: number): number { + churn(400000); + return k * 3 + 0.5; +} +function anyKey(k: number): any { + churn(400000); + return "a" + k; +} + +// THE GAP, string-typed receiver (`js_set_has_string` / `js_set_delete_string`). +function probeStringSet(k: number): string { + stringSet = new Set(); + stringSet.add("k" + k); + const has = stringSet.has(stringKey(k)); + stringSet = new Set(); + stringSet.add("k" + k); + const del = stringSet.delete(stringKey(k)); + return has + "/" + del + "/" + stringSet.size; +} + +// THE GAP, number-typed receiver (the guarded number arm). +function probeNumberSet(k: number): string { + numberSet = new Set(); + numberSet.add(k * 3 + 0.5); + const has = numberSet.has(numberKey(k)); + numberSet = new Set(); + numberSet.add(k * 3 + 0.5); + const del = numberSet.delete(numberKey(k)); + return has + "/" + del + "/" + numberSet.size; +} + +// THE GAP, untyped value (the generic `js_set_has` / `js_set_delete` arm). +function probeAnySet(k: number): string { + anySet = new Set(); + anySet.add("a" + k); + const has = anySet.has(anyKey(k)); + anySet = new Set(); + anySet.add("a" + k); + const del = anySet.delete(anyKey(k)); + return has + "/" + del + "/" + anySet.size; +} + +// CONTRACT, NOT A GAP: a value that cannot collect leaves no window, so the +// lowering must stay on its unprotected path and still answer correctly. Here +// so a fix that over-roots or mis-orders the group is caught. +function probeControl(k: number): string { + stringSet = new Set(); + const key = "k" + k; + stringSet.add(key); + const has = stringSet.has(key); + const del = stringSet.delete(key); + return has + "/" + del + "/" + stringSet.size; +} + +function main(): void { + const rounds = 6; + let bad = 0; + let first = ""; + for (let k = 0; k < rounds; k++) { + const results = [ + "string " + probeStringSet(k), + "number " + probeNumberSet(k), + "any " + probeAnySet(k), + ]; + for (const r of results) { + if (r.indexOf(" true/true/0") < 0) { + bad++; + if (first === "") { + first = r; + } + } + } + } + console.log("set-receiver bad=" + bad); + console.log("first bad=" + first); + console.log("control=" + probeControl(0)); +} + +main(); diff --git a/test-files/test_gap_date_iso_datetime_local_9449.ts b/test-files/test_gap_date_iso_datetime_local_9449.ts index eefed56a89..e96bbd0642 100644 --- a/test-files/test_gap_date_iso_datetime_local_9449.ts +++ b/test-files/test_gap_date_iso_datetime_local_9449.ts @@ -13,7 +13,11 @@ // (which read back the very digits that were written, in any zone) and // compare the instant against a locally-constructed reference `Date` by // equality. -// Every expectation is measured against `node --experimental-strip-types`. +// #9509 extends the same fixture over the parser tail that follows those date +// and clock fields. Perry used to discard any bytes it did not understand, so +// named US zones and AM/PM were ignored while junk glued to a clock was +// accepted. Every expectation is measured against +// `node --experimental-strip-types`. // ---- absolute rows: a zone designator, or no time at all ------------------- function iso(input: string): void { @@ -30,6 +34,23 @@ iso("2026"); iso("+002026-09-01"); iso("-000001-07-01"); +// Node's implementation-defined date-only surface accepts a bare zone word, +// both separated and directly attached. Missing month/day components retain +// the same defaults as the plain ISO spellings above. +iso("2026 GMT"); +iso("2026-09 GMT"); +iso("2026-09-01 GMT"); +iso("2026-09-01 Z"); +iso("2026-09-01Z"); +iso("2026-09-01 EST"); +iso("2026-09-01 EDT"); +iso("2026-09-01 CST"); +iso("2026-09-01 CDT"); +iso("2026-09-01 MST"); +iso("2026-09-01 MDT"); +iso("2026-09-01 PST"); +iso("2026-09-01 PDT"); + // An explicit designator wins in the date-time form, exactly as before. iso("2026-09-01T10:30Z"); iso("2026-09-01T10:30:45Z"); @@ -60,6 +81,29 @@ iso("2026-09-01 10:30 GMT+05:00"); iso("2026-09-01 10:30 +0500"); iso("2026-09-01 10:30:45 +05:00"); +// V8's legacy zone-name table is fixed-offset and deliberately small. These +// rows also prove that the tail is consumed rather than merely classified. +iso("2026-09-01 10:30 EST"); +iso("2026-09-01 10:30 EDT"); +iso("2026-09-01 10:30 CST"); +iso("2026-09-01 10:30 CDT"); +iso("2026-09-01 10:30 MST"); +iso("2026-09-01 10:30 MDT"); +iso("2026-09-01 10:30 PST"); +iso("2026-09-01 10:30 PDT"); +// Meridiem and zone may occur together, in either order. +iso("2026-09-01 10:30 PM EST"); +iso("2026-09-01 10:30 EST PM"); +iso("2026-09-01 12:30 AM PST"); + +// A word must be token-separated from the clock. These used to be accepted +// because only the leading HH:MM bytes were read and the rest was discarded. +iso("2026-09-01 10:30GMT"); +iso("2026-09-01 10:30EST"); +iso("2026-09-01 10:30PM"); +iso("2026-09-01 10:30 XYZ"); +iso("2026-09-01 10:30:45oops"); + // ---- wall-clock rows: a time, no designator => LOCAL ----------------------- function local(input: string): void { const d = new Date(input); @@ -93,6 +137,17 @@ local("2026-09-01 10:30"); local("2026-09-01 10:30:45"); local("2026-09-01 10:30:45.123"); local("2026-09-01 00:00"); +local("2026-09-01 10:30"); +// The implementation-defined partial forms default the missing day/month to +// one before applying the clock. +local("2026-09 10:30"); +local("2026-09T10:30"); +local("2026T10:30"); +// AM/PM is a clock modifier, including the two 12-hour boundary cases. +local("2026-09-01 10:30 AM"); +local("2026-09-01 10:30 PM"); +local("2026-09-01 12:30 AM"); +local("2026-09-01 12:30 PM"); // A January row and a July row: if the conversion used a FIXED offset rather // than the offset in effect at that instant, one of these two would be wrong // in any zone that observes DST. @@ -115,6 +170,13 @@ sameInstant("2026-09-01T10:30:45", new Date(2026, 8, 1, 10, 30, 45, 0)); sameInstant("2026-09-01T10:30:45.123", new Date(2026, 8, 1, 10, 30, 45, 123)); sameInstant("2026-09-01 10:30", new Date(2026, 8, 1, 10, 30, 0, 0)); sameInstant("2026-09-01 10:30:45.123", new Date(2026, 8, 1, 10, 30, 45, 123)); +sameInstant("2026-09-01 10:30", new Date(2026, 8, 1, 10, 30, 0, 0)); +sameInstant("2026-09 10:30", new Date(2026, 8, 1, 10, 30, 0, 0)); +sameInstant("2026-09T10:30", new Date(2026, 8, 1, 10, 30, 0, 0)); +sameInstant("2026T10:30", new Date(2026, 0, 1, 10, 30, 0, 0)); +sameInstant("2026-09-01 10:30 PM", new Date(2026, 8, 1, 22, 30, 0, 0)); +sameInstant("2026-09-01 12:30 AM", new Date(2026, 8, 1, 0, 30, 0, 0)); +sameInstant("2026-09-01 12:30 PM", new Date(2026, 8, 1, 12, 30, 0, 0)); sameInstant("2026-09-01T00:00", new Date(2026, 8, 1, 0, 0, 0, 0)); sameInstant("2026-09-01T24:00", new Date(2026, 8, 2, 0, 0, 0, 0)); sameInstant("2026-01-15T10:30", new Date(2026, 0, 15, 10, 30, 0, 0));