Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion crates/perry-runtime/src/array/generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,13 @@ pub extern "C" fn js_arraylike_slice(
} else {
clamp_index(start, len)
};
let e = if has_end == 0 {
// ECMA-262 §23.1.3.25 step 4: an `end` of `undefined` (whether omitted OR
// passed explicitly, e.g. `Array.prototype.slice.call(arr, 1, undefined)`)
// means "to the end" (relativeEnd = len). Only a present, non-undefined
// `end` is run through ToIntegerOrInfinity. `clamp_index` maps the
// TAG_UNDEFINED bit pattern (a NaN) to 0, which would wrongly empty the
// slice — so special-case it here.
let e = if has_end == 0 || end.to_bits() == crate::value::TAG_UNDEFINED {
len
} else {
clamp_index(end, len)
Expand Down
11 changes: 6 additions & 5 deletions crates/perry-runtime/src/atomics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,11 +475,12 @@ pub extern "C" fn js_atomics_load(_closure: *const ClosureHeader, view: f64, ind

#[no_mangle]
pub extern "C" fn js_atomics_is_lock_free(_closure: *const ClosureHeader, size: f64) -> f64 {
// ToIntegerOrInfinity(size) (runs `valueOf`/`toString`, so `'1'`, `true`,
// `{valueOf:()=>1}` and `3.14`→3 all behave like Node), then a membership
// test over the lock-free element widths.
let n = numeric_arg(size);
nanbox_bool(matches!(n as i64, 1 | 2 | 4 | 8))
// ToNumber(size) (runs `valueOf`/`toString`, so `'4'`→4 behaves like Node),
// then an EXACT membership test over the lock-free element widths. Node/V8
// compares the raw number, so `4.9` is NOT floored to 4 — it is simply not a
// valid element width and returns false.
let n = number_arg(size);
nanbox_bool(n == 1.0 || n == 2.0 || n == 4.0 || n == 8.0)
}

#[no_mangle]
Expand Down
18 changes: 18 additions & 0 deletions crates/perry-runtime/src/builtins/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,26 @@ pub extern "C" fn js_decode_uri_component(value: f64) -> i64 {
const ESCAPE_UNESCAPED: &[u8] =
b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789@*_+-./";

/// `ToString(value)` throws a TypeError for a Symbol argument. `escape` /
/// `unescape` (and the URI family) apply ToString to their input, so a Symbol
/// must reject rather than coerce to a `"Symbol(x)"` description string.
fn throw_if_symbol(value: f64) {
if (value.to_bits() & 0xFFFF_0000_0000_0000) == crate::value::POINTER_TAG
&& crate::symbol::is_registered_symbol(
(value.to_bits() & crate::value::POINTER_MASK) as usize,
)
{
let msg = b"Cannot convert a Symbol value to a string";
let msg_str = js_string_from_bytes(msg.as_ptr(), msg.len() as u32);
let err = crate::error::js_typeerror_new(msg_str);
crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64));
}
}

/// escape(string) -> string (legacy, ES Annex B B.2.1.1)
#[no_mangle]
pub extern "C" fn js_escape(value: f64) -> i64 {
throw_if_symbol(value);
let input = extract_str_from_nanbox(value);
let mut result = String::with_capacity(input.len() * 3);
let mut buf = [0u16; 2];
Expand All @@ -310,6 +327,7 @@ pub extern "C" fn js_escape(value: f64) -> i64 {
/// unescape(string) -> string (legacy, ES Annex B B.2.1.2)
#[no_mangle]
pub extern "C" fn js_unescape(value: f64) -> i64 {
throw_if_symbol(value);
let input = extract_str_from_nanbox(value);
let chars: Vec<char> = input.chars().collect();
// Reassemble into UTF-16 code units, then decode, so `%uD835%uDFD8`-style
Expand Down
23 changes: 17 additions & 6 deletions crates/perry-runtime/src/fs/dir_glob_watch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1522,7 +1522,10 @@ fn start_promise_watcher(id: usize, state: &mut PromiseWatchState) {
if state.active || state.closed {
return;
}
state.snapshot = snapshot_watch_target(&state.path, state.recursive).unwrap_or_default();
// Keep the creation-time baseline (seeded in `js_fs_promises_watch`) rather
// than re-snapshotting here — re-snapshotting would discard any events that
// occurred between `watch()` and this first `.next()` pull, which Node
// delivers (it buffers from FSWatcher creation, not from first iteration).
let timer_callback = poll_closure_value(promise_watcher_poll_impl as *const u8, id);
let timer_id = crate::timer::setInterval(timer_callback as i64, FS_WATCH_POLL_INTERVAL_MS);
if !state.persistent {
Expand Down Expand Up @@ -2207,11 +2210,19 @@ pub extern "C" fn js_fs_promises_watch(path_value: f64, options_value: f64) -> f
Ok(signal) => signal,
Err(err) => crate::exception::js_throw(err),
};
if let Err(err) = snapshot_watch_target(&path, recursive) {
unsafe {
// Capture the directory state at creation time. Node registers the
// FSWatcher synchronously in `watch()` and buffers events emitted before
// the first `.next()` pull, so a file written between `watch()` and the
// first iteration is still delivered. Perry's watcher is poll-based and
// previously took its baseline snapshot lazily at the first `.next()`,
// which silently dropped those pre-iteration events. Seed the baseline
// here so the first poll diffs against the creation-time state.
let initial_snapshot = match snapshot_watch_target(&path, recursive) {
Ok(snapshot) => snapshot,
Err(err) => unsafe {
crate::exception::js_throw(build_fs_error_value(&err, "watch", &path));
}
}
},
};
let id = next_watch_id();
let object_value = build_promise_watcher_object(id);
let abort_listener = signal
Expand All @@ -2235,7 +2246,7 @@ pub extern "C" fn js_fs_promises_watch(path_value: f64, options_value: f64) -> f
timer_id: 0,
persistent,
active: false,
snapshot: WatchSnapshot::new(),
snapshot: initial_snapshot,
queue: VecDeque::new(),
pending: VecDeque::new(),
signal: signal_value,
Expand Down
27 changes: 11 additions & 16 deletions crates/perry-runtime/src/promise/async_step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,17 @@ pub extern "C" fn js_promise_resolved(value: f64) -> *mut Promise {
// Issue #586: ECMAScript thenable assimilation. The async-to-generator
// transform rewrites every `await x` into `Promise.resolve(x).then(...)`
// — which means thenable assimilation has to happen here, not in the
// codegen-side `Expr::Await` lowering. `js_assimilate_thenable` returns
// a fresh Promise wrapper that follows the thenable's `.then(resolve,
// reject)` callbacks; chain its eventual state into our outer promise
// via the same `js_promise_resolve_with_promise` pattern as the real-
// Promise arm above. Drizzle's `QueryPromise` (`then` triggers the SQL
// round-trip) is the load-bearing motivating case (#488).
let assim = js_assimilate_thenable(value);
if assim.to_bits() != value.to_bits() && js_value_is_promise(assim) != 0 {
let inner = crate::value::js_nanbox_get_pointer(assim) as *mut Promise;
if !inner.is_null() && inner != promise {
js_promise_resolve_with_promise(promise, inner);
return promise;
}
}

js_promise_resolve(promise, value);
// codegen-side `Expr::Await` lowering. `promise_resolve_assimilating`
// implements the spec PromiseResolveThenableJob: a thenable's `.then` is
// invoked from a SCHEDULED microtask, never synchronously during resolve.
// This matters for `Promise.race`/`Promise.any` over a thenable, where Node
// does not call the thenable's `then` until the job runs (so the count of
// synchronous `then` invocations stays 0). Primitives (fast path above) and
// native Promises (short-circuit above) never reach here, so the per-await
// steady state is untouched; only real thenables (drizzle's `QueryPromise`,
// object literals with `then`) defer by one microtask — which the await
// loop drains, leaving the resolved value identical.
super::combinators::promise_resolve_assimilating(promise, value);
Comment on lines +51 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the async-function tail-return path on the same queued-assimilation semantics.

Line 61 now fixes js_promise_resolved by routing thenables through promise_resolve_assimilating, but resolve_trap_next_with_adoption() later in this file (Lines 350-375) still uses synchronous js_assimilate_thenable(...). That means Promise.resolve(thenable) and the js_async_step_done() reuse fast path for return thenable can now observe different timing, so this only partially fixes the PromiseResolveThenableJob parity issue.

Using the same combinator (or an equivalent queued-assimilation helper) in resolve_trap_next_with_adoption() would keep both adoption paths aligned.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/promise/async_step.rs` around lines 51 - 61, The
async-function tail-return path uses synchronous js_assimilate_thenable in
resolve_trap_next_with_adoption(), causing a timing mismatch with the new use of
super::combinators::promise_resolve_assimilating in
js_promise_resolved/js_async_step_done; update resolve_trap_next_with_adoption()
to route thenables through the same queued assimilation helper (call
promise_resolve_assimilating or an equivalent queued-assimilation wrapper)
instead of calling js_assimilate_thenable(...) so both Promise.resolve(thenable)
and the async tail-return reuse the same scheduled-thenable semantics and
preserve PromiseResolveThenableJob parity.

promise
}

Expand Down
Loading