From 7108dbbcba5339ee239aae8920e1c296d2e3dcd8 Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Sun, 23 Aug 2026 22:20:52 +0200 Subject: [PATCH 01/10] test: remove stale Effect advisory flag (#5890) --- .github/workflows/test.yml | 2 +- tests/release/packages/README.md | 10 +++++----- tests/release/packages/effect-basic/fixture.sh | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6f8f64c144..6ecbefed46 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3169,7 +3169,7 @@ jobs: - name: Run Effect fixture run: | cd tests/release/packages/effect-basic - PERRY_EFFECT_BASIC_ADVISORY=1 PERRY_BIN="$GITHUB_WORKSPACE/target/release/perry" bash fixture.sh + PERRY_BIN="$GITHUB_WORKSPACE/target/release/perry" bash fixture.sh - name: Upload Effect fixture logs if: always() diff --git a/tests/release/packages/README.md b/tests/release/packages/README.md index 89fa7a9a64..0e47787779 100644 --- a/tests/release/packages/README.md +++ b/tests/release/packages/README.md @@ -77,8 +77,8 @@ The harness defines its own pass/fail/skip totals; downstream consumers The `effect-basic` and `ink-link-smoke` fixtures also have named CI jobs in `.github/workflows/test.yml`. They run on release tags, on manual dispatch with `run_extended_tests=true`, and on PRs with the `run-extended-tests` label. The -Effect job opts into a currently advisory compile/run signal with -`PERRY_EFFECT_BASIC_ADVISORY=1`; the default tier-3 sweep records it as SKIP so -known Effect end-to-end gaps do not block unrelated package releases. The Ink -job intentionally stops at compile/link plus symbol inspection; end-to-end Ink -rendering remains tracked separately from the release fixture contract. +Effect fixture compiles and runs in both the default tier-3 sweep and its named +job. The named job remains advisory, matching the tier-3 convention, so broader +Effect end-to-end gaps do not block unrelated changes. The Ink job intentionally +stops at compile/link plus symbol inspection; end-to-end Ink rendering remains +tracked separately from the release fixture contract. diff --git a/tests/release/packages/effect-basic/fixture.sh b/tests/release/packages/effect-basic/fixture.sh index 672c31bae9..875257cbc6 100755 --- a/tests/release/packages/effect-basic/fixture.sh +++ b/tests/release/packages/effect-basic/fixture.sh @@ -14,7 +14,7 @@ fi # instead of skipping behind an opt-in flag. The CI job stays advisory # (`continue-on-error: true`) per the tier-3 convention. The historical # `PERRY_EFFECT_BASIC_ADVISORY=1` gate (added in #4391 when Effect reliably -# failed) is gone; the env var is now a harmless no-op. +# failed) was removed in #6572. fixture_setup "$NAME" || exit 1 fixture_compile_run_diff "$NAME" From 06d4ee94a8ae509e27cc0c4369858a8d5dbe336f Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Sun, 23 Aug 2026 21:19:06 +0200 Subject: [PATCH 02/10] fix(intl): expose Collator compare as an accessor --- crates/perry-runtime/src/intl.rs | 47 +++++++++++-------- .../perry-runtime/src/intl/date_collator.rs | 17 ++++--- ...test_gap_intl_collator_compare_accessor.ts | 29 ++++++++++++ 3 files changed, 64 insertions(+), 29 deletions(-) create mode 100644 test-files/test_gap_intl_collator_compare_accessor.ts diff --git a/crates/perry-runtime/src/intl.rs b/crates/perry-runtime/src/intl.rs index 9c3ca76339..57f26a4a3d 100644 --- a/crates/perry-runtime/src/intl.rs +++ b/crates/perry-runtime/src/intl.rs @@ -54,13 +54,14 @@ mod segmenter; use canon_aliases::canonicalize_unicode_extension_types; pub(crate) use date_collator::{ - collator_bound_compare_thunk, collator_bound_resolved_options_thunk, collator_compare_thunk, - collator_resolved_options_thunk, date_time_format_bound_format_thunk, - date_time_format_bound_range_thunk, date_time_format_bound_range_to_parts_thunk, - date_time_format_bound_resolved_options_thunk, date_time_format_bound_to_parts_thunk, - date_time_format_format_getter_thunk, date_time_format_range_thunk, - date_time_format_range_to_parts_thunk, date_time_format_resolved_options_thunk, - date_time_format_to_parts_thunk, temporal_locale_string, TemporalLocaleCtx, + collator_bound_compare_thunk, collator_bound_resolved_options_thunk, + collator_compare_getter_thunk, collator_resolved_options_thunk, + date_time_format_bound_format_thunk, date_time_format_bound_range_thunk, + date_time_format_bound_range_to_parts_thunk, date_time_format_bound_resolved_options_thunk, + date_time_format_bound_to_parts_thunk, date_time_format_format_getter_thunk, + date_time_format_range_thunk, date_time_format_range_to_parts_thunk, + date_time_format_resolved_options_thunk, date_time_format_to_parts_thunk, + temporal_locale_string, TemporalLocaleCtx, }; pub(crate) use list_relative_plural::{ canonicalize_calendar_id, canonicalize_offset_time_zone, is_valid_offset_time_zone, @@ -163,12 +164,13 @@ const KEY_NF_ROUNDING_INCREMENT: &str = "__intlNfRoundingIncrement"; const KEY_NF_ROUNDING_MODE: &str = "__intlNfRoundingMode"; const KEY_NF_ROUNDING_PRIORITY: &str = "__intlNfRoundingPriority"; const KEY_NF_TRAILING_ZERO: &str = "__intlNfTrailingZero"; -// Hidden [[BoundFormat]] slots. The bound format function is also installed as an -// own `format` property for the native dispatch fast path, but the prototype -// `format` getter reads it from here so user mutation/deletion of the public +// Hidden [[BoundFormat]] / [[BoundCompare]] slots. The bound function is also +// installed as an own property for the native dispatch fast path, but the +// prototype accessor reads it from here so user mutation/deletion of the public // property can't corrupt what the accessor returns. const KEY_NF_BOUND_FORMAT: &str = "__intlNfBoundFormat"; const KEY_DTF_BOUND_FORMAT: &str = "__intlDtfBoundFormat"; +const KEY_COL_BOUND_COMPARE: &str = "__intlColBoundCompare"; const KEY_COL_USAGE: &str = "__intlColUsage"; const KEY_COL_SENSITIVITY: &str = "__intlColSensitivity"; const KEY_COL_IGNORE_PUNCT: &str = "__intlColIgnorePunct"; @@ -1331,12 +1333,20 @@ fn make_instance(closure: *const ClosureHeader, kind: &str, locales: f64, option set_internal_field(obj, KEY_COL_COLLATION, string_value(&collation)); set_internal_field(obj, KEY_COL_NUMERIC, bool_value(numeric)); set_internal_field(obj, KEY_COL_CASE_FIRST, string_value(&case_first)); - install_bound_instance_function( + let compare_fn = install_bound_instance_function( obj, "compare", collator_bound_compare_thunk as *const u8, 2, ); + if !compare_fn.is_null() { + crate::object::set_bound_native_closure_name(compare_fn, ""); + set_internal_field( + obj, + KEY_COL_BOUND_COMPARE, + js_nanbox_pointer(compare_fn as i64), + ); + } install_bound_instance_function( obj, "resolvedOptions", @@ -1846,15 +1856,12 @@ pub fn install_intl_namespace(ns_obj: *mut ObjectHeader) { "Collator", collator_constructor_thunk as *const u8, 0, - &[ - ("compare", collator_compare_thunk as *const u8, 2), - ( - "resolvedOptions", - collator_resolved_options_thunk as *const u8, - 0, - ), - ], - &[], + &[( + "resolvedOptions", + collator_resolved_options_thunk as *const u8, + 0, + )], + &[("compare", collator_compare_getter_thunk as *const u8)], ); install_constructor( ns_obj, diff --git a/crates/perry-runtime/src/intl/date_collator.rs b/crates/perry-runtime/src/intl/date_collator.rs index 3e4d4ad7e0..a2e3cfab78 100644 --- a/crates/perry-runtime/src/intl/date_collator.rs +++ b/crates/perry-runtime/src/intl/date_collator.rs @@ -1668,15 +1668,6 @@ pub(super) fn validate_collator_options(options: f64) { let _ = get_option_value(options, "ignorePunctuation"); } -pub(crate) extern "C" fn collator_compare_thunk( - _closure: *const ClosureHeader, - left: f64, - right: f64, -) -> f64 { - let obj = this_intl_object("compare", KIND_COLLATOR); - collator_compare_object(obj, left, right) -} - pub(crate) extern "C" fn collator_bound_compare_thunk( closure: *const ClosureHeader, left: f64, @@ -1686,6 +1677,14 @@ pub(crate) extern "C" fn collator_bound_compare_thunk( collator_compare_object(obj, left, right) } +/// `get Intl.Collator.prototype.compare` — validate the receiver and return its +/// stable [[BoundCompare]] function. The constructor gives that function the +/// anonymous built-in shape required by ECMA-402 (`name: ""`, `length: 2`). +pub(crate) extern "C" fn collator_compare_getter_thunk(_closure: *const ClosureHeader) -> f64 { + let obj = this_intl_object("compare", KIND_COLLATOR); + get_field(obj, KEY_COL_BOUND_COMPARE) +} + /// Strip the code points a UCA `ignorePunctuation` collator treats as ignorable /// — whitespace and punctuation — so e.g. `compare("", " ")` and /// `compare("", "*")` are 0 (compare/ignorePunctuation.js). diff --git a/test-files/test_gap_intl_collator_compare_accessor.ts b/test-files/test_gap_intl_collator_compare_accessor.ts new file mode 100644 index 0000000000..5aa4082057 --- /dev/null +++ b/test-files/test_gap_intl_collator_compare_accessor.ts @@ -0,0 +1,29 @@ +const descriptor = Object.getOwnPropertyDescriptor(Intl.Collator.prototype, "compare")!; +const getter = descriptor.get!; +const collator = new Intl.Collator("en"); +const compare = collator.compare; + +console.log( + typeof getter, + descriptor.set, + descriptor.enumerable, + descriptor.configurable, +); +console.log( + getter.name, + getter.length, + Object.prototype.hasOwnProperty.call(getter, "prototype"), +); +console.log( + compare === collator.compare, + getter.call(collator) === compare, + compare.name, + compare.length, + Object.prototype.hasOwnProperty.call(compare, "prototype"), +); + +try { + getter.call({}); +} catch (error) { + console.log(error instanceof TypeError); +} From afe8c0b00cf2801a90b804ec900c7715e93be8a9 Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Sun, 23 Aug 2026 23:10:22 +0200 Subject: [PATCH 03/10] fix(codegen): share imported static update storage --- .../perry-codegen/src/expr/member_update.rs | 103 +++++++- .../issue_8654_imported_static_field_cell.rs | 245 ++++++++++++++++++ 2 files changed, 347 insertions(+), 1 deletion(-) create mode 100644 crates/perry/tests/issue_8654_imported_static_field_cell.rs diff --git a/crates/perry-codegen/src/expr/member_update.rs b/crates/perry-codegen/src/expr/member_update.rs index f4b657e507..263c58989f 100644 --- a/crates/perry-codegen/src/expr/member_update.rs +++ b/crates/perry-codegen/src/expr/member_update.rs @@ -43,7 +43,7 @@ use crate::nanbox::POINTER_MASK_I64; use crate::rooting::{self, Repr}; use crate::types::{DOUBLE, I32, I64, I8}; -use super::{lower_expr, FnCtx}; +use super::{emit_root_nanbox_store_on_block, lower_expr, FnCtx}; pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { match expr { @@ -57,6 +57,107 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { prefix, strict, } => { + // #8654: a statically-known class field has one canonical LLVM + // global shared by its defining module and every importer. + // `Class.field = value` already lowers through `StaticFieldSet` + // and updates that global, but `Class.field++` used the generic + // class-object side table instead. The two stores then diverged: + // direct reads kept seeing the initialized global while the + // update read `undefined` from (and wrote `NaN` to) the side + // table. Perform the RMW on the shared global and mirror the new + // value into the side table for genuinely dynamic reads. + let static_class_name = match object.as_ref() { + Expr::ClassRef(class_name) => Some(class_name), + // Imported class bindings are represented as an extern ref + // until codegen resolves their source-module metadata. + Expr::ExternFuncRef { name, .. } => Some(name), + _ => None, + }; + if let Some(class_name) = static_class_name { + let key = (class_name.clone(), property.clone()); + if let Some(global_name) = ctx.static_field_globals.get(&key).cloned() { + let global_ref = format!("@{global_name}"); + let old = ctx.block().load(DOUBLE, &global_ref); + let old_num = ctx.block().call(DOUBLE, "js_to_numeric", &[(DOUBLE, &old)]); + + // A postfix BigInt result remains live across the + // allocating numeric step and the runtime-table mirror. + // Keep it in a function-lifetime root: a temporary root + // cannot be released before its final load escapes this + // lowering, because `root_reload` may otherwise rederive + // that load after the pooled slot has been reused. + let postfix_result = if *prefix { + None + } else { + let old_bits = ctx.block().bitcast_double_to_i64(&old_num); + let top16 = ctx.block().lshr(I64, &old_bits, "48"); + let is_bigint = + ctx.block() + .icmp_eq(I64, &top16, crate::nanbox::BIGINT_TAG_TOP16_I64); + let rooted_bits = ctx.block().select( + crate::types::I1, + &is_bigint, + I64, + &old_bits, + crate::nanbox::TAG_UNDEFINED_I64, + ); + let slot = ctx.func.alloca_entry(I64); + ctx.func.entry_allocas_push_store( + I64, + crate::nanbox::TAG_UNDEFINED_I64, + &slot, + ); + ctx.block().store(I64, &rooted_bits, &slot); + super::root_entry_alloca(ctx, &slot); + Some((slot, is_bigint)) + }; + + let step_arg = match op { + BinaryOp::Sub => "0", + _ => "1", + }; + let new = ctx.block().call( + DOUBLE, + "js_numeric_step", + &[(DOUBLE, &old_num), (I32, step_arg)], + ); + emit_root_nanbox_store_on_block(ctx.block(), &new, &global_ref); + + if let Some(&class_id) = ctx.class_ids.get(class_name) { + let field_idx = ctx.strings.intern(property); + let field = ctx.strings.entry(field_idx); + let bytes_ref = format!("@{}", field.bytes_global); + let byte_len = field.byte_len.to_string(); + let class_id = class_id.to_string(); + // Reload from the registered root after the root + // barrier: a moving collection may rewrite it. + let mirrored = ctx.block().load(DOUBLE, &global_ref); + ctx.block().call_void( + "js_class_register_static_field", + &[ + (I32, &class_id), + (crate::types::PTR, &bytes_ref), + (I64, &byte_len), + (DOUBLE, &mirrored), + ], + ); + } + + return Ok(if let Some((slot, is_bigint)) = postfix_result { + let rooted_bits = ctx.block().load(I64, &slot); + let rooted_result = ctx.block().bitcast_i64_to_double(&rooted_bits); + ctx.block().select( + crate::types::I1, + &is_bigint, + DOUBLE, + &rooted_result, + &old_num, + ) + } else { + ctx.block().load(DOUBLE, &global_ref) + }); + } + } // Scalar replacement fast path: load → fadd/fsub 1.0 → store // on the field's alloca, no heap traffic. if let Expr::LocalGet(id) = object.as_ref() { diff --git a/crates/perry/tests/issue_8654_imported_static_field_cell.rs b/crates/perry/tests/issue_8654_imported_static_field_cell.rs new file mode 100644 index 0000000000..55beba43a4 --- /dev/null +++ b/crates/perry/tests/issue_8654_imported_static_field_cell.rs @@ -0,0 +1,245 @@ +//! Regression for #8654: direct updates of an imported class static must use +//! the same initialized storage as reads in the defining and importing modules. + +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Once; + +const EXPECTED: &str = "primary:ok\nreads:ok\necs:ok\n"; + +const GC_ENV_OVERRIDES: &[&str] = &[ + "PERRY_GEN_GC", + "PERRY_GC_SCAVENGE", + "PERRY_GC_SCAVENGE_NURSERY_MB", + "PERRY_GC_MOVING_SAFEPOINT", + "PERRY_GC_MOVING_LOOP_POLLS", + "PERRY_GC_FORCE_EVACUATE", + "PERRY_GC_VERIFY_EVACUATION", + "PERRY_CONSERVATIVE_STACK_SCAN", + "PERRY_WRITE_BARRIERS", + "PERRY_GC_INCREMENTAL", + "PERRY_GC_HEAP_LIMIT", +]; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn workspace_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("canonicalize workspace root") +} + +fn target_debug_dir() -> PathBuf { + let target = std::env::var_os("CARGO_TARGET_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| workspace_root().join("target")); + if cfg!(windows) { + target.join("x86_64-pc-windows-msvc").join("debug") + } else { + target.join("debug") + } +} + +fn ensure_runtime_archive() { + static BUILD_RUNTIME: Once = Once::new(); + BUILD_RUNTIME.call_once(|| { + let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); + let mut command = Command::new(cargo); + command + .current_dir(workspace_root()) + .arg("build") + .arg("-p") + .arg("perry-runtime-static") + .arg("-p") + .arg("perry-stdlib-static"); + if cfg!(windows) { + command.arg("--target").arg("x86_64-pc-windows-msvc"); + } + let build = command + .output() + .expect("run cargo build of static runtime archives"); + assert!( + build.status.success(), + "cargo build of static runtime archives failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&build.stdout), + String::from_utf8_lossy(&build.stderr) + ); + }); +} + +fn remove_gc_env_overrides(command: &mut Command) { + for key in GC_ENV_OVERRIDES { + command.env_remove(key); + } +} + +fn write_fixture(dir: &Path) { + std::fs::write( + dir.join("base.ts"), + r#" +export class Component { + static _id = 0; + static anchor = { label: "alive" }; +} + +export function makeComponent(constructor: any) { + constructor.id = Component._id++; +} + +export function readDefiningModule() { + return Component._id + ":" + Component.anchor.label; +} + +export abstract class EcsComponent { + static readonly _id: number = 0; + static readonly id: number; +} + +export function registerComponent(constructor: any) { + constructor.id = (EcsComponent)._id++; +} +"#, + ) + .expect("write base fixture"); + + std::fs::write( + dir.join("main.ts"), + r#" +import { + Component, + EcsComponent, + makeComponent, + readDefiningModule, + registerComponent, +} from "./base"; + +class First extends Component {} +class Second extends Component {} + +makeComponent(First); // defining-module post-increment +const importedOld = Component._id++; // importing-module post-increment + +let keep: any[] = []; +for (let i = 0; i < 12000; i++) { + const value = { i, pad: "x" + i }; + if (i % 997 === 0) keep.push(value); +} +(globalThis as any).gc?.(); + +makeComponent(Second); // defining-module update after GC +const DynamicComponent: any = Component; +console.log( + (First as any).id === 0 && importedOld === 1 && + (Second as any).id === 2 && Component._id === 3 + ? "primary:ok" + : "primary:bad", +); +console.log( + readDefiningModule() === "3:alive" && + Component._id === 3 && Component.anchor.label === "alive" && + DynamicComponent._id === 3 + ? "reads:ok" + : "reads:bad", +); + +// The component-registration shape used by perform-ecs: sequential class IDs +// become independent bit masks, and entities leave the matching view again. +class Position extends EcsComponent {} +class Velocity extends EcsComponent {} +registerComponent(Position); +registerComponent(Velocity); +const ids = [(Position as any).id, (Velocity as any).id]; +const masks = [1 << ids[0], 1 << ids[1]]; +const required = masks[0] | masks[1]; +const retained: any[] = []; +for (let i = 0; i < 128; i++) { + const entity = { mask: 0 }; + entity.mask |= masks[0]; + entity.mask |= masks[1]; + if ((entity.mask & required) === required) retained.push(entity); + entity.mask = 0; + if ((entity.mask & required) !== required) { + const index = retained.indexOf(entity); + if (index >= 0) retained.splice(index, 1); + } +} +console.log( + ids[0] === 0 && ids[1] === 1 && + masks[0] === 1 && masks[1] === 2 && retained.length === 0 + ? "ecs:ok" + : "ecs:bad", +); +"#, + ) + .expect("write main fixture"); +} + +fn compile(dir: &Path, label: &str, prebuilt_runtime: bool) -> PathBuf { + let output = dir.join(format!("main_{label}")); + let mut command = Command::new(perry_bin()); + command + .current_dir(dir) + .arg("compile") + .arg("main.ts") + .arg("--no-cache") + .arg("-o") + .arg(&output) + .env_remove("PERRY_NO_AUTO_OPTIMIZE") + .env_remove("PERRY_RUNTIME_DIR"); + remove_gc_env_overrides(&mut command); + if prebuilt_runtime { + ensure_runtime_archive(); + command + .arg("--no-auto-optimize") + .env("PERRY_RUNTIME_DIR", target_debug_dir()); + } + let compile = command.output().expect("run perry compile"); + assert!( + compile.status.success(), + "{label} compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + output +} + +fn run(binary: &Path, dir: &Path, label: &str, force_evacuation: bool) { + let mut command = Command::new(binary); + command.current_dir(dir); + remove_gc_env_overrides(&mut command); + if force_evacuation { + command + .env("PERRY_GC_SCAVENGE", "1") + .env("PERRY_GC_SCAVENGE_NURSERY_MB", "1") + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1") + .env("PERRY_GC_INCREMENTAL", "0"); + } + let run = command.output().expect("run compiled fixture"); + assert!( + run.status.success(), + "{label} run (forced evacuation: {force_evacuation}) failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + EXPECTED, + "{label} output differed (forced evacuation: {force_evacuation})" + ); +} + +#[test] +fn imported_static_post_increment_shares_one_cell_in_all_build_modes() { + let dir = tempfile::tempdir().expect("tempdir"); + write_fixture(dir.path()); + + for (label, prebuilt_runtime) in [("prebuilt", true), ("auto", false)] { + let binary = compile(dir.path(), label, prebuilt_runtime); + run(&binary, dir.path(), label, false); + run(&binary, dir.path(), label, true); + } +} From 705565f2dc614bb999768e10c8c8d2a4b164ce8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 23 Aug 2026 22:26:39 +0200 Subject: [PATCH 04/10] fix: address 5895 review follow-ups --- crates/perry-hir/src/lower/lower_module_fn.rs | 139 +++++++++++++++--- crates/perry-runtime/src/buffer/mod.rs | 4 +- crates/perry-runtime/src/buffer/own_props.rs | 28 ++++ .../perry-runtime/src/node_stream_dispatch.rs | 24 ++- .../src/object/buffer_dispatch.rs | 30 ++-- .../src/object/class_registry/construct.rs | 10 +- .../class_registry/prototype_methods.rs | 26 +++- .../class_registry/prototype_objects.rs | 78 +++++++--- .../perry-runtime/src/object/descriptors.rs | 21 +++ .../src/object/field_get_set/enumeration.rs | 67 +++++++-- .../src/object/global_this/generator.rs | 17 ++- .../src/object/iterator_prototypes.rs | 9 +- .../src/object/polymorphic_index.rs | 4 +- crates/perry-runtime/src/typed_feedback.rs | 2 +- .../perry-runtime/src/typedarray/construct.rs | 89 ++++++++--- crates/perry-runtime/src/value/dyn_index.rs | 2 +- .../test_gap_array_iterator_manual_next.ts | 20 +++ test-files/test_gap_buffer_ops.ts | 52 +++++++ test-files/test_gap_typed_arrays.ts | 19 +++ test-files/test_issue_611_globalthis.ts | 17 +++ .../expected/test_issue_611_globalthis.txt | 6 + 21 files changed, 540 insertions(+), 124 deletions(-) create mode 100644 test-parity/expected/test_issue_611_globalthis.txt diff --git a/crates/perry-hir/src/lower/lower_module_fn.rs b/crates/perry-hir/src/lower/lower_module_fn.rs index fd076d4760..5b5ecf5f97 100644 --- a/crates/perry-hir/src/lower/lower_module_fn.rs +++ b/crates/perry-hir/src/lower/lower_module_fn.rs @@ -7,7 +7,7 @@ //! reach them via `crate::lower::lower_module*` (or the `lib.rs` //! re-exports — `pub use lower::{lower_module, ...}`). -use crate::types::Type; +use crate::types::{LocalId, Type}; use anyhow::Result; use std::collections::HashSet; use swc_ecma_ast as ast; @@ -18,6 +18,114 @@ use crate::lower_types::hoisted_text_codec::{ infer_hoisted_text_codec_var_type, require_literal_specifier, }; +fn reflect_script_var_initializers( + stmts: Vec, + script_var_ids: &HashSet, +) -> Vec { + let mut reflected = Vec::with_capacity(stmts.len()); + for mut stmt in stmts { + match &mut stmt { + Stmt::If { + then_branch, + else_branch, + .. + } => { + *then_branch = + reflect_script_var_initializers(std::mem::take(then_branch), script_var_ids); + if let Some(branch) = else_branch { + *branch = + reflect_script_var_initializers(std::mem::take(branch), script_var_ids); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => { + *body = reflect_script_var_initializers(std::mem::take(body), script_var_ids); + } + Stmt::For { init, body, .. } => { + if let Some(init_stmt) = init.take() { + let mut expanded = + reflect_script_var_initializers(vec![*init_stmt], script_var_ids); + if expanded.len() == 1 { + *init = expanded.pop().map(Box::new); + } else { + // A script `var` initializer can be hoisted out of the + // `for` init slot without changing its one-shot order. + // This makes room for the immediately-following global + // mirror, since HIR's init field holds only one Stmt. + reflected.append(&mut expanded); + } + } + *body = reflect_script_var_initializers(std::mem::take(body), script_var_ids); + } + Stmt::Labeled { body, .. } => { + let inner = std::mem::replace(body, Box::new(Stmt::Break)); + let mut expanded = reflect_script_var_initializers(vec![*inner], script_var_ids); + *body = if expanded.len() == 1 { + Box::new(expanded.pop().expect("one reflected labeled statement")) + } else { + // Non-loop labels are already represented as run-once + // do/while statements. Keep the same representation if a + // direct labeled declaration expands to declaration + + // mirror so `break label` still targets one statement. + Box::new(Stmt::DoWhile { + body: expanded, + condition: Expr::Bool(false), + }) + }; + } + Stmt::Try { + body, + catch, + finally, + } => { + *body = reflect_script_var_initializers(std::mem::take(body), script_var_ids); + if let Some(catch) = catch { + catch.body = reflect_script_var_initializers( + std::mem::take(&mut catch.body), + script_var_ids, + ); + } + if let Some(finally) = finally { + *finally = + reflect_script_var_initializers(std::mem::take(finally), script_var_ids); + } + } + Stmt::Switch { cases, .. } => { + for case in cases { + case.body = reflect_script_var_initializers( + std::mem::take(&mut case.body), + script_var_ids, + ); + } + } + Stmt::Let { .. } + | Stmt::Expr(_) + | Stmt::Return(_) + | Stmt::Break + | Stmt::Continue + | Stmt::LabeledBreak(_) + | Stmt::LabeledContinue(_) + | Stmt::Throw(_) + | Stmt::PreallocateBoxes(_) + | Stmt::PreallocateTdzBoxes(_) + | Stmt::ReleaseBoxes(_) => {} + } + + let global_var = match &stmt { + Stmt::Let { id, name, .. } if script_var_ids.contains(id) => Some((*id, name.clone())), + _ => None, + }; + reflected.push(stmt); + if let Some((id, name)) = global_var { + reflected.push(Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::GlobalThisExpr), + property: name, + value: Box::new(Expr::LocalGet(id)), + })); + } + } + reflected +} + fn should_enable_react_automatic_jsx(name: &str, ast_module: &ast::Module) -> bool { let is_jsx_source = name.ends_with(".tsx") || name.ends_with(".jsx") @@ -1231,28 +1339,13 @@ pub fn lower_module_full( .iter() .filter_map(|name| ctx.lookup_local(name)) .collect(); - // Script `var` bindings are properties of the global object. Insert - // the mirror immediately after each top-level initializer so code - // later in the same script observes the initialized value through - // `globalThis` (ES modules keep their lexical/module binding only). - let mut reflected = Vec::with_capacity(module.init.len()); - for stmt in std::mem::take(&mut module.init) { - let global_var = match &stmt { - Stmt::Let { id, name, .. } if script_var_decl_ids.contains(id) => { - Some((*id, name.clone())) - } - _ => None, - }; - reflected.push(stmt); - if let Some((id, name)) = global_var { - reflected.push(Stmt::Expr(Expr::PropertySet { - object: Box::new(Expr::GlobalThisExpr), - property: name, - value: Box::new(Expr::LocalGet(id)), - })); - } - } - module.init = reflected; + // Script `var` bindings are properties of the global object. Mirror + // every write at its actual execution point, including declarations + // nested in blocks, loops, switch arms and try/catch/finally. Matching + // by LocalId prevents a same-named lexical shadow from leaking onto + // globalThis (ES modules keep their module binding only). + module.init = + reflect_script_var_initializers(std::mem::take(&mut module.init), &script_var_decl_ids); } if ctx.is_entry_module && !is_esm_entry { const RESTRICTED_GLOBAL_NAMES: [&str; 3] = ["undefined", "NaN", "Infinity"]; diff --git a/crates/perry-runtime/src/buffer/mod.rs b/crates/perry-runtime/src/buffer/mod.rs index 4b3076ae1e..b8e5891720 100644 --- a/crates/perry-runtime/src/buffer/mod.rs +++ b/crates/perry-runtime/src/buffer/mod.rs @@ -72,8 +72,8 @@ pub(crate) use detach::{array_buffer_transfer, detach_array_buffer}; pub(crate) use own_props::test_buffer_own_props_owner_count; pub use own_props::{ buffer_define_own_data_prop, buffer_delete_own_prop, buffer_get_own_prop, buffer_has_own_prop, - buffer_own_prop_names, buffer_own_props_possible, buffer_set_own_prop, clear_buffer_own_props, - scan_buffer_own_props_roots_mut, + buffer_own_prop_names, buffer_own_props_possible, buffer_read_own_prop, buffer_set_own_prop, + clear_buffer_own_props, scan_buffer_own_props_roots_mut, }; // ---- Re-exports: #8149 integer-indexed-exotic discrimination ---- diff --git a/crates/perry-runtime/src/buffer/own_props.rs b/crates/perry-runtime/src/buffer/own_props.rs index bf04295c8e..0927583c2e 100644 --- a/crates/perry-runtime/src/buffer/own_props.rs +++ b/crates/perry-runtime/src/buffer/own_props.rs @@ -102,6 +102,34 @@ pub fn buffer_get_own_prop(addr: usize, prop: &str) -> Option { .map(f64::from_bits) } +/// Read an own buffer expando with ordinary `[[Get]]` semantics. +/// +/// Accessor descriptors keep an `undefined` placeholder in the data table so +/// enumeration retains their key order. Readers must therefore consult the +/// accessor table first and invoke the getter instead of returning that +/// placeholder. The receiver and getter are rooted across the user call. +pub fn buffer_read_own_prop(addr: usize, prop: &str) -> Option { + if addr == 0 { + return None; + } + if let Some(accessor) = crate::object::get_accessor_descriptor(addr, prop) { + if accessor.get == 0 { + return Some(f64::from_bits(crate::value::TAG_UNDEFINED)); + } + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(addr as i64)); + let getter = scope.root_nanbox_u64(accessor.get); + let value = unsafe { + crate::object::invoke_accessor_getter( + getter.get_nanbox_u64(), + receiver.get_nanbox_f64(), + ) + }; + return Some(f64::from_bits(value.bits())); + } + buffer_get_own_prop(addr, prop) +} + /// Every own dynamic prop key recorded for `addr`, in insertion-independent /// (sorted) order. /// diff --git a/crates/perry-runtime/src/node_stream_dispatch.rs b/crates/perry-runtime/src/node_stream_dispatch.rs index f664c961d7..d846a52dd5 100644 --- a/crates/perry-runtime/src/node_stream_dispatch.rs +++ b/crates/perry-runtime/src/node_stream_dispatch.rs @@ -223,23 +223,37 @@ fn native_or_plain_key(name: &str, overridden: bool) -> *mut crate::string::Stri /// "no fixed receiver — read IMPLICIT_THIS", which `Function.prototype.call`/ /// `.apply` sets to the borrowed `this`. pub(crate) fn install_event_emitter_prototype_methods(proto: *mut ObjectHeader) { + let scope = crate::gc::RuntimeHandleScope::new(); + let proto = scope.root_raw_mut_ptr(proto); register_stub_arities(); let methods = super::emitter_methods(); - let mut on_method: Option = None; + let mut on_method: Option> = None; for (name, func) in methods { if name == "addListener" { - if let Some(val) = on_method { - js_object_set_field_by_name(proto, hidden_key(name.as_bytes()), val); + if let Some(val) = &on_method { + let key = scope.root_string_ptr(hidden_key(name.as_bytes())); + proto.with_mut_ptr::(|proto| { + key.with_const_ptr::(|key| { + js_object_set_field_by_name(proto, key, val.get_nanbox_f64()) + }) + }); continue; } } let closure = js_closure_alloc(func as *const u8, 1); crate::closure::js_closure_set_capture_ptr(closure, 0, crate::value::TAG_UNDEFINED as i64); - let val = f64::from_bits(JSValue::pointer(closure as *const u8).bits()); + let val = scope.root_nanbox_f64(f64::from_bits( + JSValue::pointer(closure as *const u8).bits(), + )); if name == "on" { on_method = Some(val); } - js_object_set_field_by_name(proto, hidden_key(name.as_bytes()), val); + let key = scope.root_string_ptr(hidden_key(name.as_bytes())); + proto.with_mut_ptr::(|proto| { + key.with_const_ptr::(|key| { + js_object_set_field_by_name(proto, key, val.get_nanbox_f64()) + }) + }); } } diff --git a/crates/perry-runtime/src/object/buffer_dispatch.rs b/crates/perry-runtime/src/object/buffer_dispatch.rs index 0383349399..10d9d27713 100644 --- a/crates/perry-runtime/src/object/buffer_dispatch.rs +++ b/crates/perry-runtime/src/object/buffer_dispatch.rs @@ -386,17 +386,25 @@ pub unsafe fn dispatch_buffer_method( // write methods of a zero-length Buffer with a no-op to MEASURE a packet // before allocating it; dispatching the native method regardless would // write into the empty buffer and throw RangeError [ERR_OUT_OF_RANGE]. - if let Some(own) = crate::buffer::buffer_get_own_prop(addr, method_name) { - let jv = JSValue::from_bits(own.to_bits()); - if jv.is_pointer() { - let ptr = jv.as_pointer::() as usize; - if crate::closure::is_closure_ptr(ptr) { - let prev_this = crate::object::js_implicit_this_set(buf_f64); - let r = crate::closure::js_native_call_value(own, args_ptr, args_len); - crate::object::js_implicit_this_set(prev_this); - return r; - } - } + if crate::buffer::buffer_has_own_prop(addr, method_name) { + let own_scope = crate::gc::RuntimeHandleScope::new(); + let own_receiver = own_scope.root_nanbox_f64(buf_f64); + let own = crate::buffer::buffer_read_own_prop(addr, method_name) + .unwrap_or_else(|| f64::from_bits(crate::value::TAG_UNDEFINED)); + let own = own_scope.root_nanbox_f64(own); + let own_value = own.get_nanbox_f64(); + let own_js = JSValue::from_bits(own_value.to_bits()); + if own_js.is_undefined() || own_js.is_null() { + crate::closure::throw_not_callable(); + } + return match crate::collection_iter::call_with_this_capturing_throw( + own_value, + own_receiver.get_nanbox_f64(), + args, + ) { + Ok(value) => value, + Err(error) => crate::exception::js_throw(error), + }; } let arg_i32 = |i: usize| -> i32 { if i < args.len() { diff --git a/crates/perry-runtime/src/object/class_registry/construct.rs b/crates/perry-runtime/src/object/class_registry/construct.rs index 133aa98e7c..c9c587105c 100644 --- a/crates/perry-runtime/src/object/class_registry/construct.rs +++ b/crates/perry-runtime/src/object/class_registry/construct.rs @@ -988,15 +988,15 @@ pub unsafe extern "C" fn js_new_function_construct( class_cid, class_cid, func_value, args_ptr, args_len, ); } - if extends_target_must_throw(func_value) { - super::super::object_ops::throw_object_type_error(b"is not a constructor"); - } if is_arrow_function_value(func_value) { crate::fs::validate::throw_type_error_with_code( "Arrow function is not a constructor", "ERR_INVALID_ARG_TYPE", ); } + if extends_target_must_throw(func_value) { + super::super::object_ops::throw_object_type_error(b"is not a constructor"); + } let cid = synthetic_class_id_for_function(func_value); // Allocate the instance with the synthetic class id (or 0 if the // value isn't callable). The object starts with no own props; the @@ -1673,8 +1673,8 @@ pub unsafe extern "C" fn js_new_function_construct_with_new_target( | "BigUint64Array" ) { // Validate and initialize the typed-array contents before reading - // a custom newTarget prototype. In particular, a Number/BigInt - // element-type mismatch must throw TypeError without observing a + // a custom newTarget prototype. In particular, invalid Symbol + // element conversion must throw TypeError without observing a // poisoned `newTarget.prototype` getter. let scope = crate::gc::RuntimeHandleScope::new(); let nt_h = scope.root_nanbox_f64(nt); diff --git a/crates/perry-runtime/src/object/class_registry/prototype_methods.rs b/crates/perry-runtime/src/object/class_registry/prototype_methods.rs index 59ffe49062..d09c74d746 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_methods.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_methods.rs @@ -212,17 +212,29 @@ pub(crate) unsafe fn mirror_prototype_method_on_object( if proto.is_null() || name.is_empty() { return; } - let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); - js_object_set_field_by_name(proto, key, f64::from_bits(value_bits)); + let scope = crate::gc::RuntimeHandleScope::new(); + let proto = scope.root_raw_mut_ptr(proto); + let value = scope.root_nanbox_u64(value_bits); + let key = scope.root_string_ptr(crate::string::js_string_from_bytes( + name.as_ptr(), + name.len() as u32, + )); + proto.with_mut_ptr::(|proto| { + key.with_const_ptr::(|key| { + js_object_set_field_by_name(proto, key, value.get_nanbox_f64()) + }) + }); if !enumerable { // `js_object_set_field_by_name` records the default (enumerable) attrs; // override so reflective own-key enumeration skips a defineProperty- // registered non-enumerable method. - set_builtin_property_attrs( - proto as usize, - name.to_string(), - PropertyAttrs::new(true, false, true), - ); + proto.with_mut_ptr::(|proto| { + set_builtin_property_attrs( + proto as usize, + name.to_string(), + PropertyAttrs::new(true, false, true), + ) + }); } } diff --git a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs index 8a33dda7a0..be264fab6f 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs @@ -38,35 +38,50 @@ pub(crate) fn ensure_function_prototype_object( return existing; } + let scope = crate::gc::RuntimeHandleScope::new(); + let func_handle = scope.root_nanbox_f64(func_value); let proto = js_object_alloc(0, 0); if proto.is_null() { return proto; } + let proto_handle = scope.root_raw_mut_ptr(proto); - let constructor_key = - crate::string::js_string_from_bytes(b"constructor".as_ptr(), "constructor".len() as u32); - js_object_set_field_by_name(proto, constructor_key, func_value); - set_builtin_property_attrs( - proto as usize, - "constructor".to_string(), - PropertyAttrs::new(true, false, true), - ); + let constructor_key = scope.root_string_ptr(crate::string::js_string_from_bytes( + b"constructor".as_ptr(), + "constructor".len() as u32, + )); + proto_handle.with_mut_ptr::(|proto| { + constructor_key.with_const_ptr::(|key| { + js_object_set_field_by_name(proto, key, func_handle.get_nanbox_f64()) + }) + }); + proto_handle.with_mut_ptr::(|proto| { + set_builtin_property_attrs( + proto as usize, + "constructor".to_string(), + PropertyAttrs::new(true, false, true), + ) + }); if let Some(object_proto_bits) = global_object_prototype_bits() { - super::super::prototype_chain::object_set_static_prototype( - proto as usize, - object_proto_bits, - ); + proto_handle.with_mut_ptr::(|proto| { + super::super::prototype_chain::object_set_static_prototype( + proto as usize, + object_proto_bits, + ) + }); } - class_prototype_object_root_store(class_id, proto); + proto_handle.with_mut_ptr::(|proto| { + class_prototype_object_root_store(class_id, proto) + }); // #5024: methods registered before the prototype object materialized // (`F.prototype.m = v` typically runs long before any reflective // `F.prototype` read) live only in CLASS_PROTOTYPE_METHODS. Backfill // them as ordinary own properties so enumeration sees them; later // registrations write through via class_prototype_method_root_store. - let registered: Vec<(String, u64)> = { + let registered_bits: Vec<(String, u64)> = { CLASS_PROTOTYPE_METHODS.with(|table| { let guard = table.read().unwrap(); guard @@ -76,9 +91,17 @@ pub(crate) fn ensure_function_prototype_object( .unwrap_or_default() }) }; - for (name, value_bits) in registered { + // The copied side-table values are no longer themselves scanner roots. + // Root the whole snapshot before the first mirrored property can allocate. + let registered: Vec<_> = registered_bits + .into_iter() + .map(|(name, value_bits)| (name, scope.root_nanbox_u64(value_bits))) + .collect(); + for (name, value) in registered { let enumerable = class_prototype_method_is_enumerable(class_id, &name); - unsafe { mirror_prototype_method_on_object(proto, &name, value_bits, enumerable) }; + proto_handle.with_mut_ptr::(|proto| unsafe { + mirror_prototype_method_on_object(proto, &name, value.get_nanbox_u64(), enumerable) + }); } // #5477: the bound `events.EventEmitter` / `EventEmitterAsyncResource` export's @@ -95,17 +118,21 @@ pub(crate) fn ensure_function_prototype_object( // (which arms), so binaries without module imports link neither the // probe nor the EventEmitter prototype machinery. if let Some(ops) = super::super::nm_namespace_ops() { - unsafe { (ops.ee_prototype_install)(func_value, proto) }; + proto_handle.with_mut_ptr::(|proto| unsafe { + (ops.ee_prototype_install)(func_handle.get_nanbox_f64(), proto) + }); } - let func_bits = func_value.to_bits(); + let func_bits = func_handle.get_nanbox_u64(); if (func_bits >> 48) == 0x7FFD { let func_ptr = (func_bits & crate::value::POINTER_MASK) as usize; if func_ptr != 0 { crate::closure::closure_set_dynamic_prop( func_ptr, "prototype", - crate::value::js_nanbox_pointer(proto as i64), + proto_handle.with_mut_ptr::(|proto| { + crate::value::js_nanbox_pointer(proto as i64) + }), ); set_builtin_property_attrs( func_ptr, @@ -115,7 +142,7 @@ pub(crate) fn ensure_function_prototype_object( } } - proto + proto_handle.with_mut_ptr::(|proto| proto) } per_test_global! { @@ -571,8 +598,13 @@ pub(crate) unsafe fn nm_ee_prototype_install( func_value: f64, proto: *mut crate::object::ObjectHeader, ) { + let scope = crate::gc::RuntimeHandleScope::new(); + let func_value = scope.root_nanbox_f64(func_value); + let proto = scope.root_raw_mut_ptr(proto); if let Some((module, method)) = - super::super::native_module::bound_native_callable_module_and_method(func_value) + super::super::native_module::bound_native_callable_module_and_method( + func_value.get_nanbox_f64(), + ) { if module.trim_start_matches("node:") == "events" && matches!( @@ -580,7 +612,9 @@ pub(crate) unsafe fn nm_ee_prototype_install( "EventEmitter" | "EventEmitterAsyncResource" ) { - crate::node_stream::install_event_emitter_prototype_methods(proto); + proto.with_mut_ptr::(|proto| { + crate::node_stream::install_event_emitter_prototype_methods(proto) + }); } } } diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index 79d16cd3f1..f2d88e58fd 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -284,6 +284,27 @@ pub extern "C" fn js_object_get_own_property_descriptor(obj_value: f64, key_valu return f64::from_bits(crate::value::TAG_UNDEFINED); }; let addr = crate::value::js_nanbox_get_pointer(receiver.get_nanbox_f64()) as usize; + // Plain node Buffers are byte-indexed exotic objects but are + // not always marked as Uint8Array owners, so the typed-array + // arm above can legitimately decline them. Their canonical + // in-bounds byte keys still have ordinary element descriptors; + // an out-of-bounds canonical index cannot fall through to an + // expando/accessor of the same spelling. + if crate::buffer::is_byte_indexed_buffer(addr) { + if let Some(index) = property_name_array_index(&name) { + let buf = addr as *const crate::buffer::BufferHeader; + let len = crate::buffer::js_buffer_length(buf).max(0) as u32; + if index < len && index <= i32::MAX as u32 { + return build_data_descriptor( + f64::from(crate::buffer::js_buffer_get(buf, index as i32)), + true, + true, + true, + ); + } + return f64::from_bits(crate::value::TAG_UNDEFINED); + } + } if let Some(accessor) = get_accessor_descriptor(addr, &name) { let attrs = get_property_attrs(addr, &name) .unwrap_or_else(|| PropertyAttrs::new(false, false, false)); diff --git a/crates/perry-runtime/src/object/field_get_set/enumeration.rs b/crates/perry-runtime/src/object/field_get_set/enumeration.rs index 303561d843..cff2772eff 100644 --- a/crates/perry-runtime/src/object/field_get_set/enumeration.rs +++ b/crates/perry-runtime/src/object/field_get_set/enumeration.rs @@ -956,7 +956,7 @@ pub(crate) fn registered_buffer_own_keys(addr: usize) -> Option> { /// The value each key of [`registered_buffer_own_keys`] names: the byte for an /// in-bounds index of a byte-indexed buffer, else the stored own property. pub(crate) fn registered_buffer_own_value(addr: usize, key: &str) -> f64 { - if let Some(v) = crate::buffer::buffer_get_own_prop(addr, key) { + if let Some(v) = crate::buffer::buffer_read_own_prop(addr, key) { return v; } if crate::buffer::is_byte_indexed_buffer(addr) { @@ -973,30 +973,69 @@ pub(crate) fn registered_buffer_own_value(addr: usize, key: &str) -> f64 { /// Build the `Object.keys` / `.values` / `.entries` answer for a registered /// buffer from [`registered_buffer_own_keys`]. fn registered_buffer_enum(addr: usize, what: MapSetEnum) -> Option<*mut ArrayHeader> { - let keys = registered_buffer_own_keys(addr)?; - let mut out = crate::array::js_array_alloc(keys.len().max(1) as u32); + if addr == 0 || !crate::buffer::is_registered_buffer(addr) { + return None; + } + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_raw_mut_ptr(addr as *mut crate::buffer::BufferHeader); + let keys = receiver.with_mut_ptr::(|receiver| { + registered_buffer_own_keys(receiver as usize) + })?; + let out = scope.root_raw_mut_ptr(crate::array::js_array_alloc(keys.len().max(1) as u32)); for key in keys { let key_str = || crate::string::js_string_from_bytes(key.as_ptr(), key.len() as u32); match what { MapSetEnum::Keys => { - out = crate::array::js_array_push(out, JSValue::string_ptr(key_str())); + let key = scope.root_string_ptr(key_str()); + let next = out.with_mut_ptr::(|out| { + key.with_mut_ptr::(|key| { + crate::array::js_array_push(out, JSValue::string_ptr(key)) + }) + }); + out.set_raw_mut_ptr(next); } MapSetEnum::Values => { - out = crate::array::js_array_push_f64(out, registered_buffer_own_value(addr, &key)); + let value = + scope.root_nanbox_f64(receiver.with_mut_ptr::( + |receiver| registered_buffer_own_value(receiver as usize, &key), + )); + let next = out.with_mut_ptr::(|out| { + crate::array::js_array_push_f64(out, value.get_nanbox_f64()) + }); + out.set_raw_mut_ptr(next); } MapSetEnum::Entries => { - let pair = crate::array::js_array_alloc(2); - let pair = crate::array::js_array_push(pair, JSValue::string_ptr(key_str())); - let pair = - crate::array::js_array_push_f64(pair, registered_buffer_own_value(addr, &key)); - out = crate::array::js_array_push( - out, - JSValue::from_bits(JSValue::pointer(pair as *const u8).bits()), - ); + let pair = scope.root_raw_mut_ptr(crate::array::js_array_alloc(2)); + let key_value = scope.root_string_ptr(key_str()); + let next = pair.with_mut_ptr::(|pair| { + key_value.with_mut_ptr::(|key| { + crate::array::js_array_push(pair, JSValue::string_ptr(key)) + }) + }); + pair.set_raw_mut_ptr(next); + let value = + scope.root_nanbox_f64(receiver.with_mut_ptr::( + |receiver| registered_buffer_own_value(receiver as usize, &key), + )); + let next = pair.with_mut_ptr::(|pair| { + crate::array::js_array_push_f64(pair, value.get_nanbox_f64()) + }); + pair.set_raw_mut_ptr(next); + let pair_value = + scope.root_nanbox_f64(pair.with_mut_ptr::(|pair| { + crate::value::js_nanbox_pointer(pair as i64) + })); + let next = out.with_mut_ptr::(|out| { + crate::array::js_array_push( + out, + JSValue::from_bits(pair_value.get_nanbox_u64()), + ) + }); + out.set_raw_mut_ptr(next); } } } - Some(out) + Some(out.with_mut_ptr::(|out| out)) } /// Get the keys of an object as an array of strings. diff --git a/crates/perry-runtime/src/object/global_this/generator.rs b/crates/perry-runtime/src/object/global_this/generator.rs index 4ca1337ff4..8033b1d7df 100644 --- a/crates/perry-runtime/src/object/global_this/generator.rs +++ b/crates/perry-runtime/src/object/global_this/generator.rs @@ -141,16 +141,23 @@ pub(crate) fn wire_async_function_intrinsic_parents() { if ctor == 0 || proto == 0 { return; } + let scope = crate::gc::RuntimeHandleScope::new(); + let ctor = scope.root_raw_mut_ptr(ctor as *mut crate::closure::ClosureHeader); + let proto = scope.root_raw_mut_ptr(proto as *mut ObjectHeader); let function_ctor = js_get_global_this_builtin_value(b"Function".as_ptr(), 8); if crate::value::JSValue::from_bits(function_ctor.to_bits()).is_pointer() { - crate::closure::closure_set_static_prototype(ctor as usize, function_ctor.to_bits()); + ctor.with_mut_ptr::(|ctor| { + crate::closure::closure_set_static_prototype(ctor as usize, function_ctor.to_bits()) + }); } let function_proto = builtin_prototype_value("Function"); if crate::value::JSValue::from_bits(function_proto.to_bits()).is_pointer() { - super::super::prototype_chain::object_set_static_prototype( - proto as usize, - function_proto.to_bits(), - ); + proto.with_mut_ptr::(|proto| { + super::super::prototype_chain::object_set_static_prototype( + proto as usize, + function_proto.to_bits(), + ) + }); } } diff --git a/crates/perry-runtime/src/object/iterator_prototypes.rs b/crates/perry-runtime/src/object/iterator_prototypes.rs index dd1e82195e..a30778ab26 100644 --- a/crates/perry-runtime/src/object/iterator_prototypes.rs +++ b/crates/perry-runtime/src/object/iterator_prototypes.rs @@ -363,7 +363,12 @@ pub(crate) unsafe fn call_overridden_iterator_next( let method = scope.root_nanbox_f64(method); super::js_implicit_this_set(iter.get_nanbox_f64()); - let result = crate::closure::js_native_call_value(method.get_nanbox_f64(), std::ptr::null(), 0); + let result = crate::exception::js_call_catching(|| { + crate::closure::js_native_call_value(method.get_nanbox_f64(), std::ptr::null(), 0) + }); super::js_implicit_this_set(previous.get_nanbox_f64()); - Some(result) + match result { + Ok(value) => Some(value), + Err(error) => crate::exception::js_throw(error), + } } diff --git a/crates/perry-runtime/src/object/polymorphic_index.rs b/crates/perry-runtime/src/object/polymorphic_index.rs index ff14715c1d..c1cedb987a 100644 --- a/crates/perry-runtime/src/object/polymorphic_index.rs +++ b/crates/perry-runtime/src/object/polymorphic_index.rs @@ -208,7 +208,7 @@ pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) -> && crate::buffer::is_non_indexed_buffer_view(raw as usize) { if let Some(key) = crate::buffer::canonical_index_key(idx) { - return crate::buffer::buffer_get_own_prop(raw as usize, &key) + return crate::buffer::buffer_read_own_prop(raw as usize, &key) .unwrap_or_else(|| f64::from_bits(crate::value::TAG_UNDEFINED)); } } @@ -223,7 +223,7 @@ pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) -> // pass wrote into a zero-length Buffer (RangeError // [ERR_OUT_OF_RANGE] at the MySQL handshake). if let Some(name) = buffer_key_name(idx) { - if let Some(v) = crate::buffer::buffer_get_own_prop(raw as usize, &name) { + if let Some(v) = crate::buffer::buffer_read_own_prop(raw as usize, &name) { return v; } // The bound closure keeps the name POINTER and re-reads it at diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index 71557fc166..43bfafe724 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -2095,7 +2095,7 @@ pub extern "C" fn js_typed_feedback_array_index_get_fallback_boxed( && crate::buffer::is_non_indexed_buffer_view(raw_addr) { if let Some(key) = crate::buffer::canonical_index_key(index) { - return crate::buffer::buffer_get_own_prop(raw_addr, &key) + return crate::buffer::buffer_read_own_prop(raw_addr, &key) .unwrap_or_else(|| f64::from_bits(TAG_UNDEFINED)); } } diff --git a/crates/perry-runtime/src/typedarray/construct.rs b/crates/perry-runtime/src/typedarray/construct.rs index 85120ebe08..2c0b3bf382 100644 --- a/crates/perry-runtime/src/typedarray/construct.rs +++ b/crates/perry-runtime/src/typedarray/construct.rs @@ -227,13 +227,13 @@ unsafe fn typed_array_plain_object_values(val: f64) -> Vec { let iter = crate::closure::js_native_call_value(bound_rooted.get_nanbox_f64(), ptr::null(), 0); let iter_rooted = scope.root_nanbox_f64(iter); - let mut raw: Vec = Vec::new(); + let mut raw = Vec::new(); while let Some(v) = crate::collection_iter::iterator_next_value(iter_rooted.get_nanbox_f64()) { - raw.push(v); + raw.push(scope.root_nanbox_f64(v)); } - return raw; + return crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(&raw); } // Array-like path. let len_val = object_like_get(rooted.get_nanbox_f64(), "length"); @@ -250,11 +250,11 @@ unsafe fn typed_array_plain_object_values(val: f64) -> Vec { throw_range_error(format!("Invalid typed array length: {}", len as u64).as_bytes()); } let len = len as u32; - let mut raw: Vec = Vec::with_capacity(len as usize); + let mut raw = Vec::with_capacity(len as usize); for k in 0..len { - raw.push(object_like_get(rooted.get_nanbox_f64(), &k.to_string())); + raw.push(scope.root_nanbox_f64(object_like_get(rooted.get_nanbox_f64(), &k.to_string()))); } - raw + crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(&raw) } /// Collect the raw (uncoerced) source values for `%TypedArray%.from(source)`: @@ -298,10 +298,16 @@ pub(crate) unsafe fn typed_array_from_source_raw_values(val: f64) -> Vec { // `js_array_from_value` allocated; re-root on the RESULT, which is the // value the element reads below must observe. let arr_rooted = scope.root_raw_const_ptr(arr); - let len = crate::array::js_array_length(arr_rooted.get_raw_const_ptr()); - (0..len) - .map(|i| crate::array::js_array_get_f64(arr_rooted.get_raw_const_ptr(), i)) - .collect() + let len = arr_rooted.with_const_ptr::(|arr| crate::array::js_array_length(arr)); + let raw: Vec<_> = (0..len) + .map(|i| { + scope.root_nanbox_f64( + arr_rooted + .with_const_ptr::(|arr| crate::array::js_array_get_f64(arr, i)), + ) + }) + .collect(); + crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(&raw) } /// Coerce a snapshot of raw element values per `kind` (observable, may throw) @@ -417,21 +423,56 @@ pub extern "C" fn js_typed_array_new_from_array( return typed_array_alloc(kind, 0); } rooted.set_raw_const_ptr(arr); - // InitializeTypedArrayFromArrayLike obtains and drives the source's - // iterator. Going through the real iterator protocol matters even for a - // dense Array: user code can replace Array.prototype[Symbol.iterator] or - // %ArrayIteratorPrototype%.next, and construction must observe either. - // Collect the raw values first and only then coerce them, retaining the - // mutation/snapshot rule described by `typed_array_from_snapshot`. - let source = rooted - .with_const_ptr::(|source| crate::value::js_nanbox_pointer(source as i64)); - let iter = crate::symbol::js_get_iterator(source); - let iter_rooted = scope.root_nanbox_f64(iter); + // GetMethod(source, @@iterator) is observable even for a dense Array. A + // callable method drives the real iterator protocol; undefined/null falls + // back to InitializeTypedArrayFromArrayLike. In particular, deleting + // Array.prototype[Symbol.iterator] must not make `new Uint8Array(array)` + // throw. Collect raw values before coercion, retaining the mutation/ + // snapshot rule described by `typed_array_from_snapshot`. + let iterator_symbol = crate::symbol::well_known_symbol("iterator"); + let method = if iterator_symbol.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + let symbol = + f64::from_bits(crate::value::JSValue::pointer(iterator_symbol as *const u8).bits()); + unsafe { + crate::symbol::js_object_get_symbol_property( + rooted.with_const_ptr::(|source| { + crate::value::js_nanbox_pointer(source as i64) + }), + symbol, + ) + } + }; + let method_bits = method.to_bits(); let mut raw = Vec::new(); - while let Some(value) = - crate::collection_iter::iterator_next_value(iter_rooted.get_nanbox_f64()) - { - raw.push(scope.root_nanbox_f64(value)); + if method_bits == crate::value::TAG_UNDEFINED || method_bits == crate::value::TAG_NULL { + let len = rooted.with_const_ptr::(|arr| crate::array::js_array_length(arr)); + for index in 0..len { + raw.push( + scope.root_nanbox_f64(rooted.with_const_ptr::(|arr| { + crate::array::js_array_get_f64(arr, index) + })), + ); + } + } else { + let method = scope.root_nanbox_f64(method); + let iter = match crate::collection_iter::call_with_this_capturing_throw( + method.get_nanbox_f64(), + rooted.with_const_ptr::(|source| { + crate::value::js_nanbox_pointer(source as i64) + }), + &[], + ) { + Ok(iter) => iter, + Err(error) => crate::exception::js_throw(error), + }; + let iter_rooted = scope.root_nanbox_f64(iter); + while let Some(value) = + crate::collection_iter::iterator_next_value(iter_rooted.get_nanbox_f64()) + { + raw.push(scope.root_nanbox_f64(value)); + } } unsafe { typed_array_from_rooted_snapshot(kind, &raw) } } diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index d5252539c2..cc668bcbc5 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -291,7 +291,7 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { && crate::buffer::is_non_indexed_buffer_view(raw_ptr) { if let Some(key) = crate::buffer::canonical_index_key(index) { - return crate::buffer::buffer_get_own_prop(raw_ptr, &key) + return crate::buffer::buffer_read_own_prop(raw_ptr, &key) .unwrap_or_else(|| f64::from_bits(TAG_UNDEFINED)); } } diff --git a/test-files/test_gap_array_iterator_manual_next.ts b/test-files/test_gap_array_iterator_manual_next.ts index 41bacce27e..6f6f3afa50 100644 --- a/test-files/test_gap_array_iterator_manual_next.ts +++ b/test-files/test_gap_array_iterator_manual_next.ts @@ -59,3 +59,23 @@ console.log(JSON.stringify([].values().next())); // {"done":true} const e2 = [100, 200].entries(); const [, first] = e2.next().value; console.log(first); // 100 + +// (7) A user replacement can throw. The iterator dispatcher temporarily +// installs the iterator as the implicit receiver; abrupt completion must +// restore the surrounding method's receiver before its catch block resumes. +const iteratorPrototype: any = Object.getPrototypeOf([][Symbol.iterator]()); +const originalNext = iteratorPrototype.next; +iteratorPrototype.next = function () { + throw new Error("replacement next"); +}; +const outerReceiver = { + marker: "outer", + run(this: any) { + try { + [1].values().next(); + } catch (_error) {} + return this.marker; + }, +}; +console.log("throw restores this:", outerReceiver.run()); // outer +iteratorPrototype.next = originalNext; diff --git a/test-files/test_gap_buffer_ops.ts b/test-files/test_gap_buffer_ops.ts index 258dcdd5bb..003cf03a2c 100644 --- a/test-files/test_gap_buffer_ops.ts +++ b/test-files/test_gap_buffer_ops.ts @@ -95,6 +95,58 @@ console.log("utf8 len:", Buffer.from("hello").length); console.log("emoji len:", Buffer.from("😀").length); console.log("empty len:", Buffer.from("").length); +// --- Buffer exotic descriptors and accessor reads --- +const descriptorBuffer = Buffer.from([7]); +const byteDescriptor = Object.getOwnPropertyDescriptor(descriptorBuffer, "0"); +console.log( + "byte descriptor:", + byteDescriptor?.value, + byteDescriptor?.writable, + byteDescriptor?.enumerable, + byteDescriptor?.configurable, +); + +const accessorBuffer: any = Buffer.from([1, 2]); +let accessorHits = 0; +Object.defineProperty(accessorBuffer, "computed", { + enumerable: true, + configurable: true, + get() { + accessorHits++; + return 41; + }, +}); +const computedKey: any = "computed"; +console.log("buffer computed getter:", accessorBuffer[computedKey], accessorHits); +console.log("buffer values getter:", Object.values(accessorBuffer).join(","), accessorHits); +console.log( + "buffer entries getter:", + JSON.stringify(Object.entries(accessorBuffer)), + accessorHits, +); + +Object.defineProperty(accessorBuffer, "readUInt8", { + configurable: true, + get() { + accessorHits++; + return function (_offset: number) { + return this === accessorBuffer ? 99 : -1; + }; + }, +}); +console.log("buffer accessor method:", accessorBuffer.readUInt8(0), accessorHits); + +const accessorView: any = new DataView(new ArrayBuffer(1)); +Object.defineProperty(accessorView, "0", { + enumerable: true, + configurable: true, + get() { + return 77; + }, +}); +const zeroKey: any = 0; +console.log("dataview numeric getter:", accessorView[zeroKey]); + // --- Buffer.isBuffer --- (not yet in codegen: BufferIsBuffer) // console.log("isBuffer buf:", Buffer.isBuffer(Buffer.from("x"))); diff --git a/test-files/test_gap_typed_arrays.ts b/test-files/test_gap_typed_arrays.ts index 7c74cd8148..e457428dd7 100644 --- a/test-files/test_gap_typed_arrays.ts +++ b/test-files/test_gap_typed_arrays.ts @@ -10,6 +10,25 @@ const u2 = new Uint8Array([10, 20, 30, 40, 50]); console.log("from array:", u2[0], u2[1], u2[2], u2[3], u2[4]); console.log("from array length:", u2.length); +// GetMethod(source, @@iterator) falls back to array-like reads when the +// method is undefined or null. +const iteratorDescriptor = Object.getOwnPropertyDescriptor( + Array.prototype, + Symbol.iterator, +)!; +delete (Array.prototype as any)[Symbol.iterator]; +const noPrototypeIterator = new Uint8Array([6, 7]); +Object.defineProperty(Array.prototype, Symbol.iterator, iteratorDescriptor); +console.log( + "from array without prototype iterator:", + noPrototypeIterator[0], + noPrototypeIterator[1], +); +const nullIteratorSource: any = [8, 9]; +nullIteratorSource[Symbol.iterator] = null; +const nullIterator = new Uint8Array(nullIteratorSource); +console.log("from array with null iterator:", nullIterator[0], nullIterator[1]); + // --- Uint8Array read/write --- const u3 = new Uint8Array(3); u3[0] = 100; diff --git a/test-files/test_issue_611_globalthis.ts b/test-files/test_issue_611_globalthis.ts index 11f8852bb5..31989f90e1 100644 --- a/test-files/test_issue_611_globalthis.ts +++ b/test-files/test_issue_611_globalthis.ts @@ -17,3 +17,20 @@ const store = (globalThis as any)[storeId] as Map; console.log("typeof store:", typeof store); store.set("k", "v"); console.log("store.get('k'):", store.get("k")); + +// Script-level `var` remains a global-object binding even when its initializer +// executes inside a compound statement. +if (true) { + var nestedScriptVar = 42; +} +console.log("nested var:", (globalThis as any).nestedScriptVar); +try { + var caughtScriptVar = "try"; +} finally { + var finalScriptVar = "finally"; +} +console.log( + "try/finally vars:", + (globalThis as any).caughtScriptVar, + (globalThis as any).finalScriptVar, +); diff --git a/test-parity/expected/test_issue_611_globalthis.txt b/test-parity/expected/test_issue_611_globalthis.txt new file mode 100644 index 0000000000..ce2fcb2987 --- /dev/null +++ b/test-parity/expected/test_issue_611_globalthis.txt @@ -0,0 +1,6 @@ +v: hello +typeof v: string +typeof store: object +store.get('k'): v +nested var: 42 +try/finally vars: try finally From fc362103a48fdb387da22eceba4edab0f6cb18b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 23 Aug 2026 23:01:03 +0200 Subject: [PATCH 05/10] fix: address final 5895 review findings --- crates/perry-hir/src/lower/lower_module_fn.rs | 79 ++++++++++++++----- crates/perry-runtime/src/array/indexing.rs | 29 ++++++- crates/perry-runtime/src/array/mod.rs | 1 + crates/perry-runtime/src/array/sort.rs | 8 +- .../class_registry/prototype_objects.rs | 3 +- .../src/object/global_this/generator.rs | 9 ++- .../perry-runtime/src/typedarray/construct.rs | 10 +-- test-files/test_gap_typed_arrays.ts | 24 ++++++ test-files/test_issue_611_globalthis.ts | 18 +++++ .../expected/test_issue_611_globalthis.txt | 2 + 10 files changed, 152 insertions(+), 31 deletions(-) diff --git a/crates/perry-hir/src/lower/lower_module_fn.rs b/crates/perry-hir/src/lower/lower_module_fn.rs index 5b5ecf5f97..4d05b47bb7 100644 --- a/crates/perry-hir/src/lower/lower_module_fn.rs +++ b/crates/perry-hir/src/lower/lower_module_fn.rs @@ -9,7 +9,7 @@ use crate::types::{LocalId, Type}; use anyhow::Result; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use swc_ecma_ast as ast; use super::*; @@ -20,7 +20,7 @@ use crate::lower_types::hoisted_text_codec::{ fn reflect_script_var_initializers( stmts: Vec, - script_var_ids: &HashSet, + script_vars: &HashMap, ) -> Vec { let mut reflected = Vec::with_capacity(stmts.len()); for mut stmt in stmts { @@ -31,19 +31,20 @@ fn reflect_script_var_initializers( .. } => { *then_branch = - reflect_script_var_initializers(std::mem::take(then_branch), script_var_ids); + reflect_script_var_initializers(std::mem::take(then_branch), script_vars); if let Some(branch) = else_branch { - *branch = - reflect_script_var_initializers(std::mem::take(branch), script_var_ids); + *branch = reflect_script_var_initializers(std::mem::take(branch), script_vars); } } Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => { - *body = reflect_script_var_initializers(std::mem::take(body), script_var_ids); + *body = reflect_script_var_initializers(std::mem::take(body), script_vars); } - Stmt::For { init, body, .. } => { + Stmt::For { + init, update, body, .. + } => { if let Some(init_stmt) = init.take() { let mut expanded = - reflect_script_var_initializers(vec![*init_stmt], script_var_ids); + reflect_script_var_initializers(vec![*init_stmt], script_vars); if expanded.len() == 1 { *init = expanded.pop().map(Box::new); } else { @@ -54,12 +55,44 @@ fn reflect_script_var_initializers( reflected.append(&mut expanded); } } - *body = reflect_script_var_initializers(std::mem::take(body), script_var_ids); + if let Some(update_expr) = update.take() { + let mut updated_globals: Vec<_> = script_vars + .iter() + .filter(|(id, _)| expr_updates_local(&update_expr, **id)) + .map(|(id, name)| (*id, name.clone())) + .collect(); + updated_globals.sort_unstable_by_key(|(id, _)| *id); + if updated_globals.is_empty() { + *update = Some(update_expr); + } else { + let mut sequence = Vec::with_capacity(1 + updated_globals.len()); + sequence.push(update_expr); + sequence.extend(updated_globals.into_iter().map(|(id, name)| { + Expr::PropertySet { + object: Box::new(Expr::GlobalThisExpr), + property: name, + value: Box::new(Expr::LocalGet(id)), + } + })); + *update = Some(Expr::Sequence(sequence)); + } + } + *body = reflect_script_var_initializers(std::mem::take(body), script_vars); } Stmt::Labeled { body, .. } => { let inner = std::mem::replace(body, Box::new(Stmt::Break)); - let mut expanded = reflect_script_var_initializers(vec![*inner], script_var_ids); - *body = if expanded.len() == 1 { + let mut expanded = reflect_script_var_initializers(vec![*inner], script_vars); + *body = if expanded.len() > 1 && matches!(expanded.last(), Some(Stmt::For { .. })) { + // A reflected `for (var ...)` init expands to the init, + // its global mirror, and the loop. Keep those one-shot + // statements immediately before the labeled loop: wrapping + // the whole expansion in `do { ... } while (false)` would + // make `continue label` target the wrapper instead of the + // original `for` statement. + let loop_stmt = expanded.pop().expect("reflected labeled for statement"); + reflected.append(&mut expanded); + Box::new(loop_stmt) + } else if expanded.len() == 1 { Box::new(expanded.pop().expect("one reflected labeled statement")) } else { // Non-loop labels are already represented as run-once @@ -77,23 +110,23 @@ fn reflect_script_var_initializers( catch, finally, } => { - *body = reflect_script_var_initializers(std::mem::take(body), script_var_ids); + *body = reflect_script_var_initializers(std::mem::take(body), script_vars); if let Some(catch) = catch { catch.body = reflect_script_var_initializers( std::mem::take(&mut catch.body), - script_var_ids, + script_vars, ); } if let Some(finally) = finally { *finally = - reflect_script_var_initializers(std::mem::take(finally), script_var_ids); + reflect_script_var_initializers(std::mem::take(finally), script_vars); } } Stmt::Switch { cases, .. } => { for case in cases { case.body = reflect_script_var_initializers( std::mem::take(&mut case.body), - script_var_ids, + script_vars, ); } } @@ -111,7 +144,7 @@ fn reflect_script_var_initializers( } let global_var = match &stmt { - Stmt::Let { id, name, .. } if script_var_ids.contains(id) => Some((*id, name.clone())), + Stmt::Let { id, name, .. } if script_vars.contains_key(id) => Some((*id, name.clone())), _ => None, }; reflected.push(stmt); @@ -126,6 +159,14 @@ fn reflect_script_var_initializers( reflected } +fn expr_updates_local(expr: &Expr, target: LocalId) -> bool { + match expr { + Expr::LocalSet(id, _) | Expr::Update { id, .. } => *id == target, + Expr::Sequence(exprs) => exprs.iter().any(|expr| expr_updates_local(expr, target)), + _ => false, + } +} + fn should_enable_react_automatic_jsx(name: &str, ast_module: &ast::Module) -> bool { let is_jsx_source = name.ends_with(".tsx") || name.ends_with(".jsx") @@ -1334,10 +1375,10 @@ pub fn lower_module_full( let is_esm_entry = !module.imports.is_empty() || !module.exports.is_empty() || module.has_top_level_await; if ctx.is_entry_module && !is_esm_entry && module.references_global_this { - let script_var_decl_ids: HashSet<_> = ctx + let script_vars: HashMap<_, _> = ctx .script_var_decl_names .iter() - .filter_map(|name| ctx.lookup_local(name)) + .filter_map(|name| ctx.lookup_local(name).map(|id| (id, name.clone()))) .collect(); // Script `var` bindings are properties of the global object. Mirror // every write at its actual execution point, including declarations @@ -1345,7 +1386,7 @@ pub fn lower_module_full( // by LocalId prevents a same-named lexical shadow from leaking onto // globalThis (ES modules keep their module binding only). module.init = - reflect_script_var_initializers(std::mem::take(&mut module.init), &script_var_decl_ids); + reflect_script_var_initializers(std::mem::take(&mut module.init), &script_vars); } if ctx.is_entry_module && !is_esm_entry { const RESTRICTED_GLOBAL_NAMES: [&str; 3] = ["undefined", "NaN", "Infinity"]; diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 61efb5c448..06ddb49ffb 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -353,12 +353,13 @@ pub(crate) fn array_spec_get(arr: *const ArrayHeader, index: u32) -> f64 { return TAG_UNDEFINED_F64; } unsafe { + let receiver = crate::value::js_nanbox_pointer(arr as i64); if array_has_own_index(arr, index) { return js_array_get_f64(arr, index); } if let Some(proto_arr) = array_custom_array_prototype(arr) { if index < (*proto_arr).length && array_has_own_index(proto_arr, index) { - return js_array_get_f64(proto_arr, index); + return array_inherited_index_get(proto_arr, index, receiver); } } if ARRAY_PROTO_HAS_INDEX.load(Ordering::Relaxed) { @@ -366,19 +367,41 @@ pub(crate) fn array_spec_get(arr: *const ArrayHeader, index: u32) -> f64 { if proto != 0 && proto != arr as usize { let proto_arr = proto as *const ArrayHeader; if index < (*proto_arr).length && array_has_own_index(proto_arr, index) { - return js_array_get_f64(proto_arr, index); + return array_inherited_index_get(proto_arr, index, receiver); } } } if OBJECT_PROTO_HAS_INDEX.load(Ordering::Relaxed) && crate::array::object_prototype_has_index_prop(index) { - return crate::array::sort_object_prototype_index_get(index); + return crate::array::sort_object_prototype_index_get_with_receiver(index, receiver); } TAG_UNDEFINED_F64 } } +/// Read an own indexed property from an Array prototype while preserving the +/// original receiver for an inherited accessor's `this` value. +unsafe fn array_inherited_index_get( + proto_arr: *const ArrayHeader, + index: u32, + receiver: f64, +) -> f64 { + if array_object_flags(proto_arr) & crate::gc::OBJ_FLAG_ARRAY_DESCRIPTORS != 0 { + if let Some(acc) = + crate::object::get_accessor_descriptor(proto_arr as usize, &index.to_string()) + { + if acc.get != 0 { + return f64::from_bits( + crate::object::invoke_accessor_getter(acc.get, receiver).bits(), + ); + } + return f64::from_bits(crate::value::TAG_UNDEFINED); + } + } + js_array_get_f64(proto_arr, index) +} + fn array_get_property_by_key(arr: *const ArrayHeader, key: *const crate::StringHeader) -> f64 { // #7891: an erased Array declaration can feed this ABI a heap StringHeader. // The receiver arrived unboxed and no longer carries STRING_TAG, so recover diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 39004ced86..5447f78dbf 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -161,6 +161,7 @@ pub(crate) use self::prototype_addr::{ }; pub(crate) use self::sort::object_prototype_has_index_prop; pub(crate) use self::sort::object_prototype_index_get as sort_object_prototype_index_get; +pub(crate) use self::sort::object_prototype_index_get_with_receiver as sort_object_prototype_index_get_with_receiver; pub use self::subclass::{ array_subclass_dense_snapshot, array_subclass_has_iterator_override, is_array_subclass_instance, }; diff --git a/crates/perry-runtime/src/array/sort.rs b/crates/perry-runtime/src/array/sort.rs index 5f047036fc..b8b667c856 100644 --- a/crates/perry-runtime/src/array/sort.rs +++ b/crates/perry-runtime/src/array/sort.rs @@ -355,6 +355,12 @@ fn object_prototype_numeric_keys() -> Vec { } pub(crate) fn object_prototype_index_get(index: u32) -> f64 { + let receiver = + object_prototype_value().unwrap_or_else(|| f64::from_bits(crate::value::TAG_UNDEFINED)); + object_prototype_index_get_with_receiver(index, receiver) +} + +pub(crate) fn object_prototype_index_get_with_receiver(index: u32, receiver: f64) -> f64 { match object_prototype_value() { Some(proto) => { // Fire an accessor getter installed via @@ -364,7 +370,7 @@ pub(crate) fn object_prototype_index_get(index: u32) -> f64 { if let Some(acc) = crate::object::get_accessor_descriptor(addr, &index.to_string()) { if acc.get != 0 { return f64::from_bits( - unsafe { crate::object::invoke_accessor_getter(acc.get, proto) }.bits(), + unsafe { crate::object::invoke_accessor_getter(acc.get, receiver) }.bits(), ); } return f64::from_bits(crate::value::TAG_UNDEFINED); diff --git a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs index be264fab6f..d37ec11dfd 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs @@ -64,10 +64,11 @@ pub(crate) fn ensure_function_prototype_object( }); if let Some(object_proto_bits) = global_object_prototype_bits() { + let object_proto = scope.root_nanbox_u64(object_proto_bits); proto_handle.with_mut_ptr::(|proto| { super::super::prototype_chain::object_set_static_prototype( proto as usize, - object_proto_bits, + object_proto.get_nanbox_u64(), ) }); } diff --git a/crates/perry-runtime/src/object/global_this/generator.rs b/crates/perry-runtime/src/object/global_this/generator.rs index 8033b1d7df..7ed5215e9b 100644 --- a/crates/perry-runtime/src/object/global_this/generator.rs +++ b/crates/perry-runtime/src/object/global_this/generator.rs @@ -146,16 +146,21 @@ pub(crate) fn wire_async_function_intrinsic_parents() { let proto = scope.root_raw_mut_ptr(proto as *mut ObjectHeader); let function_ctor = js_get_global_this_builtin_value(b"Function".as_ptr(), 8); if crate::value::JSValue::from_bits(function_ctor.to_bits()).is_pointer() { + let function_ctor = scope.root_nanbox_f64(function_ctor); ctor.with_mut_ptr::(|ctor| { - crate::closure::closure_set_static_prototype(ctor as usize, function_ctor.to_bits()) + crate::closure::closure_set_static_prototype( + ctor as usize, + function_ctor.get_nanbox_f64().to_bits(), + ) }); } let function_proto = builtin_prototype_value("Function"); if crate::value::JSValue::from_bits(function_proto.to_bits()).is_pointer() { + let function_proto = scope.root_nanbox_f64(function_proto); proto.with_mut_ptr::(|proto| { super::super::prototype_chain::object_set_static_prototype( proto as usize, - function_proto.to_bits(), + function_proto.get_nanbox_f64().to_bits(), ) }); } diff --git a/crates/perry-runtime/src/typedarray/construct.rs b/crates/perry-runtime/src/typedarray/construct.rs index 2c0b3bf382..212b5c8193 100644 --- a/crates/perry-runtime/src/typedarray/construct.rs +++ b/crates/perry-runtime/src/typedarray/construct.rs @@ -449,11 +449,11 @@ pub extern "C" fn js_typed_array_new_from_array( if method_bits == crate::value::TAG_UNDEFINED || method_bits == crate::value::TAG_NULL { let len = rooted.with_const_ptr::(|arr| crate::array::js_array_length(arr)); for index in 0..len { - raw.push( - scope.root_nanbox_f64(rooted.with_const_ptr::(|arr| { - crate::array::js_array_get_f64(arr, index) - })), - ); + raw.push(scope.root_nanbox_f64( + rooted.with_const_ptr::(|arr| { + crate::array::array_spec_get(arr, index) + }), + )); } } else { let method = scope.root_nanbox_f64(method); diff --git a/test-files/test_gap_typed_arrays.ts b/test-files/test_gap_typed_arrays.ts index e457428dd7..4ec50532a4 100644 --- a/test-files/test_gap_typed_arrays.ts +++ b/test-files/test_gap_typed_arrays.ts @@ -29,6 +29,30 @@ nullIteratorSource[Symbol.iterator] = null; const nullIterator = new Uint8Array(nullIteratorSource); console.log("from array with null iterator:", nullIterator[0], nullIterator[1]); +// Array-like indexed Get preserves the source as `this` when an inherited +// accessor supplies the element. +const inheritedIndexSource: any = new Array(1); +inheritedIndexSource[Symbol.iterator] = null; +let inheritedIndexReceiverMatches = false; +let inheritedIndexGetterCalls = 0; +const indexedPrototype: any = []; +Object.defineProperty(indexedPrototype, "0", { + configurable: true, + get() { + inheritedIndexGetterCalls++; + inheritedIndexReceiverMatches = this === inheritedIndexSource; + return inheritedIndexReceiverMatches ? 73 : 74; + }, +}); +Object.setPrototypeOf(inheritedIndexSource, indexedPrototype); +const inheritedIndexResult = new Int16Array(inheritedIndexSource); +console.log( + "from inherited indexed getter:", + inheritedIndexResult[0], + inheritedIndexReceiverMatches, + inheritedIndexGetterCalls, +); + // --- Uint8Array read/write --- const u3 = new Uint8Array(3); u3[0] = 100; diff --git a/test-files/test_issue_611_globalthis.ts b/test-files/test_issue_611_globalthis.ts index 31989f90e1..b857eb39d1 100644 --- a/test-files/test_issue_611_globalthis.ts +++ b/test-files/test_issue_611_globalthis.ts @@ -34,3 +34,21 @@ console.log( (globalThis as any).caughtScriptVar, (globalThis as any).finalScriptVar, ); + +const seen: number[] = []; +outer: for (var loopScriptVar = 0; loopScriptVar < 3; loopScriptVar++) { + if (loopScriptVar === 0) continue outer; + seen.push(loopScriptVar); +} +console.log( + "labeled loop:", + (globalThis as any).loopScriptVar, + seen.join(","), +); + +switch (1) { + case 1: + var switchScriptVar = "case"; + break; +} +console.log("switch var:", (globalThis as any).switchScriptVar); diff --git a/test-parity/expected/test_issue_611_globalthis.txt b/test-parity/expected/test_issue_611_globalthis.txt index ce2fcb2987..6d0770f520 100644 --- a/test-parity/expected/test_issue_611_globalthis.txt +++ b/test-parity/expected/test_issue_611_globalthis.txt @@ -4,3 +4,5 @@ typeof store: object store.get('k'): v nested var: 42 try/finally vars: try finally +labeled loop: 3 1,2 +switch var: case From 23ed68ef5b5c5f62a1b8ac3de5083e2c2d1a535a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 00:31:42 +0200 Subject: [PATCH 06/10] fix: close 5895 review and parity regressions --- .../perry-codegen/src/expr/this_super_call.rs | 9 +++ crates/perry-hir/src/lower/lower_module_fn.rs | 68 ++++++++++++------- crates/perry-runtime/src/array/indexing.rs | 11 ++- crates/perry-runtime/src/array/sort.rs | 10 ++- .../src/object/class_registry/construct.rs | 9 +++ crates/perry-runtime/src/object/tests.rs | 15 ++++ test-files/test_issue_611_globalthis.ts | 11 ++- .../expected/test_issue_611_globalthis.txt | 2 +- 8 files changed, 104 insertions(+), 31 deletions(-) diff --git a/crates/perry-codegen/src/expr/this_super_call.rs b/crates/perry-codegen/src/expr/this_super_call.rs index d2bda80476..abf55e6313 100644 --- a/crates/perry-codegen/src/expr/this_super_call.rs +++ b/crates/perry-codegen/src/expr/this_super_call.rs @@ -806,6 +806,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { None => double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)), }; lower_event_emitter_subclass_init(ctx, &this_box); + bind_derived_this_after_super(ctx); let current_class_name = ctx.class_stack.last().cloned().unwrap_or_default(); crate::lower_call::apply_field_initializers_recursive( @@ -1162,6 +1163,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &this_box, &lowered_args, ); + // The native base initialized the provisional receiver, so a + // successful super() must now initialize the derived `this` + // binding before field initializers or the remaining + // constructor body can observe it. Without this, an indirect + // chain such as Counter -> B -> EventEmitter installed the + // emitter surface but the next `this.seen = ...` still threw + // the pre-super ReferenceError. + bind_derived_this_after_super(ctx); // Spec: derived-class field initializers run AFTER `super()` // returns. The native base is the chain root and has no TS // fields, so everything after it still needs initializing — diff --git a/crates/perry-hir/src/lower/lower_module_fn.rs b/crates/perry-hir/src/lower/lower_module_fn.rs index 4d05b47bb7..2538d4323d 100644 --- a/crates/perry-hir/src/lower/lower_module_fn.rs +++ b/crates/perry-hir/src/lower/lower_module_fn.rs @@ -56,26 +56,7 @@ fn reflect_script_var_initializers( } } if let Some(update_expr) = update.take() { - let mut updated_globals: Vec<_> = script_vars - .iter() - .filter(|(id, _)| expr_updates_local(&update_expr, **id)) - .map(|(id, name)| (*id, name.clone())) - .collect(); - updated_globals.sort_unstable_by_key(|(id, _)| *id); - if updated_globals.is_empty() { - *update = Some(update_expr); - } else { - let mut sequence = Vec::with_capacity(1 + updated_globals.len()); - sequence.push(update_expr); - sequence.extend(updated_globals.into_iter().map(|(id, name)| { - Expr::PropertySet { - object: Box::new(Expr::GlobalThisExpr), - property: name, - value: Box::new(Expr::LocalGet(id)), - } - })); - *update = Some(Expr::Sequence(sequence)); - } + *update = Some(reflect_script_var_update_expr(update_expr, script_vars)); } *body = reflect_script_var_initializers(std::mem::take(body), script_vars); } @@ -159,11 +140,50 @@ fn reflect_script_var_initializers( reflected } -fn expr_updates_local(expr: &Expr, target: LocalId) -> bool { +fn reflect_script_var_update_expr(expr: Expr, script_vars: &HashMap) -> Expr { + let mut reflected = Vec::new(); + reflect_script_var_update_parts(expr, script_vars, &mut reflected); + if reflected.len() == 1 { + reflected.pop().expect("one reflected update expression") + } else { + Expr::Sequence(reflected) + } +} + +fn reflect_script_var_update_parts( + expr: Expr, + script_vars: &HashMap, + reflected: &mut Vec, +) { match expr { - Expr::LocalSet(id, _) | Expr::Update { id, .. } => *id == target, - Expr::Sequence(exprs) => exprs.iter().any(|expr| expr_updates_local(expr, target)), - _ => false, + Expr::Sequence(exprs) => { + for expr in exprs { + reflect_script_var_update_parts(expr, script_vars, reflected); + } + } + Expr::LocalSet(id, value) => { + reflected.push(Expr::LocalSet(id, value)); + reflect_script_var_update(id, script_vars, reflected); + } + Expr::Update { id, op, prefix } => { + reflected.push(Expr::Update { id, op, prefix }); + reflect_script_var_update(id, script_vars, reflected); + } + expr => reflected.push(expr), + } +} + +fn reflect_script_var_update( + id: LocalId, + script_vars: &HashMap, + reflected: &mut Vec, +) { + if let Some(name) = script_vars.get(&id) { + reflected.push(Expr::PropertySet { + object: Box::new(Expr::GlobalThisExpr), + property: name.clone(), + value: Box::new(Expr::LocalGet(id)), + }); } } diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 06ddb49ffb..d6a25b9343 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -354,12 +354,14 @@ pub(crate) fn array_spec_get(arr: *const ArrayHeader, index: u32) -> f64 { } unsafe { let receiver = crate::value::js_nanbox_pointer(arr as i64); + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(receiver); if array_has_own_index(arr, index) { return js_array_get_f64(arr, index); } if let Some(proto_arr) = array_custom_array_prototype(arr) { if index < (*proto_arr).length && array_has_own_index(proto_arr, index) { - return array_inherited_index_get(proto_arr, index, receiver); + return array_inherited_index_get(proto_arr, index, receiver.get_nanbox_f64()); } } if ARRAY_PROTO_HAS_INDEX.load(Ordering::Relaxed) { @@ -367,14 +369,17 @@ pub(crate) fn array_spec_get(arr: *const ArrayHeader, index: u32) -> f64 { if proto != 0 && proto != arr as usize { let proto_arr = proto as *const ArrayHeader; if index < (*proto_arr).length && array_has_own_index(proto_arr, index) { - return array_inherited_index_get(proto_arr, index, receiver); + return array_inherited_index_get(proto_arr, index, receiver.get_nanbox_f64()); } } } if OBJECT_PROTO_HAS_INDEX.load(Ordering::Relaxed) && crate::array::object_prototype_has_index_prop(index) { - return crate::array::sort_object_prototype_index_get_with_receiver(index, receiver); + return crate::array::sort_object_prototype_index_get_with_receiver( + index, + receiver.get_nanbox_f64(), + ); } TAG_UNDEFINED_F64 } diff --git a/crates/perry-runtime/src/array/sort.rs b/crates/perry-runtime/src/array/sort.rs index b8b667c856..bc494e9b69 100644 --- a/crates/perry-runtime/src/array/sort.rs +++ b/crates/perry-runtime/src/array/sort.rs @@ -361,6 +361,8 @@ pub(crate) fn object_prototype_index_get(index: u32) -> f64 { } pub(crate) fn object_prototype_index_get_with_receiver(index: u32, receiver: f64) -> f64 { + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(receiver); match object_prototype_value() { Some(proto) => { // Fire an accessor getter installed via @@ -370,7 +372,13 @@ pub(crate) fn object_prototype_index_get_with_receiver(index: u32, receiver: f64 if let Some(acc) = crate::object::get_accessor_descriptor(addr, &index.to_string()) { if acc.get != 0 { return f64::from_bits( - unsafe { crate::object::invoke_accessor_getter(acc.get, receiver) }.bits(), + unsafe { + crate::object::invoke_accessor_getter( + acc.get, + receiver.get_nanbox_f64(), + ) + } + .bits(), ); } return f64::from_bits(crate::value::TAG_UNDEFINED); diff --git a/crates/perry-runtime/src/object/class_registry/construct.rs b/crates/perry-runtime/src/object/class_registry/construct.rs index c9c587105c..0995e0757d 100644 --- a/crates/perry-runtime/src/object/class_registry/construct.rs +++ b/crates/perry-runtime/src/object/class_registry/construct.rs @@ -1249,6 +1249,15 @@ pub(crate) fn extends_target_must_throw(value: f64) -> bool { if is_arrow_function_value(value) || is_non_constructable_builtin_function_value(value) { return true; } + // Native-module constructor exports use the same BOUND_METHOD + // trampoline as ordinary method reads. Their module/method captures + // are the distinguishing [[Construct]] metadata: rejecting the raw + // trampoline here breaks dynamic aliases such as + // `const Console = console.Console; new Console(...)` and native base + // construction reached through an indirect user-class chain. + if is_bound_native_method_closure_value(value) { + return false; + } let ptr = jv.as_pointer::(); if !ptr.is_null() && is_valid_obj_ptr(ptr as *const u8) { // A bound *method* (class/instance method read as a value) is never diff --git a/crates/perry-runtime/src/object/tests.rs b/crates/perry-runtime/src/object/tests.rs index 3398d3d8c4..addaf32c8d 100644 --- a/crates/perry-runtime/src/object/tests.rs +++ b/crates/perry-runtime/src/object/tests.rs @@ -272,6 +272,21 @@ fn builtin_prototype_methods_reject_dynamic_new() { } } +#[test] +fn bound_native_constructor_export_is_not_rejected_as_a_bound_method() { + let _global = crate::gc::global_side_table_test_lock(); + let console = super::native_module::bound_native_callable_export_value("console", "Console"); + + assert!( + js_value_is_constructor(console), + "console.Console is a constructor" + ); + assert!( + !extends_target_must_throw(console), + "bound native constructor metadata must win over the shared bound-method trampoline" + ); +} + #[test] fn recorded_prototype_constructor_overrides_plain_object_constructor() { unsafe { diff --git a/test-files/test_issue_611_globalthis.ts b/test-files/test_issue_611_globalthis.ts index b857eb39d1..e4322edc31 100644 --- a/test-files/test_issue_611_globalthis.ts +++ b/test-files/test_issue_611_globalthis.ts @@ -36,7 +36,12 @@ console.log( ); const seen: number[] = []; -outer: for (var loopScriptVar = 0; loopScriptVar < 3; loopScriptVar++) { +const mirroredUpdates: number[] = []; +outer: for ( + var loopScriptVar = 0; + loopScriptVar < 3; + loopScriptVar++, mirroredUpdates.push((globalThis as any).loopScriptVar) +) { if (loopScriptVar === 0) continue outer; seen.push(loopScriptVar); } @@ -44,11 +49,13 @@ console.log( "labeled loop:", (globalThis as any).loopScriptVar, seen.join(","), + mirroredUpdates.join(","), ); switch (1) { - case 1: + case 1: { var switchScriptVar = "case"; break; + } } console.log("switch var:", (globalThis as any).switchScriptVar); diff --git a/test-parity/expected/test_issue_611_globalthis.txt b/test-parity/expected/test_issue_611_globalthis.txt index 6d0770f520..58af154c3f 100644 --- a/test-parity/expected/test_issue_611_globalthis.txt +++ b/test-parity/expected/test_issue_611_globalthis.txt @@ -4,5 +4,5 @@ typeof store: object store.get('k'): v nested var: 42 try/finally vars: try finally -labeled loop: 3 1,2 +labeled loop: 3 1,2 1,2,3 switch var: case From 9b244ac2361717a16dd3a5b2d864d4afd4fa87d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 01:41:12 +0200 Subject: [PATCH 07/10] fix: finish 5895 review follow-ups --- crates/perry-hir/src/lower/expr_new.rs | 26 +++ crates/perry-hir/src/lower/lower_module_fn.rs | 174 +++++++++++----- crates/perry-hir/src/lower/tests.rs | 27 +++ .../src/object/class_registry.rs | 2 +- .../src/object/class_registry/construct.rs | 13 +- .../class_registry/function_prototype.rs | 20 +- .../object/class_registry/parent_static.rs | 12 +- .../src/object/class_registry/state.rs | 21 +- .../perry-runtime/src/object/native_module.rs | 4 + .../native_module/constructor_exports.rs | 191 ++++++++++++++++++ crates/perry-runtime/src/object/tests.rs | 26 +-- test-files/test_gap_console_validate_write.ts | 19 ++ test-files/test_issue_611_globalthis.ts | 18 ++ .../expected/test_issue_611_globalthis.txt | 1 + 14 files changed, 456 insertions(+), 98 deletions(-) create mode 100644 crates/perry-runtime/src/object/native_module/constructor_exports.rs diff --git a/crates/perry-hir/src/lower/expr_new.rs b/crates/perry-hir/src/lower/expr_new.rs index afed3542bc..564d811065 100644 --- a/crates/perry-hir/src/lower/expr_new.rs +++ b/crates/perry-hir/src/lower/expr_new.rs @@ -1500,6 +1500,32 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R // via `js_new_function_construct` — see // `perry-codegen/src/lower_call/new.rs`. } + // A named native-module export whose public name is not a class + // name still has a real runtime function value. Route `new` over + // that value through the dynamic constructor check instead of the + // static `Expr::New { class_name }` fallback, which would merely + // allocate an empty placeholder. This is where Node distinguishes + // constructable JavaScript wrappers (`repl.start`, `events.init`) + // from native non-constructors (`path.toNamespacedPath`). The + // runtime's explicit export metadata makes that decision. Keep + // capitalized class exports on the specialized paths below. + if let Some((module, Some(export))) = ctx.lookup_native_module(&class_name) { + if export + .chars() + .next() + .is_some_and(|first| !first.is_uppercase()) + { + return Ok(Expr::NewDynamic { + callee: Box::new(Expr::PropertyGet { + byte_offset: 0, + object: Box::new(Expr::NativeModuleRef(module.to_string())), + property: export.to_string(), + }), + args, + byte_offset: new_byte_offset, + }); + } + } // #wall: an ALIASED named import of a native built-in class // (`import { BlockList as Wj4 } from "net"; new Wj4()`) must // construct exactly like the un-aliased form. The bare-ident diff --git a/crates/perry-hir/src/lower/lower_module_fn.rs b/crates/perry-hir/src/lower/lower_module_fn.rs index 2538d4323d..a49d26108e 100644 --- a/crates/perry-hir/src/lower/lower_module_fn.rs +++ b/crates/perry-hir/src/lower/lower_module_fn.rs @@ -21,6 +21,7 @@ use crate::lower_types::hoisted_text_codec::{ fn reflect_script_var_initializers( stmts: Vec, script_vars: &HashMap, + next_local_id: &mut LocalId, ) -> Vec { let mut reflected = Vec::with_capacity(stmts.len()); for mut stmt in stmts { @@ -30,21 +31,35 @@ fn reflect_script_var_initializers( else_branch, .. } => { - *then_branch = - reflect_script_var_initializers(std::mem::take(then_branch), script_vars); + *then_branch = reflect_script_var_initializers( + std::mem::take(then_branch), + script_vars, + next_local_id, + ); if let Some(branch) = else_branch { - *branch = reflect_script_var_initializers(std::mem::take(branch), script_vars); + *branch = reflect_script_var_initializers( + std::mem::take(branch), + script_vars, + next_local_id, + ); } } Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => { - *body = reflect_script_var_initializers(std::mem::take(body), script_vars); + *body = reflect_script_var_initializers( + std::mem::take(body), + script_vars, + next_local_id, + ); } Stmt::For { init, update, body, .. } => { if let Some(init_stmt) = init.take() { - let mut expanded = - reflect_script_var_initializers(vec![*init_stmt], script_vars); + let mut expanded = reflect_script_var_initializers( + vec![*init_stmt], + script_vars, + next_local_id, + ); if expanded.len() == 1 { *init = expanded.pop().map(Box::new); } else { @@ -56,13 +71,25 @@ fn reflect_script_var_initializers( } } if let Some(update_expr) = update.take() { - *update = Some(reflect_script_var_update_expr(update_expr, script_vars)); + let mut update_temps = Vec::new(); + *update = Some(reflect_script_var_update_expr( + update_expr, + script_vars, + next_local_id, + &mut update_temps, + )); + reflected.append(&mut update_temps); } - *body = reflect_script_var_initializers(std::mem::take(body), script_vars); + *body = reflect_script_var_initializers( + std::mem::take(body), + script_vars, + next_local_id, + ); } Stmt::Labeled { body, .. } => { let inner = std::mem::replace(body, Box::new(Stmt::Break)); - let mut expanded = reflect_script_var_initializers(vec![*inner], script_vars); + let mut expanded = + reflect_script_var_initializers(vec![*inner], script_vars, next_local_id); *body = if expanded.len() > 1 && matches!(expanded.last(), Some(Stmt::For { .. })) { // A reflected `for (var ...)` init expands to the init, // its global mirror, and the loop. Keep those one-shot @@ -91,16 +118,24 @@ fn reflect_script_var_initializers( catch, finally, } => { - *body = reflect_script_var_initializers(std::mem::take(body), script_vars); + *body = reflect_script_var_initializers( + std::mem::take(body), + script_vars, + next_local_id, + ); if let Some(catch) = catch { catch.body = reflect_script_var_initializers( std::mem::take(&mut catch.body), script_vars, + next_local_id, ); } if let Some(finally) = finally { - *finally = - reflect_script_var_initializers(std::mem::take(finally), script_vars); + *finally = reflect_script_var_initializers( + std::mem::take(finally), + script_vars, + next_local_id, + ); } } Stmt::Switch { cases, .. } => { @@ -108,6 +143,7 @@ fn reflect_script_var_initializers( case.body = reflect_script_var_initializers( std::mem::take(&mut case.body), script_vars, + next_local_id, ); } } @@ -140,50 +176,81 @@ fn reflect_script_var_initializers( reflected } -fn reflect_script_var_update_expr(expr: Expr, script_vars: &HashMap) -> Expr { - let mut reflected = Vec::new(); - reflect_script_var_update_parts(expr, script_vars, &mut reflected); - if reflected.len() == 1 { - reflected.pop().expect("one reflected update expression") - } else { - Expr::Sequence(reflected) - } -} - -fn reflect_script_var_update_parts( - expr: Expr, +fn reflect_script_var_update_expr( + mut expr: Expr, script_vars: &HashMap, - reflected: &mut Vec, -) { + next_local_id: &mut LocalId, + temp_decls: &mut Vec, +) -> Expr { + // A loop update is an arbitrary expression tree, not necessarily a bare + // assignment or a top-level comma sequence. Rewrite children first in + // their evaluation order so writes nested in call arguments, computed + // keys, conditionals, etc. are mirrored at the instant they execute. + crate::walker::walk_expr_children_mut(&mut expr, &mut |child| { + let original = std::mem::replace(child, Expr::Undefined); + *child = reflect_script_var_update_expr(original, script_vars, next_local_id, temp_decls); + }); + match expr { - Expr::Sequence(exprs) => { - for expr in exprs { - reflect_script_var_update_parts(expr, script_vars, reflected); - } - } - Expr::LocalSet(id, value) => { - reflected.push(Expr::LocalSet(id, value)); - reflect_script_var_update(id, script_vars, reflected); - } - Expr::Update { id, op, prefix } => { - reflected.push(Expr::Update { id, op, prefix }); - reflect_script_var_update(id, script_vars, reflected); + Expr::LocalSet(id, value) if script_vars.contains_key(&id) => Expr::Sequence(vec![ + Expr::LocalSet(id, value), + script_var_mirror_expr(id, script_vars), + ]), + Expr::Update { + id, + op, + prefix: true, + } if script_vars.contains_key(&id) => Expr::Sequence(vec![ + Expr::Update { + id, + op, + prefix: true, + }, + script_var_mirror_expr(id, script_vars), + ]), + Expr::Update { + id, + op, + prefix: false, + } if script_vars.contains_key(&id) => { + // The mirror must run after the update, while postfix `x++` must + // still evaluate to the old value for its parent expression. Save + // that result in a compiler-only local, publish the new binding, + // then restore the expression result. + let temp_id = *next_local_id; + *next_local_id += 1; + temp_decls.push(Stmt::Let { + id: temp_id, + name: format!("__perry_script_var_postfix_{temp_id}"), + ty: Type::Any, + mutable: true, + init: None, + }); + Expr::Sequence(vec![ + Expr::LocalSet( + temp_id, + Box::new(Expr::Update { + id, + op, + prefix: false, + }), + ), + script_var_mirror_expr(id, script_vars), + Expr::LocalGet(temp_id), + ]) } - expr => reflected.push(expr), + expr => expr, } } -fn reflect_script_var_update( - id: LocalId, - script_vars: &HashMap, - reflected: &mut Vec, -) { - if let Some(name) = script_vars.get(&id) { - reflected.push(Expr::PropertySet { - object: Box::new(Expr::GlobalThisExpr), - property: name.clone(), - value: Box::new(Expr::LocalGet(id)), - }); +fn script_var_mirror_expr(id: LocalId, script_vars: &HashMap) -> Expr { + Expr::PropertySet { + object: Box::new(Expr::GlobalThisExpr), + property: script_vars + .get(&id) + .expect("script var mirror requires a script var") + .clone(), + value: Box::new(Expr::LocalGet(id)), } } @@ -1405,8 +1472,11 @@ pub fn lower_module_full( // nested in blocks, loops, switch arms and try/catch/finally. Matching // by LocalId prevents a same-named lexical shadow from leaking onto // globalThis (ES modules keep their module binding only). - module.init = - reflect_script_var_initializers(std::mem::take(&mut module.init), &script_vars); + module.init = reflect_script_var_initializers( + std::mem::take(&mut module.init), + &script_vars, + &mut ctx.next_local_id, + ); } if ctx.is_entry_module && !is_esm_entry { const RESTRICTED_GLOBAL_NAMES: [&str; 3] = ["undefined", "NaN", "Infinity"]; diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index f5d8de263d..92b3df7cdb 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -244,6 +244,33 @@ fn test_native_module_binding_value_named_import() { } } +#[test] +fn new_named_native_function_routes_through_runtime_constructor_check() { + let source = r#" +import { toNamespacedPath } from "node:path"; +new toNamespacedPath(); +"#; + let module = perry_parser::parse_typescript(source, "native-new.ts").expect("source parses"); + let hir = super::lower_module(&module, "native-new", "native-new.ts").expect("source lowers"); + assert!( + hir.init.iter().any(|stmt| matches!( + stmt, + Stmt::Expr(crate::ir::Expr::NewDynamic { callee, .. }) + if matches!( + callee.as_ref(), + crate::ir::Expr::PropertyGet { object, property, .. } + if property == "toNamespacedPath" + && matches!( + object.as_ref(), + crate::ir::Expr::NativeModuleRef(module) if module == "path" + ) + ) + )), + "new over a named native function must construct its runtime export value: {:#?}", + hir.init + ); +} + #[test] fn test_native_module_binding_value_os_eol() { // `import { EOL } from 'os'` resolves to the OsEOL intrinsic value, whether diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index c0387c3bbe..01556db98e 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -64,7 +64,7 @@ pub(crate) use state::{ class_parent_closure, class_parent_closure_root_store, class_prototype_method_is_enumerable, class_prototype_method_set_enumerable, class_prototype_method_value_cache_root_store, class_prototype_object_root_store, class_static_defined_attrs, class_static_set_defined_attrs, - global_object_prototype_bits, is_bound_native_method_closure_value, + global_object_prototype_bits, is_bound_native_constructor_closure_value, is_non_constructable_builtin_function_value, parent_closure_in_chain, throw_non_constructable_builtin_function, }; diff --git a/crates/perry-runtime/src/object/class_registry/construct.rs b/crates/perry-runtime/src/object/class_registry/construct.rs index 0995e0757d..23257fff28 100644 --- a/crates/perry-runtime/src/object/class_registry/construct.rs +++ b/crates/perry-runtime/src/object/class_registry/construct.rs @@ -313,6 +313,13 @@ pub unsafe extern "C" fn js_new_function_construct( super::super::object_ops::throw_object_type_error(b"is not a constructor"); } if let Some((module, method)) = bound_native_callable_module_and_method(func_value) { + // Native constructors and ordinary exports share the bound-method + // trampoline. Consult the export metadata before falling through to + // generic closure construction; some lower-case JavaScript wrappers + // are constructors while native functions such as path methods are not. + if !super::super::native_module::is_native_module_constructor_export(&module, &method) { + super::super::object_ops::throw_object_type_error(b"is not a constructor"); + } if module == "perf_hooks" { if let Some(result) = crate::perf_hooks::construct_perf_hooks_class(&method, args_ptr, args_len) @@ -1215,6 +1222,10 @@ pub(crate) fn js_value_is_constructor(value: f64) -> bool { if is_non_constructable_builtin_function_value(value) { return false; } + let ptr = JSValue::from_bits(value.to_bits()).as_pointer::(); + if crate::closure::closure_is_bound_method(ptr) { + return is_bound_native_constructor_closure_value(value); + } true } @@ -1255,7 +1266,7 @@ pub(crate) fn extends_target_must_throw(value: f64) -> bool { // trampoline here breaks dynamic aliases such as // `const Console = console.Console; new Console(...)` and native base // construction reached through an indirect user-class chain. - if is_bound_native_method_closure_value(value) { + if is_bound_native_constructor_closure_value(value) { return false; } let ptr = jv.as_pointer::(); diff --git a/crates/perry-runtime/src/object/class_registry/function_prototype.rs b/crates/perry-runtime/src/object/class_registry/function_prototype.rs index da90ae67fd..2d50034724 100644 --- a/crates/perry-runtime/src/object/class_registry/function_prototype.rs +++ b/crates/perry-runtime/src/object/class_registry/function_prototype.rs @@ -11,6 +11,15 @@ pub(crate) fn function_would_have_own_prototype(func_value: f64) -> bool { { return false; } + let jv = crate::value::JSValue::from_bits(func_value.to_bits()); + if jv.is_pointer() { + let ptr = jv.as_pointer::(); + if crate::closure::closure_is_bound_method(ptr) + && !super::super::native_module::bound_native_callable_is_constructor_value(func_value) + { + return false; + } + } synthetic_class_id_for_function(func_value) != 0 } @@ -33,15 +42,8 @@ pub(crate) fn ordinary_function_prototype_value_for_read(func_value: f64) -> Opt if super::super::native_module::builtin_closure_is_non_constructable_value(func_value) { return None; } - let is_native_class_export = unsafe { - super::super::native_module::bound_native_callable_module_and_method(func_value) - } - .is_some_and(|(_, method)| { - method - .as_bytes() - .first() - .is_some_and(|b| b.is_ascii_uppercase()) - }); + let is_native_class_export = + super::super::native_module::bound_native_callable_is_constructor_value(func_value); if !is_native_class_export { return None; } diff --git a/crates/perry-runtime/src/object/class_registry/parent_static.rs b/crates/perry-runtime/src/object/class_registry/parent_static.rs index 25276ec985..12e80348c7 100644 --- a/crates/perry-runtime/src/object/class_registry/parent_static.rs +++ b/crates/perry-runtime/src/object/class_registry/parent_static.rs @@ -1,5 +1,5 @@ use super::*; -use crate::JSValue; +use crate::{object::object_ops::throw_object_type_error, JSValue}; use std::collections::HashMap; use std::sync::atomic::Ordering; @@ -122,6 +122,9 @@ pub extern "C" fn js_register_class_parent_dynamic(class_id: u32, mut parent_val if let Some((module, method)) = unsafe { super::super::native_module::bound_native_callable_module_and_method(parent_value) } { + if !super::super::native_module::is_native_module_constructor_export(&module, &method) { + throw_object_type_error(b"Class extends value is not a constructor"); + } if super::super::native_module::normalize_native_module_alias(&module) == "wasi" && method == "WASI" { @@ -129,16 +132,11 @@ pub extern "C" fn js_register_class_parent_dynamic(class_id: u32, mut parent_val } return; } - if is_bound_native_method_closure_value(parent_value) { - return; - } // Spec: a non-`null` superclass that is not a constructor throws a TypeError // at class-definition time (before any `.prototype` access). (Test262 // subclass/superclass-* and definition/invalid-extends.) if extends_target_must_throw(parent_value) { - super::super::object_ops::throw_object_type_error( - b"Class extends value is not a constructor", - ); + throw_object_type_error(b"Class extends value is not a constructor"); } // #5893 (ClassDefinitionEvaluation): once the superclass is confirmed a diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index 01ace7af6e..bebfa9038b 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -11,22 +11,11 @@ pub(crate) fn is_non_constructable_builtin_function_value(value: f64) -> bool { super::super::native_module::builtin_closure_is_non_constructable_value(value) } -/// True when `value` is a bound native-module method/export closure -/// (`BOUND_METHOD_FUNC_PTR` trampoline — what a `require('stream').Writable` -/// property read produces). These represent real Node classes/functions and -/// must be accepted as `extends` targets. -pub(crate) fn is_bound_native_method_closure_value(value: f64) -> bool { - // Gate on the native-module metadata, not the raw BOUND_METHOD_FUNC_PTR - // trampoline: reified `Function.prototype.{bind,call,apply}` values - // (`reify_function_method_value`) share that trampoline but are NOT native - // constructors, so matching the sentinel alone would let `class X extends - // obj.method {}` skip the spec-required TypeError and silently stay - // parentless. A real native-module export carries a non-empty module name. - unsafe { - super::super::native_module::bound_native_callable_module_and_method(value) - .map(|(module, _)| !module.is_empty()) - .unwrap_or(false) - } +/// True when `value` is a bound native-module *constructor* export. Native +/// constructors and ordinary module functions share `BOUND_METHOD_FUNC_PTR`, +/// so the export's explicit constructor metadata must make the distinction. +pub(crate) fn is_bound_native_constructor_closure_value(value: f64) -> bool { + super::super::native_module::bound_native_callable_is_constructor_value(value) } pub(crate) fn throw_non_constructable_builtin_function() -> ! { diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index 77ddf49960..1b53f7575e 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -21,6 +21,7 @@ mod perf_instance_bind; pub(crate) use perf_instance_bind::instance_bound_perf_method; mod constants; mod constants_tables; +mod constructor_exports; mod module_keys; mod namespace_builders; mod web_locks; @@ -47,6 +48,9 @@ pub(crate) use callable_exports::{ zlib_codes_object, }; pub(crate) use constants::get_native_module_constant; +pub(crate) use constructor_exports::{ + bound_native_callable_is_constructor_value, is_native_module_constructor_export, +}; pub(crate) use module_keys::{native_module_enumerable_keys, native_module_has_enumerable_key}; #[cfg(test)] pub(crate) use namespace_builders::create_fs_constants_object; diff --git a/crates/perry-runtime/src/object/native_module/constructor_exports.rs b/crates/perry-runtime/src/object/native_module/constructor_exports.rs new file mode 100644 index 0000000000..b47d731617 --- /dev/null +++ b/crates/perry-runtime/src/object/native_module/constructor_exports.rs @@ -0,0 +1,191 @@ +use super::*; + +/// Constructor metadata for native-module callable exports. +/// +/// Native exports and ordinary module functions share the same bound-method +/// closure trampoline, so callability alone cannot answer `IsConstructor`. +/// Keep the distinction explicit here instead of treating every export (or +/// every capitalized name) as constructable. Node exposes many lower-case +/// JavaScript wrapper functions that *are* constructable (`repl.start`, +/// `events.init`, most `fs` callbacks), so this is deliberately an exact +/// non-constructor metadata table rather than a capitalization heuristic. +pub(crate) fn is_native_module_constructor_export(module: &str, property: &str) -> bool { + let module = normalize_native_module_alias(module); + let module = cjs_default_base_module(module).unwrap_or(module); + let module = assert_instance_base_module(module).unwrap_or(module); + let module = normalize_native_module_alias(module); + let property = canonical_native_callable_property(module, property); + + if !is_native_module_callable_export(module, property) { + return false; + } + + !match module { + "assert" | "assert/strict" => matches!(property, "doesNotReject" | "rejects"), + "buffer.Buffer" => property == "of", + "console" => matches!( + property, + "assert" + | "clear" + | "context" + | "count" + | "countReset" + | "createTask" + | "debug" + | "dir" + | "dirxml" + | "error" + | "group" + | "groupCollapsed" + | "groupEnd" + | "info" + | "log" + | "profile" + | "profileEnd" + | "table" + | "time" + | "timeEnd" + | "timeLog" + | "timeStamp" + | "trace" + | "warn" + ), + "crypto" => matches!( + property, + "getCiphers" + | "getCurves" + | "getHashes" + | "privateDecrypt" + | "privateEncrypt" + | "publicDecrypt" + | "publicEncrypt" + | "timingSafeEqual" + ), + "crypto.KeyObject" => property == "from", + "dns" | "dns/promises" => property == "getServers", + "events" => property == "once", + "http" => property == "setMaxIdleHTTPParsers", + "inspector" => matches!(property, "close" | "url"), + "inspector.DOMStorage" => matches!( + property, + "domStorageItemAdded" + | "domStorageItemRemoved" + | "domStorageItemUpdated" + | "domStorageItemsCleared" + | "registerStorage" + ), + "inspector.Network" => matches!( + property, + "dataReceived" + | "dataSent" + | "loadingFailed" + | "loadingFinished" + | "requestWillBeSent" + | "responseReceived" + | "webSocketClosed" + | "webSocketCreated" + | "webSocketHandshakeResponseReceived" + ), + "inspector.Session" | "inspector/promises.Session" => property == "once", + "module" => matches!(property, "flushCompileCache" | "isBuiltin"), + "os" => matches!( + property, + "availableParallelism" + | "freemem" + | "machine" + | "release" + | "totalmem" + | "type" + | "version" + ), + "path" | "path.posix" | "path.win32" => matches!( + property, + "basename" + | "dirname" + | "extname" + | "isAbsolute" + | "join" + | "matchesGlob" + | "normalize" + | "parse" + | "relative" + | "resolve" + | "toNamespacedPath" + ), + "process" => matches!( + property, + "_debugEnd" + | "_debugProcess" + | "_fatalException" + | "_getActiveHandles" + | "_getActiveRequests" + | "_kill" + | "_startProfilerIdleNotifier" + | "_stopProfilerIdleNotifier" + | "abort" + | "availableMemory" + | "constrainedMemory" + | "dlopen" + | "getActiveResourcesInfo" + | "getegid" + | "geteuid" + | "getgid" + | "getgroups" + | "getuid" + | "reallyExit" + | "uptime" + ), + "punycode.ucs2" => property == "encode", + "stream" => property == "_isArrayBufferView", + "timers/promises" => property == "setInterval", + "tls" => property == "getCiphers", + "util" => matches!( + property, + "aborted" | "isDeepStrictEqual" | "parseArgs" | "toUSVString" + ), + "util.types" | "util/types" => matches!( + property, + "isAnyArrayBuffer" + | "isArgumentsObject" + | "isArrayBuffer" + | "isArrayBufferView" + | "isAsyncFunction" + | "isBigIntObject" + | "isBooleanObject" + | "isBoxedPrimitive" + | "isCryptoKey" + | "isDate" + | "isExternal" + | "isGeneratorFunction" + | "isGeneratorObject" + | "isKeyObject" + | "isMap" + | "isMapIterator" + | "isModuleNamespaceObject" + | "isNativeError" + | "isNumberObject" + | "isPromise" + | "isProxy" + | "isRegExp" + | "isSet" + | "isSetIterator" + | "isSharedArrayBuffer" + | "isStringObject" + | "isSymbolObject" + | "isWeakMap" + | "isWeakSet" + ), + "v8" => matches!( + property, + "cachedDataVersionTag" | "stopCoverage" | "takeCoverage" + ), + "v8.promiseHooks" => matches!(property, "onAfter" | "onBefore" | "onInit" | "onSettled"), + "worker_threads" => matches!(property, "moveMessagePortToContext" | "postMessageToThread"), + _ => false, + } +} + +pub(crate) fn bound_native_callable_is_constructor_value(value: f64) -> bool { + unsafe { bound_native_callable_module_and_method(value) } + .is_some_and(|(module, property)| is_native_module_constructor_export(&module, &property)) +} diff --git a/crates/perry-runtime/src/object/tests.rs b/crates/perry-runtime/src/object/tests.rs index addaf32c8d..ad92b4d2fb 100644 --- a/crates/perry-runtime/src/object/tests.rs +++ b/crates/perry-runtime/src/object/tests.rs @@ -273,18 +273,20 @@ fn builtin_prototype_methods_reject_dynamic_new() { } #[test] -fn bound_native_constructor_export_is_not_rejected_as_a_bound_method() { - let _global = crate::gc::global_side_table_test_lock(); - let console = super::native_module::bound_native_callable_export_value("console", "Console"); - - assert!( - js_value_is_constructor(console), - "console.Console is a constructor" - ); - assert!( - !extends_target_must_throw(console), - "bound native constructor metadata must win over the shared bound-method trampoline" - ); +fn bound_native_constructor_metadata_distinguishes_module_functions() { + assert!(super::native_module::is_native_module_constructor_export( + "console", "Console" + )); + assert!(super::native_module::is_native_module_constructor_export( + "repl", "start" + )); + assert!(super::native_module::is_native_module_constructor_export( + "events", "init" + )); + assert!(!super::native_module::is_native_module_constructor_export( + "node:path", + "toNamespacedPath" + )); } #[test] diff --git a/test-files/test_gap_console_validate_write.ts b/test-files/test_gap_console_validate_write.ts index 9519f2f514..5ed933eb2e 100644 --- a/test-files/test_gap_console_validate_write.ts +++ b/test-files/test_gap_console_validate_write.ts @@ -1,6 +1,8 @@ // Console stream-write validation parity (#3080). // Node's `new Console(...)` throws ERR_CONSOLE_WRITABLE_STREAM (a TypeError) // when a resolved stdout/stderr does not expose a callable `write` method. +import { toNamespacedPath } from "node:path"; + const Console = (console as any).Console; function check(label: string, fn: () => void): void { @@ -60,3 +62,20 @@ check("opts-both-ok", () => { const c = new Console({ stdout: { write() {} }, stderr: { write() {} } }); void c; }); + +// Native constructors and ordinary module functions use the same runtime +// callable trampoline. Constructor metadata must preserve Console while still +// rejecting a non-constructable export in both `extends` and `new`. +try { + class InvalidNativeBase extends toNamespacedPath {} + void InvalidNativeBase; + console.log("extends-path-function: accepted"); +} catch (e: any) { + console.log("extends-path-function:", e.name); +} +try { + new toNamespacedPath(); + console.log("new-path-function: accepted"); +} catch (e: any) { + console.log("new-path-function:", e.name); +} diff --git a/test-files/test_issue_611_globalthis.ts b/test-files/test_issue_611_globalthis.ts index e4322edc31..dbb1a8d290 100644 --- a/test-files/test_issue_611_globalthis.ts +++ b/test-files/test_issue_611_globalthis.ts @@ -52,6 +52,24 @@ console.log( mirroredUpdates.join(","), ); +// The update may hide the script-var write inside another expression. Mirror +// it before the next argument reads globalThis, while preserving the old value +// produced by postfix `++` for the first argument. +const nestedUpdateSeen: number[] = []; +for ( + var nestedUpdateScriptVar = 0; + nestedUpdateScriptVar < 1; + nestedUpdateSeen.push( + nestedUpdateScriptVar++, + (globalThis as any).nestedUpdateScriptVar, + ) +) {} +console.log( + "nested update:", + (globalThis as any).nestedUpdateScriptVar, + nestedUpdateSeen.join(","), +); + switch (1) { case 1: { var switchScriptVar = "case"; diff --git a/test-parity/expected/test_issue_611_globalthis.txt b/test-parity/expected/test_issue_611_globalthis.txt index 58af154c3f..36e107b8dc 100644 --- a/test-parity/expected/test_issue_611_globalthis.txt +++ b/test-parity/expected/test_issue_611_globalthis.txt @@ -5,4 +5,5 @@ store.get('k'): v nested var: 42 try/finally vars: try finally labeled loop: 3 1,2 1,2,3 +nested update: 1 0,1 switch var: case From 18969b9dd31665bf87ae716306675a654db5ad8e Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Sun, 23 Aug 2026 21:41:42 +0200 Subject: [PATCH 08/10] fix(regex): implement RepeatMatcher capture semantics --- Cargo.lock | 11 + Cargo.toml | 1 + crates/perry-runtime/Cargo.toml | 7 +- crates/perry-runtime/src/regex.rs | 87 ++++- crates/perry-runtime/src/regex/compile.rs | 16 + crates/perry-runtime/src/regex/exec.rs | 25 +- crates/perry-runtime/src/regex/exec_array.rs | 35 ++ crates/perry-runtime/src/regex/match_all.rs | 28 +- .../perry-runtime/src/regex/match_string.rs | 31 +- .../perry-runtime/src/regex/repeat_matcher.rs | 311 ++++++++++++++++++ .../perry-runtime/src/regex/replace_expand.rs | 55 ++++ crates/perry-runtime/src/regex/tests.rs | 77 ++++- 12 files changed, 673 insertions(+), 11 deletions(-) create mode 100644 crates/perry-runtime/src/regex/repeat_matcher.rs diff --git a/Cargo.lock b/Cargo.lock index 56cb11a139..c1f2dc8119 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6322,6 +6322,7 @@ dependencies = [ "perry-parser", "rand 0.10.1", "regex", + "regress", "resolv-conf", "ryu", "serde", @@ -7525,6 +7526,16 @@ version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" +[[package]] +name = "regress" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "158a764437582235e3501f683b93a0a6f8d825d04a789dbe5ed30b8799b8908a" +dependencies = [ + "hashbrown 0.16.1", + "memchr", +] + [[package]] name = "rend" version = "0.4.2" diff --git a/Cargo.toml b/Cargo.toml index 2623e8a948..0826ae1c39 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -372,6 +372,7 @@ libc = "0.2" lazy_static = "1.5" chrono = "0.4" regex = "1.12" +regress = "0.11.1" hex = "0.4" tempfile = "3" itoa = "1.0" diff --git a/crates/perry-runtime/Cargo.toml b/crates/perry-runtime/Cargo.toml index dd78b941b5..daf8bad55f 100644 --- a/crates/perry-runtime/Cargo.toml +++ b/crates/perry-runtime/Cargo.toml @@ -89,14 +89,14 @@ mod-node-test = [] # degrade gracefully when off (auto-optimize leaves this off unless the program # uses a heap-snapshot / `process.report` API, which the compiler detects). diagnostics = [] -# The user's regular-expression engine (`regex` + `fancy-regex`, ~1.2 MB of -# DFA/NFA machinery). A program that never evaluates a regex literal, `RegExp`, +# The user's regular-expression engines (`regex`, `fancy-regex`, and `regress`). +# A program that never evaluates a regex literal, `RegExp`, # a regex-coercing string method, or a glob API can't produce a RegExp at # runtime, so the compiler leaves this off and the engine is never linked. The # RegExp object's identity/display layer (header, `is_regex_pointer`, `toString`) # stays always compiled, so console-formatting / value-to-string paths keep # working with no engine present. -regex-engine = ["dep:regex", "dep:fancy-regex"] +regex-engine = ["dep:regex", "dep:fancy-regex", "dep:regress"] # The TC39 `Temporal.*` API (`temporal_rs` + its transitive tz/calendar deps: # jiff-tzdb, icu_calendar, timezone_provider, calendrical_calculations — # ~580 KB). Independent of JS `Date` (which has its own `date.rs` impl), so a @@ -261,6 +261,7 @@ libc.workspace = true gimli = { version = "0.34", default-features = false, features = ["read"] } rand = "0.10" regex = { workspace = true, optional = true } +regress = { workspace = true, optional = true } # Taffy — flexbox / grid layout engine for the perry/tui module # (#358 Phase 3). Same crate Bevy and Dioxus use; pure Rust, no FFI. taffy = { version = "0.13", default-features = false, features = ["std", "flexbox", "taffy_tree"] } diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 67ac66ce07..324fbb3e5e 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -40,6 +40,8 @@ mod grammar; #[cfg(feature = "regex-engine")] mod match_all; #[cfg(feature = "regex-engine")] +mod repeat_matcher; +#[cfg(feature = "regex-engine")] mod replace_expand; mod replace_fn; #[cfg(feature = "regex-engine")] @@ -224,6 +226,7 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * (*ptr).last_index = crate::value::JSValue::number(0.0).bits(); (*ptr).magic = REGEXP_MAGIC; (*ptr).fancy_ptr = std::ptr::null(); + (*ptr).repeat_matcher_ptr = std::ptr::null(); REGEX_EVER_REGISTERED.arm(); REGEX_POINTERS.with(|table| { @@ -277,7 +280,7 @@ pub(crate) fn regex_header_has_magic(re: *const RegExpHeader) -> bool { /// * `flags_ptr` — the flags `StringHeader`, /// * `last_index` — a writable JSValue (`re.lastIndex = …`) that may be a /// NaN-boxed heap pointer. -/// `regex_ptr`/`fancy_ptr` point to OFF-heap leaked Rust allocations and the +/// The compiled matcher pointers point to OFF-heap leaked Rust allocations and the /// bool/`magic` fields are never heap refs, so they must NOT be scanned. /// /// `pattern_ptr` and `flags_ptr` are consecutive equal-width fields, so under @@ -301,6 +304,11 @@ crate::perry_thread_local! { static REGEX_CACHE: RefCell>> = RefCell::new(HashMap::new()); /// Fancy-regex fallback cache for patterns with lookbehind/lookahead. static FANCY_CACHE: RefCell>> = RefCell::new(HashMap::new()); + + /// ECMAScript backtracking matchers for quantified capture groups. These + /// are the patterns where `regex`/`fancy-regex` cannot reproduce + /// `RepeatMatcher` capture reset and nullable-iteration semantics (#5897). + static REPEAT_MATCHER_CACHE: RefCell>> = RefCell::new(HashMap::new()); } /// Compiled-program size budget handed to both regex engines. @@ -343,13 +351,13 @@ pub(crate) fn build_fancy_regex(pattern: &str) -> Result bool { if already { return true; } + if let Some(repeat_matcher) = repeat_matcher::compile(pattern, flags) { + REPEAT_MATCHER_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + evict_regex_cache_if_full(&mut cache); + cache.insert( + (pattern.to_string(), flags.to_string()), + Arc::new(repeat_matcher), + ); + }); + } // Translate JS regex to Rust-compatible pattern let translated = js_regex_to_rust(pattern); let case_insensitive = flags.contains('i'); @@ -524,6 +542,10 @@ pub struct RegExpHeader { /// Header-resident twin of the `FANCY_CACHE` thread-local so the fancy /// fallback survives the duplicate-runtime split described above. pub fancy_ptr: *const (), + /// Header-owned `Arc` for quantified capture groups, + /// or null for the ordinary linear/fancy paths. Like `fancy_ptr`, this + /// survives cache eviction and duplicate statically-linked runtime copies. + pub repeat_matcher_ptr: *const (), } /// Self-identifying sentinel stamped into every `RegExpHeader.magic` by @@ -961,6 +983,15 @@ pub extern "C" fn js_regexp_new( None => std::ptr::null(), } }); + (*ptr).repeat_matcher_ptr = REPEAT_MATCHER_CACHE.with(|cache| { + match cache + .borrow() + .get(&(owned_pattern.clone(), flags_str.to_string())) + { + Some(arc) => Arc::into_raw(arc.clone()) as *const (), + None => std::ptr::null(), + } + }); // Record the pointer so that js_string_split can detect // `s.split(regex)` without a dedicated runtime decl. @@ -1131,6 +1162,14 @@ pub extern "C" fn js_regexp_test(re: *const RegExpHeader, s: *const StringHeader return if arr.is_null() { 0 } else { 1 }; } + if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + return if repeat_matcher.regex.find(str_data).is_some() { + 1 + } else { + 0 + }; + } + if let Some(fre) = lookup_fancy_regex(re) { return match fre.is_match(str_data) { Ok(true) => 1, @@ -1175,6 +1214,31 @@ pub(crate) fn lookup_fancy_regex(re: *const RegExpHeader) -> Option Option> { + unsafe { + if regex_header_has_magic(re) && !(*re).repeat_matcher_ptr.is_null() { + let raw = (*re).repeat_matcher_ptr as *const repeat_matcher::RepeatMatcherRegex; + let arc = Arc::from_raw(raw); + let cloned = arc.clone(); + std::mem::forget(arc); + return Some(cloned); + } + let pat = string_as_str((*re).pattern_ptr); + let flags_str = string_as_str((*re).flags_ptr); + REPEAT_MATCHER_CACHE.with(|cache| { + cache + .borrow() + .get(&(pat.to_string(), flags_str.to_string())) + .cloned() + }) + } +} + /// Replace matches in a string /// Expand a JS replacement string against one match, supporting the full set @@ -1346,6 +1410,11 @@ pub extern "C" fn js_string_replace_regex( } unsafe { + if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + let result = repeat_matcher.replace(str_data, repl_str, (*re).global); + return finish_replace_bytes(result.as_bytes()); + } + // Pattern the `regex` crate couldn't compile (lookbehind/backreferences) // → drive the replacement through fancy-regex. Otherwise the never-match // placeholder in `regex_ptr` would leave the input unchanged. @@ -1450,7 +1519,9 @@ pub extern "C" fn js_string_split_regex_n( unsafe { // Each element is either a substring (`Some`) or `undefined` (`None`, // for an unmatched capture group spliced into the result). - let parts: Vec> = if let Some(fre) = lookup_fancy_regex(re) { + 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.) @@ -1507,6 +1578,14 @@ pub extern "C" fn js_string_search_regex(s: *const StringHeader, re: *const RegE let str_data = string_as_str(s); unsafe { + if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + return repeat_matcher + .regex + .find(str_data) + .map(|matched| byte_index_to_utf16_index(str_data, matched.start()) as i32) + .unwrap_or(-1); + } + // Fancy-regex fallback (lookbehind/backreferences): the never-match // placeholder in `regex_ptr` would always report -1 otherwise. if let Some(fre) = lookup_fancy_regex(re) { diff --git a/crates/perry-runtime/src/regex/compile.rs b/crates/perry-runtime/src/regex/compile.rs index 548d2a01e8..a363a0a777 100644 --- a/crates/perry-runtime/src/regex/compile.rs +++ b/crates/perry-runtime/src/regex/compile.rs @@ -161,6 +161,15 @@ pub extern "C" fn js_regexp_compile_value( None => std::ptr::null(), } }); + let repeat_matcher_ptr: *const () = super::REPEAT_MATCHER_CACHE.with(|cache| { + match cache + .borrow() + .get(&(pattern_str.to_string(), flags_str.to_string())) + { + Some(arc) => Arc::into_raw(arc.clone()) as *const (), + None => std::ptr::null(), + } + }); let (canonical_flags_ptr, _) = re_handle.across_mut::(|| js_string_from_str(flags_str)); let canonical_flags_handle = scope.root_string_ptr(canonical_flags_ptr); @@ -171,8 +180,10 @@ pub extern "C" fn js_regexp_compile_value( unsafe { let old_regex_ptr = (*re).regex_ptr; let old_fancy_ptr = (*re).fancy_ptr; + let old_repeat_matcher_ptr = (*re).repeat_matcher_ptr; (*re).regex_ptr = regex_ptr; (*re).fancy_ptr = fancy_ptr; + (*re).repeat_matcher_ptr = repeat_matcher_ptr; // Release the receiver's PREVIOUS owned references now that the new // ones are installed (recompiling the same pattern is fine: the fresh // `into_raw` reference above keeps the shared program alive). @@ -182,6 +193,11 @@ pub extern "C" fn js_regexp_compile_value( if !old_fancy_ptr.is_null() { drop(Arc::from_raw(old_fancy_ptr as *const fancy_regex::Regex)); } + if !old_repeat_matcher_ptr.is_null() { + drop(Arc::from_raw( + old_repeat_matcher_ptr as *const super::repeat_matcher::RepeatMatcherRegex, + )); + } (*re).pattern_ptr = pattern_ptr; (*re).flags_ptr = canonical_flags_ptr; (*re).case_insensitive = flags_str.contains('i'); diff --git a/crates/perry-runtime/src/regex/exec.rs b/crates/perry-runtime/src/regex/exec.rs index fbb6a1f39d..fadfd9dbe7 100644 --- a/crates/perry-runtime/src/regex/exec.rs +++ b/crates/perry-runtime/src/regex/exec.rs @@ -60,7 +60,30 @@ pub extern "C" fn js_regexp_exec( } let search_str = &str_data[search_start_byte..]; - let owned = if let Some(fre) = lookup_fancy_regex(re) { + let owned = if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + repeat_matcher + .regex + .find(search_str) + .filter(|matched| !sticky || matched.start() == 0) + .map(|matched| { + if use_last_index { + set_last_index_throwing( + re, + super::exec_array::byte_index_to_utf16_index( + str_data, + search_start_byte + matched.end(), + ), + ); + } + OwnedExecMatch::from_repeat_matcher( + str_data, + search_start_byte, + &repeat_matcher, + &matched, + has_indices, + ) + }) + } else if let Some(fre) = lookup_fancy_regex(re) { match fre.captures(search_str) { Ok(Some(caps)) if !sticky || caps.get(0).is_some_and(|full| full.start() == 0) => { let full = caps.get(0).expect("capture zero is the full match"); diff --git a/crates/perry-runtime/src/regex/exec_array.rs b/crates/perry-runtime/src/regex/exec_array.rs index 8f5d0252c7..8c9602f6ed 100644 --- a/crates/perry-runtime/src/regex/exec_array.rs +++ b/crates/perry-runtime/src/regex/exec_array.rs @@ -150,6 +150,41 @@ impl OwnedExecMatch { match_index, } } + + pub(super) fn from_repeat_matcher( + str_data: &str, + search_start_byte: usize, + regex: &super::repeat_matcher::RepeatMatcherRegex, + matched: ®ress::Match, + has_indices: bool, + ) -> Self { + let captures: Vec> = matched + .groups() + .map(|capture| { + capture.map(|range| { + OwnedCapture::from_range_with_indices( + str_data, + search_start_byte + range.start, + search_start_byte + range.end, + has_indices, + ) + }) + }) + .collect(); + let named = regex + .capture_names + .iter() + .enumerate() + .filter_map(|(index, name)| name.as_ref().map(|name| (name.clone(), index + 1))) + .collect(); + let match_index = + byte_index_to_utf16_index(str_data, search_start_byte + matched.start()) as f64; + Self { + captures, + named, + match_index, + } + } } /// Match-result metadata helper taking the `input` property as an already-boxed diff --git a/crates/perry-runtime/src/regex/match_all.rs b/crates/perry-runtime/src/regex/match_all.rs index a4a91d99f5..d688418d54 100644 --- a/crates/perry-runtime/src/regex/match_all.rs +++ b/crates/perry-runtime/src/regex/match_all.rs @@ -94,7 +94,33 @@ unsafe fn materialize_match_all_results( let search_str = &str_data[search_start..]; let mut owned: Vec = Vec::new(); - if let Some(fre) = super::lookup_fancy_regex(re) { + if let Some(repeat_matcher) = super::lookup_repeat_matcher(re) { + for matched in repeat_matcher.regex.find_iter(search_str) { + owned.push(OwnedMatchAllData { + groups: matched + .groups() + .map(|group| group.map(|range| search_str[range].to_string())) + .collect(), + named: repeat_matcher + .capture_names + .iter() + .enumerate() + .filter_map(|(index, name)| { + name.as_ref().map(|name| { + ( + name.clone(), + matched + .group(index + 1) + .map(|range| search_str[range].to_string()), + ) + }) + }) + .collect(), + match_index: byte_index_to_utf16_index(str_data, search_start + matched.start()) + as f64, + }); + } + } else if let Some(fre) = super::lookup_fancy_regex(re) { let named_names: Vec<(usize, String)> = fre .capture_names() .enumerate() diff --git a/crates/perry-runtime/src/regex/match_string.rs b/crates/perry-runtime/src/regex/match_string.rs index 9ef4b03f14..dd9f0cd497 100644 --- a/crates/perry-runtime/src/regex/match_string.rs +++ b/crates/perry-runtime/src/regex/match_string.rs @@ -82,7 +82,36 @@ pub extern "C" fn js_string_match( let global = (*re).global; let has_indices = (*re).has_indices; - if let Some(fre) = lookup_fancy_regex(re) { + if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + if global { + let matches: Vec = repeat_matcher + .regex + .find_iter(str_data) + .map(|matched| { + OwnedCapture::from_range(str_data, matched.start(), matched.end()) + }) + .collect(); + if matches.is_empty() { + return ptr::null_mut(); + } + OwnedStringMatch::Global(matches) + } else { + let Some(matched) = repeat_matcher.regex.find(str_data) else { + LAST_EXEC_GROUPS.with(|g| *g.borrow_mut() = ptr::null_mut()); + return ptr::null_mut(); + }; + OwnedStringMatch::NonGlobal( + OwnedExecMatch::from_repeat_matcher( + str_data, + 0, + &repeat_matcher, + &matched, + has_indices, + ), + has_indices, + ) + } + } else if let Some(fre) = lookup_fancy_regex(re) { if global { let matches: Vec = fre .find_iter(str_data) diff --git a/crates/perry-runtime/src/regex/repeat_matcher.rs b/crates/perry-runtime/src/regex/repeat_matcher.rs new file mode 100644 index 0000000000..5ce703b71f --- /dev/null +++ b/crates/perry-runtime/src/regex/repeat_matcher.rs @@ -0,0 +1,311 @@ +//! ECMA-262 `RepeatMatcher` compatibility path. +//! +//! Rust's linear `regex` engine intentionally does not implement JavaScript's +//! backtracking capture semantics. In particular, captures nested below a +//! quantified group must be cleared before every iteration, and an optional +//! iteration that matches the empty string must be discarded. Keep the linear +//! engine as the default, but compile patterns where those captures are +//! observable with `regress`, an ECMAScript-native backtracking matcher. + +/// A compiled matcher plus the capture-name ordering that the public `regress` +/// match API does not expose directly. +pub(super) struct RepeatMatcherRegex { + pub(super) regex: regress::Regex, + pub(super) capture_names: Vec>, +} + +impl RepeatMatcherRegex { + fn named_group_range( + &self, + matched: ®ress::Match, + name: &str, + ) -> Option> { + self.capture_names + .iter() + .position(|candidate| candidate.as_deref() == Some(name)) + .and_then(|index| matched.group(index + 1)) + } + + pub(super) fn expand_replacement( + &self, + replacement: &str, + matched: ®ress::Match, + subject: &str, + ) -> String { + let bytes = replacement.as_bytes(); + let group_count = matched.captures.len() + 1; + let has_named_groups = self.capture_names.iter().any(Option::is_some); + let mut out = String::with_capacity(replacement.len() + 16); + let mut index = 0; + while index < bytes.len() { + if bytes[index] != b'$' { + let start = index; + while index < bytes.len() && bytes[index] != b'$' { + index += 1; + } + out.push_str(&replacement[start..index]); + continue; + } + if index + 1 >= bytes.len() { + out.push('$'); + break; + } + match bytes[index + 1] { + b'$' => { + out.push('$'); + index += 2; + } + b'&' => { + out.push_str(&subject[matched.range()]); + index += 2; + } + b'`' => { + out.push_str(&subject[..matched.start()]); + index += 2; + } + b'\'' => { + out.push_str(&subject[matched.end()..]); + index += 2; + } + b'0'..=b'9' => { + let first = (bytes[index + 1] - b'0') as usize; + let (group, consumed) = + if index + 2 < bytes.len() && bytes[index + 2].is_ascii_digit() { + let two = first * 10 + (bytes[index + 2] - b'0') as usize; + if (1..group_count).contains(&two) { + (Some(two), 2) + } else if (1..group_count).contains(&first) { + (Some(first), 1) + } else { + (None, 0) + } + } else if (1..group_count).contains(&first) { + (Some(first), 1) + } else { + (None, 0) + }; + if let Some(group) = group { + if let Some(range) = matched.group(group) { + out.push_str(&subject[range]); + } + index += 1 + consumed; + } else { + out.push('$'); + index += 1; + } + } + b'<' if has_named_groups => { + if let Some(relative_end) = replacement[index + 2..].find('>') { + let name = &replacement[index + 2..index + 2 + relative_end]; + if let Some(range) = self.named_group_range(matched, name) { + out.push_str(&subject[range]); + } + index += relative_end + 3; + } else { + out.push('$'); + index += 1; + } + } + _ => { + out.push('$'); + index += 1; + } + } + } + out + } + + pub(super) fn replace(&self, subject: &str, replacement: &str, global: bool) -> String { + let mut out = String::new(); + let mut last_end = 0; + for matched in self.regex.find_iter(subject) { + out.push_str(&subject[last_end..matched.start()]); + out.push_str(&self.expand_replacement(replacement, &matched, subject)); + last_end = matched.end(); + if !global { + break; + } + } + out.push_str(&subject[last_end..]); + out + } + + pub(super) fn split(&self, subject: &str, limit: i32) -> Vec> { + let mut out = Vec::new(); + let unbounded = limit < 0; + let push = |out: &mut Vec>, value: Option| -> bool { + out.push(value); + !unbounded && out.len() as i32 >= limit + }; + if subject.is_empty() { + if self.regex.find(subject).is_none() { + out.push(Some(String::new())); + } + return out; + } + + let mut pending_start = 0; + let mut cursor = 0; + while cursor < subject.len() { + let Some(matched) = self.regex.find_from(subject, cursor).next() else { + break; + }; + if matched.start() != cursor { + cursor = matched.start(); + continue; + } + let end = matched.end().min(subject.len()); + if end == pending_start { + cursor += subject[cursor..] + .chars() + .next() + .map(char::len_utf8) + .unwrap_or(1); + continue; + } + if push(&mut out, Some(subject[pending_start..cursor].to_string())) { + return out; + } + for capture in matched.captures { + let value = capture.map(|range| subject[range].to_string()); + if push(&mut out, value) { + return out; + } + } + pending_start = end; + cursor = end; + } + if unbounded || (out.len() as i32) < limit { + out.push(Some(subject[pending_start..].to_string())); + } + out + } +} + +#[derive(Clone, Copy)] +struct GroupFrame { + captures_before: usize, +} + +fn named_capture_end(bytes: &[u8], open: usize) -> Option { + if bytes.get(open + 1) != Some(&b'?') || bytes.get(open + 2) != Some(&b'<') { + return None; + } + if matches!(bytes.get(open + 3), Some(b'=') | Some(b'!')) { + return None; + } + bytes[open + 3..] + .iter() + .position(|byte| *byte == b'>') + .map(|offset| open + 3 + offset) +} + +fn is_capturing_group(bytes: &[u8], open: usize) -> bool { + bytes.get(open + 1) != Some(&b'?') || named_capture_end(bytes, open).is_some() +} + +fn has_braced_quantifier(bytes: &[u8], mut index: usize) -> bool { + if bytes.get(index) != Some(&b'{') { + return false; + } + index += 1; + let digits_start = index; + while bytes.get(index).is_some_and(u8::is_ascii_digit) { + index += 1; + } + if index == digits_start { + return false; + } + if bytes.get(index) == Some(&b',') { + index += 1; + while bytes.get(index).is_some_and(u8::is_ascii_digit) { + index += 1; + } + } + bytes.get(index) == Some(&b'}') +} + +fn quantifier_follows(bytes: &[u8], index: usize) -> bool { + matches!(bytes.get(index), Some(b'*') | Some(b'+') | Some(b'?')) + || has_braced_quantifier(bytes, index) +} + +/// Return the capture-name layout when a pattern has a capture inside a +/// quantified group. That is precisely the shape for which the linear engine's +/// leftmost-first result can expose stale captures or stop after the wrong +/// nullable iteration. +fn quantified_capture_layout(pattern: &str) -> Option>> { + let bytes = pattern.as_bytes(); + let mut captures = Vec::new(); + let mut groups = Vec::new(); + let mut needs_repeat_matcher = false; + let mut in_class = false; + let mut index = 0; + + while index < bytes.len() { + match bytes[index] { + b'\\' => index = (index + 2).min(bytes.len()), + b'[' if !in_class => { + in_class = true; + index += 1; + } + b']' if in_class => { + in_class = false; + index += 1; + } + b'(' if !in_class => { + let captures_before = captures.len(); + if is_capturing_group(bytes, index) { + let name = named_capture_end(bytes, index) + .map(|end| pattern[index + 3..end].to_string()); + captures.push(name); + } + groups.push(GroupFrame { captures_before }); + index += 1; + } + b')' if !in_class => { + let Some(group) = groups.pop() else { + index += 1; + continue; + }; + if captures.len() > group.captures_before && quantifier_follows(bytes, index + 1) { + needs_repeat_matcher = true; + } + index += 1; + } + _ => index += 1, + } + } + needs_repeat_matcher.then_some(captures) +} + +pub(super) fn compile(pattern: &str, flags: &str) -> Option { + let capture_names = quantified_capture_layout(pattern)?; + let regex = regress::Regex::with_flags(pattern, flags).ok()?; + Some(RepeatMatcherRegex { + regex, + capture_names, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detects_only_quantified_groups_with_captures() { + assert!(quantified_capture_layout(r"(a?b??)*").is_some()); + assert!(quantified_capture_layout(r"(?:(?=(abc))){0,1}a").is_some()); + assert!(quantified_capture_layout(r"[()]\\(literal\\)").is_none()); + assert!(quantified_capture_layout(r"(?:ab)*").is_none()); + assert!(quantified_capture_layout(r"(ab)c").is_none()); + } + + #[test] + fn records_named_capture_indices() { + assert_eq!( + quantified_capture_layout(r"(?:(?a)(b))*"), + Some(vec![Some("first".to_string()), None]) + ); + } +} diff --git a/crates/perry-runtime/src/regex/replace_expand.rs b/crates/perry-runtime/src/regex/replace_expand.rs index 431dc4944a..e65a19759c 100644 --- a/crates/perry-runtime/src/regex/replace_expand.rs +++ b/crates/perry-runtime/src/regex/replace_expand.rs @@ -265,6 +265,47 @@ pub(super) unsafe fn replace_regex_fn_fancy( replace_fn_run_matches(s_handle, &matches, closure_ptr, has_named_groups) } +pub(super) unsafe fn replace_regex_fn_repeat_matcher( + s_handle: &crate::gc::RuntimeHandle<'_>, + repeat_matcher: &super::repeat_matcher::RepeatMatcherRegex, + global: bool, + closure_ptr: *const crate::closure::ClosureHeader, +) -> *mut StringHeader { + let has_named_groups = repeat_matcher.capture_names.iter().any(Option::is_some); + let str_data = string_as_str(s_handle.get_raw_const_ptr::()); + let mut matches = Vec::new(); + for matched in repeat_matcher.regex.find_iter(str_data) { + matches.push(OwnedMatchData { + start: matched.start(), + end: matched.end(), + char_offset: super::exec_array::byte_index_to_utf16_index(str_data, matched.start()), + groups: matched + .groups() + .map(|group| group.map(|range| str_data[range].to_string())) + .collect(), + named: repeat_matcher + .capture_names + .iter() + .enumerate() + .filter_map(|(index, name)| { + name.as_ref().map(|name| { + ( + name.clone(), + matched + .group(index + 1) + .map(|range| str_data[range].to_string()), + ) + }) + }) + .collect(), + }); + if !global { + break; + } + } + replace_fn_run_matches(s_handle, &matches, closure_ptr, has_named_groups) +} + /// string.replace(regex, replacerFn) — replace with a callback function. /// /// The callback receives the full ECMAScript argument list (#2867): @@ -309,6 +350,15 @@ pub extern "C" fn js_string_replace_regex_fn( .with_const_ptr(|s_now: *const StringHeader| copy_replace_source(s_now)); } + if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + return replace_regex_fn_repeat_matcher( + &s_handle, + &repeat_matcher, + global, + closure_ptr, + ); + } + // If the `regex` crate couldn't compile this pattern (lookahead, // backreferences, …), `get_or_compile_regex` stashed a never-match // placeholder in `(*re).regex_ptr` and the real pattern in @@ -414,6 +464,11 @@ pub extern "C" fn js_string_replace_regex_named( } unsafe { + if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + let result = repeat_matcher.replace(str_data, repl_str, (*re).global); + return finish_replace_bytes(result.as_bytes()); + } + // Fancy-regex fallback (lookbehind/backreferences): expand `$` // and friends against the fancy captures instead of the never-match // placeholder stored in `regex_ptr`. diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index 0cc718740b..5ce8ea4ee1 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -260,6 +260,62 @@ fn fancy_lookbehind_exec_index() { } } +fn match_capture_text(arr: *const ArrayHeader, index: u32) -> Option { + let value = crate::array::js_array_get_f64(arr, index); + if crate::value::JSValue::from_bits(value.to_bits()).is_undefined() { + return None; + } + let string = crate::value::js_get_string_pointer_unified(value) as *const StringHeader; + Some(string_as_str(string).to_string()) +} + +#[test] +fn repeat_matcher_resets_nested_captures_each_iteration() { + let re = js_regexp_new(make_string(r"(z)((a+)?(b+)?(c))*"), make_string("")); + let matched = js_regexp_exec(re, make_string("zaacbbbcac")); + assert!(!matched.is_null()); + assert_eq!( + (0..6) + .map(|index| match_capture_text(matched, index)) + .collect::>(), + vec![ + Some("zaacbbbcac".to_string()), + Some("z".to_string()), + Some("ac".to_string()), + Some("a".to_string()), + None, + Some("c".to_string()), + ] + ); +} + +#[test] +fn repeat_matcher_discards_empty_optional_iterations() { + let re = js_regexp_new(make_string(r"(a?b??)*"), make_string("")); + let matched = js_regexp_exec(re, make_string("ab")); + assert!(!matched.is_null()); + assert_eq!(match_capture_text(matched, 0).as_deref(), Some("ab")); + assert_eq!(match_capture_text(matched, 1).as_deref(), Some("b")); +} + +#[test] +fn repeat_matcher_clears_captures_when_optional_lookahead_is_skipped() { + for pattern in [r"(?:(?=(abc)))?a", r"(?:(?=(abc))){0,1}a"] { + let re = js_regexp_new(make_string(pattern), make_string("")); + let matched = js_string_match(make_string("abc"), re); + assert!(!matched.is_null(), "{pattern}"); + assert_eq!(match_capture_text(matched, 0).as_deref(), Some("a")); + assert_eq!(match_capture_text(matched, 1), None, "{pattern}"); + } + + for pattern in [r"(?:(?=(abc)))a", r"(?:(?=(abc))){1,1}a"] { + let re = js_regexp_new(make_string(pattern), make_string("")); + let matched = js_string_match(make_string("abc"), re); + assert!(!matched.is_null(), "{pattern}"); + assert_eq!(match_capture_text(matched, 1).as_deref(), Some("abc")); + } +} + #[test] fn test_regexp_test_basic() { let pattern = make_string("hello"); @@ -617,7 +673,7 @@ fn unicode17_scripts_expand_to_codepoint_ranges() { ); } -/// 2026-07-09 GC audit (wave 2 batch A): `REGEX_CACHE`/`FANCY_CACHE` were +/// 2026-07-09 GC audit (wave 2 batch A): the compiled-regex caches were /// unbounded — one entry per distinct `(pattern, flags)` ever compiled, up to /// 64 MiB each — so `new RegExp(userInput)` was an attacker-driven OOM. The /// caches are now capped (clear-on-overflow) and every `RegExpHeader` OWNS a @@ -633,6 +689,11 @@ fn regex_cache_capped_and_prior_headers_survive_eviction() { let fancy = js_regexp_new(make_string(r"(?<=pre)\d+"), make_string("")); assert!(js_regexp_test(fancy, make_string("pre77")) != 0); + // A RepeatMatcher header whose ECMAScript matcher must likewise outlive + // its thread-local cache entry. + let repeat_matcher = js_regexp_new(make_string(r"(a?b??)*"), make_string("")); + assert!(js_regexp_test(repeat_matcher, make_string("ab")) != 0); + // Flood the cache with distinct patterns — far past the cap. for i in 0..(REGEX_CACHE_MAX_ENTRIES * 2 + 10) { let _ = get_or_compile_regex(&format!("cachefill{i}[a-z]+"), ""); @@ -653,6 +714,16 @@ fn regex_cache_capped_and_prior_headers_survive_eviction() { "FANCY_CACHE must stay capped at {REGEX_CACHE_MAX_ENTRIES} entries, got {fancy_len}" ); + // Quantified captures populate the ECMAScript RepeatMatcher cache. + for i in 0..(REGEX_CACHE_MAX_ENTRIES + 10) { + let _ = get_or_compile_regex(&format!("(repeat{i})*"), ""); + } + let repeat_len = REPEAT_MATCHER_CACHE.with(|c| c.borrow().len()); + assert!( + repeat_len <= REGEX_CACHE_MAX_ENTRIES, + "REPEAT_MATCHER_CACHE must stay capped at {REGEX_CACHE_MAX_ENTRIES} entries, got {repeat_len}" + ); + // The pre-flood headers still execute correctly: their compiled programs // are owned by the headers (leaked Arc refs), not borrowed from the // now-cleared caches. @@ -673,6 +744,10 @@ fn regex_cache_capped_and_prior_headers_survive_eviction() { js_regexp_test(fancy, make_string("nope77")) == 0, "fancy-fallback header must keep rejecting after cache eviction" ); + assert!( + js_regexp_test(repeat_matcher, make_string("ab")) != 0, + "RepeatMatcher header must keep matching after cache eviction" + ); } // --------------------------------------------------------------------------- From 703cff2aebe8c822af20b4e7534d951b1151a38a Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Sun, 23 Aug 2026 21:45:24 +0200 Subject: [PATCH 09/10] docs(changelog): note RegExp RepeatMatcher fix --- changelog.d/8660-regexp-repeat-matcher.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8660-regexp-repeat-matcher.md diff --git a/changelog.d/8660-regexp-repeat-matcher.md b/changelog.d/8660-regexp-repeat-matcher.md new file mode 100644 index 0000000000..58ef4221d9 --- /dev/null +++ b/changelog.d/8660-regexp-repeat-matcher.md @@ -0,0 +1 @@ +Fixed RegExp quantified capture groups to follow ECMA-262 RepeatMatcher semantics, including clearing skipped captures and discarding empty optional iterations. From 80b4cd2c87e2861f8217c12532deb8b5c82353b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 04:16:42 +0200 Subject: [PATCH 10/10] chore: changelog fragments and gate fixes for the five-PR batch - fragments for #8661, #8656, #8666, #8662 - #8660's replace_expand.rs raw-handle read taken through a scoped with_const_ptr (ceiling 7 -> 8 -> 7) - #8660's REPEAT_MATCHER_CACHE pinned on the gc-holder frontier --- changelog.d/8656-collator-compare-accessor.md | 2 ++ changelog.d/8661-effect-advisory-cleanup.md | 1 + changelog.d/8662-5895-review-followups.md | 1 + .../8666-imported-static-update-share.md | 2 ++ .../perry-runtime/src/regex/replace_expand.rs | 12 +++++++++--- scripts/gc_runtime_root_holders.json | 17 +++++++++++------ 6 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 changelog.d/8656-collator-compare-accessor.md create mode 100644 changelog.d/8661-effect-advisory-cleanup.md create mode 100644 changelog.d/8662-5895-review-followups.md create mode 100644 changelog.d/8666-imported-static-update-share.md diff --git a/changelog.d/8656-collator-compare-accessor.md b/changelog.d/8656-collator-compare-accessor.md new file mode 100644 index 0000000000..a006be4e43 --- /dev/null +++ b/changelog.d/8656-collator-compare-accessor.md @@ -0,0 +1,2 @@ +`Intl.Collator.prototype.compare` is now exposed as an accessor returning a +bound function, matching the spec's getter shape rather than a plain method. diff --git a/changelog.d/8661-effect-advisory-cleanup.md b/changelog.d/8661-effect-advisory-cleanup.md new file mode 100644 index 0000000000..f676618852 --- /dev/null +++ b/changelog.d/8661-effect-advisory-cleanup.md @@ -0,0 +1 @@ +Removed a stale Effect advisory entry from the package test expectations. diff --git a/changelog.d/8662-5895-review-followups.md b/changelog.d/8662-5895-review-followups.md new file mode 100644 index 0000000000..9d006653dd --- /dev/null +++ b/changelog.d/8662-5895-review-followups.md @@ -0,0 +1 @@ +Addressed the #5895 review follow-ups. diff --git a/changelog.d/8666-imported-static-update-share.md b/changelog.d/8666-imported-static-update-share.md new file mode 100644 index 0000000000..c82cab496e --- /dev/null +++ b/changelog.d/8666-imported-static-update-share.md @@ -0,0 +1,2 @@ +Shared the imported-static update path in codegen instead of re-emitting it per +call site. diff --git a/crates/perry-runtime/src/regex/replace_expand.rs b/crates/perry-runtime/src/regex/replace_expand.rs index e65a19759c..43dfa5a3db 100644 --- a/crates/perry-runtime/src/regex/replace_expand.rs +++ b/crates/perry-runtime/src/regex/replace_expand.rs @@ -146,7 +146,6 @@ unsafe fn replace_fn_run_matches( closure_ptr: *const crate::closure::ClosureHeader, has_named_groups: bool, ) -> *mut StringHeader { - let cur_str = || string_as_str(s_handle.get_raw_const_ptr::()); if matches.is_empty() { return s_handle.with_const_ptr(|s_now: *const StringHeader| copy_replace_source(s_now)); } @@ -159,7 +158,12 @@ unsafe fn replace_fn_run_matches( for m in matches { // Between-match text is re-sliced from the CURRENT subject address // (the previous callback may have moved the string). - result.push_str(&cur_str()[last_end..m.start]); + // #7341: re-derive the subject from the rooted handle inside a scoped + // borrow -- the previous callback may have moved it, and the `&str` + // never outlives this statement. + s_handle.with_const_ptr(|s_now: *const StringHeader| { + result.push_str(&string_as_str(s_now)[last_end..m.start]); + }); // Build the full ECMAScript callback argument list: // (match, p1, ..., pN, offset, string, groups?) @@ -216,7 +220,9 @@ unsafe fn replace_fn_run_matches( last_end = m.end; } - result.push_str(&cur_str()[last_end..]); + s_handle.with_const_ptr(|s_now: *const StringHeader| { + result.push_str(&string_as_str(s_now)[last_end..]); + }); finish_replace_bytes(result.as_bytes()) } diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index fe829e1a29..ba045ee175 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -786,13 +786,13 @@ }, { "file": "crates/perry-runtime/src/object/field_get_set/ic_miss/private_member_access.rs", - "name": "PRIVATE_METHOD_OWNER_HINT", - "why": "RefCell>: a declaring class_id plus an OWNED Rust String naming the private method. Neither field is, or is derived from, a heap address, so a copying collection cannot invalidate it and there is nothing for a scanner to visit." + "name": "PRIVATE_MEMBER_ACCESS_HINTS", + "why": "RefCell>; the struct is class_id: u32, name: String, kind: u32, is_static: bool, is_write: bool -- all owned Rust data. No JSValue, NaN-box word or raw heap pointer is stored." }, { "file": "crates/perry-runtime/src/object/field_get_set/ic_miss/private_member_access.rs", - "name": "PRIVATE_MEMBER_ACCESS_HINTS", - "why": "RefCell>; the struct is class_id: u32, name: String, kind: u32, is_static: bool, is_write: bool -- all owned Rust data. No JSValue, NaN-box word or raw heap pointer is stored." + "name": "PRIVATE_METHOD_OWNER_HINT", + "why": "RefCell>: a declaring class_id plus an OWNED Rust String naming the private method. Neither field is, or is derived from, a heap address, so a copying collection cannot invalidate it and there is nothing for a scanner to visit." }, { "file": "crates/perry-runtime/src/object/instanceof.rs", @@ -1051,6 +1051,11 @@ "file": "crates/perry-runtime/src/regex.rs", "name": "REGEX_CACHE" }, + { + "file": "crates/perry-runtime/src/regex.rs", + "name": "REPEAT_MATCHER_CACHE", + "why": "HashMap<(String,String), Arc> -- owned Rust strings keying an Arc to a compiled matcher. No JS heap pointer, so nothing asks a scanner to visit it." + }, { "file": "crates/perry-runtime/src/set.rs", "name": "SET_REGISTRY" @@ -1749,11 +1754,11 @@ }, { "file": "crates/perry-ui-ios/src/widgets/scrollview.rs", - "name": "SCROLL_MUTATION_GATE" + "name": "SCROLL_END_STATES" }, { "file": "crates/perry-ui-ios/src/widgets/scrollview.rs", - "name": "SCROLL_END_STATES" + "name": "SCROLL_MUTATION_GATE" }, { "file": "crates/perry-ui-ios/src/widgets/securefield.rs",