diff --git a/CLAUDE.md b/CLAUDE.md index e6a9fb4a90..a0076624b2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1620 +**Current Version:** 0.5.1621 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index f94796a57c..96325b1467 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5543,7 +5543,7 @@ checksum = "1473d470930ed48574515a25df34900f3af89c6fa422d903e019121312a9f13e" [[package]] name = "perry" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "base64 0.22.1", @@ -5607,7 +5607,7 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-dispatch", "serde", @@ -5615,7 +5615,7 @@ dependencies = [ [[package]] name = "perry-audio-miniaudio" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "cc", "libc", @@ -5624,7 +5624,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "aho-corasick", "anyhow", @@ -5641,7 +5641,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-hir", @@ -5649,7 +5649,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-hir", @@ -5657,7 +5657,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-dispatch", @@ -5666,7 +5666,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-hir", @@ -5674,7 +5674,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "base64 0.22.1", @@ -5686,7 +5686,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-hir", @@ -5694,7 +5694,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "async-trait", "clap", @@ -5718,14 +5718,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "serde", "serde_json", @@ -5733,7 +5733,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1620" +version = "0.5.1621" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5744,7 +5744,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "clap", @@ -5759,7 +5759,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "block2", "objc2", @@ -5769,7 +5769,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "argon2", "perry-ffi", @@ -5778,7 +5778,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "bcrypt", "perry-ffi", @@ -5786,7 +5786,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "rusqlite", @@ -5794,7 +5794,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "scraper", @@ -5802,7 +5802,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "rust_decimal", @@ -5810,7 +5810,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "rand 0.10.2", @@ -5818,7 +5818,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "perry-runtime", @@ -5826,7 +5826,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "bytes", "lazy_static", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "bytes", @@ -5871,7 +5871,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "lazy_static", "perry-ffi", @@ -5881,7 +5881,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "bson", "futures-util", @@ -5893,7 +5893,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "chrono", "perry-ffi", @@ -5905,7 +5905,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "bytes", "perry-ffi", @@ -5920,7 +5920,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "lettre", "perry-ffi", @@ -5930,7 +5930,7 @@ dependencies = [ [[package]] name = "perry-ext-parcel-watcher" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "notify", "perry-ffi", @@ -5942,7 +5942,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "printpdf", @@ -5950,7 +5950,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "sqlx", @@ -5959,7 +5959,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "fast_image_resize", "image", @@ -5970,7 +5970,7 @@ dependencies = [ [[package]] name = "perry-ext-streams" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "lazy_static", "perry-ffi", @@ -5979,7 +5979,7 @@ dependencies = [ [[package]] name = "perry-ext-typescript" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-ffi", @@ -5999,7 +5999,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-ffi", "perry-runtime", @@ -6008,7 +6008,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "futures-util", "lazy_static", @@ -6021,7 +6021,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "brotli", "flate2", @@ -6031,7 +6031,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "dashmap 6.2.1", "once_cell", @@ -6041,7 +6041,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-api-manifest", @@ -6061,11 +6061,11 @@ dependencies = [ [[package]] name = "perry-native-registration" -version = "0.5.1620" +version = "0.5.1621" [[package]] name = "perry-parser" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "perry-diagnostics", @@ -6078,7 +6078,7 @@ dependencies = [ [[package]] name = "perry-perex" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perex", "regex", @@ -6086,7 +6086,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "ahash", "base64 0.22.1", @@ -6144,14 +6144,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6234,21 +6234,21 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "perry-hir", ] [[package]] name = "perry-ui" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "dirs", "perry-ffi", @@ -6258,7 +6258,7 @@ dependencies = [ [[package]] name = "perry-ui-android" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "jni", @@ -6273,7 +6273,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "rand 0.10.2", "serde", @@ -6283,7 +6283,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "cairo-rs 0.22.9", @@ -6306,7 +6306,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "block2", @@ -6323,7 +6323,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "block2", @@ -6340,7 +6340,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1620" +version = "0.5.1621" [[package]] name = "perry-ui-test" @@ -6351,11 +6351,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1620" +version = "0.5.1621" [[package]] name = "perry-ui-tvos" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "block2", @@ -6372,7 +6372,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "block2", @@ -6389,7 +6389,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "block2", "libc", @@ -6403,7 +6403,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "libc", @@ -6422,7 +6422,7 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "base64 0.22.1", "libc", @@ -6435,7 +6435,7 @@ dependencies = [ [[package]] name = "perry-updater" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "anyhow", "base64 0.22.1", @@ -6450,7 +6450,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1620" +version = "0.5.1621" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index b6e65ba66c..24bc1b26cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -317,7 +317,7 @@ codegen-units = 1 codegen-units = 1 [workspace.package] -version = "0.5.1620" +version = "0.5.1621" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/10805-stream-legacy-heritage.md b/changelog.d/10805-stream-legacy-heritage.md new file mode 100644 index 0000000000..6d103fc69e --- /dev/null +++ b/changelog.d/10805-stream-legacy-heritage.md @@ -0,0 +1,12 @@ +### Fixed + +- **`class X extends Stream` (the bare `node:stream` base) — `pipe()`, the + EventEmitter listener/emit surface, and `instanceof Stream` are no longer + missing.** `#10649` fixed the analogous dynamic-heritage dispatch for + `Readable`/`Writable`/`Duplex`/`Transform` but stopped short of `Stream` — + the base those four derive from. Every heritage shape (bare import, + namespace member, or CJS destructured `require('stream')`) now installs + the correct surface, matching Node byte-for-byte. `instanceof Stream` is + scoped to genuine `extends Stream` subclasses only — a plain + `extends EventEmitter` class does not newly satisfy it. `PassThrough` + (`#10745`) is a separate, deeper gap and remains unaffected. diff --git a/changelog.d/10807-urlsearchparams-prototype-value-reads.md b/changelog.d/10807-urlsearchparams-prototype-value-reads.md new file mode 100644 index 0000000000..b61fb00366 --- /dev/null +++ b/changelog.d/10807-urlsearchparams-prototype-value-reads.md @@ -0,0 +1,13 @@ +### Fixed + +- **`URLSearchParams.prototype` methods read as a value now resolve to real, + callable closures.** `URLSearchParams.prototype.append`, `.prototype["has"]`, + and the same reads through a Proxy `get` trap previously returned + `undefined`, so `Function.prototype.call`/`.apply` on the result threw + `Function.prototype.call was called on a value that is not a function`. + node-fetch@3.3.2's `Headers extends URLSearchParams` hits this via a + constructor-returned `Proxy` whose `get` trap does exactly this, on every + `fetch()` call. The six other `#10555`-group members + (`AbortController`, `AbortSignal`, `CustomEvent`, `Event`, `EventTarget`, + `URL`) have the identical defect and remain unfixed — see #10807's PR body + for the audit. (#10759) diff --git a/changelog.d/10813-map-get-string-index-ptrhasher.md b/changelog.d/10813-map-get-string-index-ptrhasher.md new file mode 100644 index 0000000000..c22d57be22 --- /dev/null +++ b/changelog.d/10813-map-get-string-index-ptrhasher.md @@ -0,0 +1,52 @@ +`MAP_STRING_INDEX` (the content-hashed side table behind string-keyed +`Map.get`/`set`/`has`/`delete` once a map grows past `SIDE_TABLE_THRESHOLD`) +hashed its inner `u64 -> Vec` table with `std::collections::HashMap`'s +default SipHash. The `u64` key there is not raw input — it is already the +FNV-1a content hash computed above it, so SipHash was paying for a second, +unrelated hash of an already-well-mixed value on every string-keyed lookup. +Switched the inner table to `PtrHasher` (a single multiply-by-Fibonacci- +constant plus an xorshift avalanche), the same treatment the sibling +`NumericIndex.hashed` table already gets for the same reason (a computed, +non-adversarial `u64` key). + +This is not a HashDoS regression: two colliding `u64` FNV-1a hashes land in +the same `Vec` bucket under *any* hasher — SipHash on the outer table +could never have defended against a crafted FNV-1a collision, because the +attack surface is FNV-1a itself, which is unchanged. `HashMap::get` also re-checks `K: Eq` on every candidate regardless of `S`, so a +hasher swap cannot change which entry a lookup resolves to, only how fast it +gets there — a property pinned by a new test that inserts 10,000 distinct +string keys (forcing real bucket collisions at both the outer map-pointer +and inner content-hash levels) and asserts every key still resolves to its +own value, including reverse-order reads, content-equal-but-freshly- +allocated keys, adversarial shared-prefix misses, and delete-half-then- +verify. + +Measured as a flat constant-factor win across an N-sweep from 16 to 4,096 +map entries (well past the growth threshold): 115-121 instructions saved +per lookup at every size tested, for both hits (interned and dynamically +built keys) and misses — not a small-map-only effect. (Absolute figures +were taken under `PERRY_NO_AUTO_OPTIMIZE=1`, whose own overhead varies +wildly by workload -- ~0.3% on string concat, 8x on `String.prototype. +split` elsewhere -- so treat the absolutes as flag-qualified; the 115-121 +delta is unaffected, since both arms of every comparison shared the flag +and the same runtime archive otherwise.) An initial reading of one shape +(dynamically-rebuilt keys) showed an apparent ~38-instruction-per-doubling +growth with N; that turned out to be a probe artifact (the decimal key +suffix `"key" + (i % size)` grows a digit as `size` grows, so key length +was correlating with N) and vanished under a length-controlled follow-up. + +This change is **not** a fix for the separately-reported (#10697) +string-Map slowdown on small (below-`SIDE_TABLE_THRESHOLD`) maps: that +session profiled the generic dispatch path directly and found +`find_key_index_cold` running on every lookup of a four-entry map, with +`jsvalue_eq` doing ~149 instructions of generic equality on a key already +known at compile time to be a string, plus `string_view_from_bits` +re-deriving what codegen had already proven -- ~361 instructions against +node's ~50. A related but narrower observation surfaced while chasing that +repro (`m.get(arr[i])` dispatches to the specialized string-keyed path, +but binding the same expression through a local first, `let key = arr[i]; +m.get(key)`, does not) -- confirmed to require both an inline index +expression *and* a ``-annotated map, not the binding shape +alone. Neither is a hashing cost; both are codegen type-proof gaps, tracked +and owned separately from this change. diff --git a/changelog.d/10814-destructuring-rest-array-literal.md b/changelog.d/10814-destructuring-rest-array-literal.md new file mode 100644 index 0000000000..7a0d3d46ed --- /dev/null +++ b/changelog.d/10814-destructuring-rest-array-literal.md @@ -0,0 +1,31 @@ +**perf(codegen):** `const {a, b, ...rest} = obj` built its excluded-key array +(the list of statically-named keys `ObjectRest` must NOT copy into `rest`) via +one `js_array_alloc_with_length` call plus one `js_array_set_f64_unchecked` +call *per excluded key* — each of those per-key calls re-derived and +re-bounds-checked a receiver the site had just allocated itself, so every +check inside (frozen? has index descriptors? index in range?) was statically +true. The excluded keys are compile-time-known string literals, so this is +exactly the "array literal of known values" shape `perry-codegen` already has +a cheap path for (`lower_array_literal`/`emit_array_from_lowered_values`, +previously reused only for the rest/`arguments` call-bundle case): one inline +bump allocation plus N `store double`, with a single call only on the cold +arena-full arm. `js_object_rest` itself, and everything downstream of it, is +unchanged — only how its `exclude_keys` argument gets built changes. The +source object's pointer is now also derived *after* that allocation rather +than cached across it. + +Also added `test-files/test_gap_object_destructuring_field_and_rest_guard.ts`, +covering 2-field, 5-field, nested, defaulted, and rest object destructuring +(including computed-key exclusion, an empty pattern before rest, and function +parameter destructuring), plus the easy-to-break edge cases: missing +properties reading as `undefined`, defaults applying only to `undefined` and +not `null`, getter evaluation order following the *pattern's* key order (not +the source object's), and `null`/`undefined` sources throwing `TypeError` +(including for an empty pattern and a `...rest`-only pattern). + +Along the way, found and confirmed **pre-existing** (unaffected by this +change, reproduces identically on unmodified `main`): `const {...rest} = obj` +silently drops any Symbol-keyed own property of `obj` from `rest` instead of +copying it through. Not fixed here — it's in `js_object_rest`'s own key-copy +logic, unrelated to how the `exclude_keys` array is constructed — but worth a +follow-up issue. diff --git a/changelog.d/10816-sepjv-cfg-gate.md b/changelog.d/10816-sepjv-cfg-gate.md new file mode 100644 index 0000000000..121c5baa55 --- /dev/null +++ b/changelog.d/10816-sepjv-cfg-gate.md @@ -0,0 +1,10 @@ +Gated `sep_jv` in `string/split.rs` behind `#[cfg(feature = "regex-engine")]`, +the only arm that reads it. Bound unconditionally it is an unused variable in +any build without that feature, so `RUSTFLAGS="-D warnings" cargo check -p perry +--bins` — one of the six compile commands `run_lint_gates.sh` derives — failed. + +Worth recording why review missed it: a one-invocation whole-workspace build +**unifies cargo features**, so the regex engine is always on and the binding is +always read. Only the per-package `-p perry --bins` command, which does not get +that unification, sees it. This is the same trap as the `cfg(test)` one — the +narrower command is the one that tells the truth. diff --git a/changelog.d/10817-manifest-drift-fix.md b/changelog.d/10817-manifest-drift-fix.md new file mode 100644 index 0000000000..1a627aeb05 --- /dev/null +++ b/changelog.d/10817-manifest-drift-fix.md @@ -0,0 +1,17 @@ +**Manifest drift fix + gate relocation (#463/#512):** `API_MANIFEST` was missing 15 entries that +exist in `NATIVE_MODULE_TABLE` — 5 from `b36554a2d7` (node:http client `rawHeaders`/ +`httpVersionMajor`/`httpVersionMinor`/`complete`, #10467/#10468/#10469) and 10 from `64ca0ebfe7` +(net.Socket surface cluster: `prependListener`/`prependOnceListener`/`pipe`/`unpipe`/`writable`/ +`readable`/`writableEnded`/`readableEnded`/`_writableState`/`_readableState`, +#10441/#10442/#10444/#10465). Added to `crates/perry-api-manifest/src/entries/part_1.rs` and +`part_4.rs`, matching the file's existing convention of representing a zero-arg +`NativeMethodCall` property read as `ApiKind::Method { has_receiver: true, .. }`. + +Nothing caught this drift when it landed because `every_dispatch_entry_has_manifest_counterpart` +lived in `crates/perry-codegen/tests/manifest_consistency.rs`, an integration suite CI's +`e2e-scoped` only runs per-PR when the diff names that file — the one file a PR that merely adds +`NATIVE_MODULE_TABLE` rows has no reason to touch. Moved the check to a `#[cfg(test)]` unit test +(`crates/perry-codegen/src/manifest_consistency.rs`) so it runs on every `cargo-test` invocation +regardless of diff scope; `perry-codegen` already depends on `perry-api-manifest` as an ordinary +dependency, so no new dependency edge was needed. The integration test's copy was removed (its +trigger condition was a strict subset of the unit test's); the file's other checks are unchanged. diff --git a/changelog.d/10817-regen-api-docs.md b/changelog.d/10817-regen-api-docs.md new file mode 100644 index 0000000000..df91910e66 --- /dev/null +++ b/changelog.d/10817-regen-api-docs.md @@ -0,0 +1,16 @@ +Regenerated `docs/src/api/reference.md` for the 15 dispatch-table entries this +change adds: **2855 → 2870**. `docs/api/perry.d.ts` is unchanged at 2026, which +is correct — the added rows are dispatch-table entries, not public API surface. + +The PR added the entries without regenerating, so `lint`'s "Check for API docs +drift" step (`git diff --quiet -- docs/src/api/reference.md docs/api/perry.d.ts`) +failed on the assembled tree. + +Regenerated from a built binary, not hand-edited, and checked for the failure +mode that gate has: `scripts/regen_api_docs.sh` hardcodes +`/target/release/perry`, and when that binary is absent it +regenerates from nothing and leaves both files **truncated**. A real +regeneration moves the header counts and leaves the tail intact; truncation +cuts the end. Both tails were verified present afterwards, and `perry.d.ts` +staying at 2026 is the corroboration — a truncating run would have emptied it +too. diff --git a/changelog.d/10818-callback-bound-dispatch-skip-vec-alloc.md b/changelog.d/10818-callback-bound-dispatch-skip-vec-alloc.md new file mode 100644 index 0000000000..9fc376b138 --- /dev/null +++ b/changelog.d/10818-callback-bound-dispatch-skip-vec-alloc.md @@ -0,0 +1,76 @@ +Removed a per-call heap allocation from `dispatch_bound_function` +(`crates/perry-runtime/src/closure/dispatch/bound.rs`), the runtime entry +every `js_closure_call` routes a `Function.prototype.bind` result through. + +The function unconditionally built a `Vec` (`Vec::with_capacity(args.len() ++ 4)`, a push loop over any bound args, then `extend_from_slice(args)`) before +calling the bound target, even for the overwhelmingly common `.bind(thisArg)` +shape with **no** partial-applied arguments — a plain method reference +(`arr.forEach(obj.method.bind(obj))`). `js_function_bind` already leaves +capture slot 2 (`bound_args_ptr`) null whenever no extra args were bound, so +that case now skips the allocate/copy/free cycle entirely and passes the +caller's own `args` slice straight through to `js_native_call_value`. The +actual partial-application shape (`.bind(obj, extra)`) is untouched other than +a tighter `Vec` capacity hint (`n + args.len()` instead of the old +`args.len() + 4`). + +This targets the "method reference passed as callback" shape specifically: +unlike a plain closure held in a local, a `.bind()`-created function is +excluded from `direct.rs`'s per-loop dispatch hoisting (`BoundFunction` is +deliberately not resolved by `resolve_direct_func_ptr` — +`crates/perry-runtime/src/closure/dispatch/direct.rs:70`, inside the +`func_ptr.is_null() || func_ptr == BOUND_METHOD_FUNC_PTR || func_ptr == +BOUND_FUNCTION_FUNC_PTR` early-return at lines 68-73), so `arr.forEach(fn)` +over a bound method re-runs this allocation on every element. + +**Aliasing / rooting**: traced every one of `dispatch_bound_function`'s 9 +call sites (exhaustive repo grep, not just perry-runtime). Every one hands it +either a `[f64; N]` literal array of by-value `f64` parameters living on the +Rust native stack (`closure/dispatch/calln.rs`'s per-arity entry points, +lines 38/83/151/181/215/255/299, and `dispatch_registered_call`'s 8 callers +at lines 382/415/449/496/549/606/665/727, each building `let args = [arg0, +arg1, ...]`) or a freshly Rust-`Vec`-allocated copy (`value_call.rs`'s +`full`, built by a `push` loop over `a(i)` before dispatch, for the >16-arg / +dynamic-call path). Neither shape is ever GC-managed memory — the collector +only owns memory it allocates itself (the `js_*_alloc` family into the +arena/nursery/old-gen) and has no knowledge of the Rust stack or +`Vec`/`Box` heap. So `args`'s *backing storage* can't be moved or freed by a +collection inside this call on any path, before or after this change, and +handing `js_native_call_value` the caller's slice directly instead of a +byte-copy of it is safe. + +That is a narrower claim than "GC-safe" and worth stating precisely: a stack +array of NaN-boxed values is not a GC root, and neither was the old `Vec` +copy. If a collection *moves* an object referenced by one of the argument +*values* during `js_native_call_value` (e.g. inside `rebind_explicit_this`, +which can allocate), neither the old buffer nor the new one gets its bits +rewritten — copying bytes into a fresh `Vec` is not registering a root, so it +never protected against that. This change neither introduces nor fixes that +pre-existing exposure; it is identical before and after (and the conservative +native-stack scan that could theoretically cover it is diagnostic-only by +default, `Auto` → `SkipDisabled`). + +Measured with the repo's differential probe technique (marginal cost isolated +from loop overhead, instructions retired plus wall/CPU time under a +measurement mutex, best-of-N): the `.bind(obj)`-with-no-extra-args shape drops +from ~2795 to ~2676 instructions per call (~4.3%), reproduced across two +independent runs (N=50000 and N=250000, 7 and 15 reps). Wall-clock time on the +measurement host was noisy under heavy unrelated contention (system load +averaged 60-117 on a 10-core box) and did not resolve a stable direction; the +more contention-robust process CPU-time metric (user+sys) showed no +regression (flat-to-favorable across both runs). The partially-applied-bind +shape (`.bind(obj, extra)`), the already-optimized loop-local-closure shape, +and a bare-loop control all measured ~0 delta, confirming the change is +isolated to its target shape. + +Added `test-files/test_gap_callback_dispatch_shapes.ts`, byte-identical +against node 26.5.1, covering: direct arrow inline to a builtin array method, +a closure held in a local (once and in a loop), a callback threaded through a +second function frame, a callback parameter called directly in a loop, a +`.bind()` method reference with and without partial args, `this` binding +across arrow/ordinary/bound call shapes (including the receiverless-call +`this === undefined` case), `arguments`/extra/missing-argument/`.length` +handling, a callback that throws through one and two frames, recursion +through a plain and a bound callback reference, closures capturing a loop +variable (`let` and the classic `var` + IIFE idiom), and a bound method used +as a hot per-element `forEach` callback. diff --git a/changelog.d/10820-diag-feature-scope-comment.md b/changelog.d/10820-diag-feature-scope-comment.md new file mode 100644 index 0000000000..7e98417858 --- /dev/null +++ b/changelog.d/10820-diag-feature-scope-comment.md @@ -0,0 +1,28 @@ +**docs(runtime): correct the `diagnostics` feature comment's scope.** +`crates/perry-runtime/Cargo.toml`'s comment above `diagnostics = []` asserted +"None are on a hot path". That is true of the things it enumerates +(`PERRY_GC_DIAG`, the typed-feedback dump, the v8 heap-snapshot builder, +`process.report`) and false as a blanket statement, because the feature does +**not** gate `hot_diag` at all: `lib.rs` declares `pub mod hot_diag;` with no +cfg, the file contains zero `cfg(feature` occurrences, and `enum_on()` is +called per string concat (`string/concat.rs:688`, `:1052`) and per property +enumeration in **every** build, shipped included. + +This matters because the comment is load-bearing in people's reasoning: a peer +session quoted a neighbouring `hot_diag` comment as evidence about generated +code and proposed a fix on that basis. Comments here are read as evidence, so +a blanket claim that is only true of an enumerated subset is worth narrowing. + +**No performance claim attached, deliberately.** The obvious follow-up — +collapsing `enum_on()`'s `OnceLock`-probe-plus-`AtomicBool`-load into a single +three-state `AtomicU8` with a `#[cold] #[inline(never)]` resolve arm — was +implemented and measured against its exact parent commit: **399.76 → 401.19 +instructions per short concat**, bare-loop control 3.01 / 2.99 in both arms. +That is +0.4% on a ~400-instruction operation, indistinguishable from zero, so +the change was reverted rather than shipped as churn. + +The instructive part is why the theory was wrong. A peer measured a real win +(zod −5.2%, an S40 fixture −12.4%) from *deleting* an early-returning +diagnostic gate. That win came from removing the call and its inlining +barrier entirely — not from making the gate's body cheaper. Saving one load +out of four hundred instructions is below what any probe here resolves. diff --git a/crates/perry-api-manifest/src/entries/part_1.rs b/crates/perry-api-manifest/src/entries/part_1.rs index a3a8f12bb0..1883b231de 100644 --- a/crates/perry-api-manifest/src/entries/part_1.rs +++ b/crates/perry-api-manifest/src/entries/part_1.rs @@ -710,6 +710,26 @@ pub(crate) const API_MANIFEST_PART_1: &[ApiEntry] = &[ method("net", "getX509Certificate", true, Some("Socket")), method("net", "getPeerX509Certificate", true, Some("Socket")), method("net", "setKeyCert", true, Some("Socket")), + // #10441/#10442 — front-inserting listener variants (net.Socket is + // an EventEmitter), and #10444 pipe/unpipe (net.Socket is a + // stream.Duplex). Absent entirely pre-fix: a typed `net.Socket` + // receiver fell through to a plain property read for these names + // and got `undefined` instead of dispatching. + method("net", "prependListener", true, Some("Socket")), + method("net", "prependOnceListener", true, Some("Socket")), + method("net", "pipe", true, Some("Socket")), + method("net", "unpipe", true, Some("Socket")), + // #10465 — writable/readable/writableEnded/readableEnded/ + // _writableState/_readableState state accessors. No class_filter + // in the dispatch table (native_table/net_events.rs) — same + // class_filter: None shape the generic `stream` module rows use + // for their own writable/readable/writableEnded/readableEnded. + method("net", "writable", true, None), + method("net", "readable", true, None), + method("net", "writableEnded", true, None), + method("net", "readableEnded", true, None), + method("net", "_writableState", true, None), + method("net", "_readableState", true, None), // Issue #1123 followup — `net.Server` instance methods backing // `createServer(...).listen/.close/.address/.on`. Mirrors the // shape of the http-server rows at entries.rs:2298. The diff --git a/crates/perry-api-manifest/src/entries/part_4.rs b/crates/perry-api-manifest/src/entries/part_4.rs index 19e043aada..be4c50cc09 100644 --- a/crates/perry-api-manifest/src/entries/part_4.rs +++ b/crates/perry-api-manifest/src/entries/part_4.rs @@ -874,6 +874,16 @@ pub(crate) const API_MANIFEST_PART_4: &[ApiEntry] = &[ method("http", "statusMessage", true, Some("IncomingMessage")), method("http", "headers", true, Some("IncomingMessage")), method("http", "trailers", true, Some("IncomingMessage")), + // #10467 — client-side rawHeaders/httpVersionMajor/httpVersionMinor/ + // complete bare-name accessors (paired with the __get_rawHeaders row + // below). Previously only the __get_* HIR-rewrite targets existed for + // httpVersionMajor/httpVersionMinor/complete and rawHeaders had no + // manifest row at all, so a typed `IncomingMessage` receiver reading + // the bare property missed dispatch entirely. + method("http", "rawHeaders", true, Some("IncomingMessage")), + method("http", "httpVersionMajor", true, Some("IncomingMessage")), + method("http", "httpVersionMinor", true, Some("IncomingMessage")), + method("http", "complete", true, Some("IncomingMessage")), method("http", "setStatus", true, Some("ServerResponse")), method("http", "getStatus", true, Some("ServerResponse")), method("http", "__get_method", true, Some("IncomingMessage")), @@ -898,6 +908,7 @@ pub(crate) const API_MANIFEST_PART_4: &[ApiEntry] = &[ method("http", "__get_statusMessage", true, Some("IncomingMessage")), method("http", "__get_headers", true, Some("IncomingMessage")), method("http", "__get_trailers", true, Some("IncomingMessage")), + method("http", "__get_rawHeaders", true, Some("IncomingMessage")), method("http", "__get_statusCode", true, Some("ServerResponse")), method("http", "__set_statusCode", true, Some("ServerResponse")), method("http", "__set_statusMessage", true, Some("ServerResponse")), diff --git a/crates/perry-codegen/src/expr/bigint_set.rs b/crates/perry-codegen/src/expr/bigint_set.rs index 4184dcd86f..f8060de878 100644 --- a/crates/perry-codegen/src/expr/bigint_set.rs +++ b/crates/perry-codegen/src/expr/bigint_set.rs @@ -16,8 +16,8 @@ use crate::type_analysis::{ use crate::types::{DOUBLE, F32, I1, I32, I64, PTR}; use super::{ - can_lower_expr_as_i32, i32_bool_to_nanbox, lower_expr, lower_expr_native, nanbox_bigint_inline, - nanbox_pointer_inline, record_collection_number_key_fallback, + can_lower_expr_as_i32, i32_bool_to_nanbox, lower_array_literal, lower_expr, lower_expr_native, + nanbox_bigint_inline, nanbox_pointer_inline, record_collection_number_key_fallback, record_collection_number_key_selected, record_collection_string_key_fallback, record_collection_string_key_selected, record_collection_typed_value_fallback, record_collection_typed_value_selected, unbox_collection_receiver, unbox_to_i64, FnCtx, @@ -437,29 +437,52 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { object, exclude_keys, } => { - let obj_box = lower_expr(ctx, object)?; - let key_handle_globals: Vec = exclude_keys + // `exclude_keys` is ALWAYS statically-named property strings — + // never a computed key. The HIR lowering that populates this + // field (`destructuring/pattern_binding.rs`'s `Pat::Object` + // arm) only ever pushes `PropName::Ident`/`Str`/`Num` onto + // `static_keys`; a `{ [k]: v, ...rest }` computed key goes + // through a completely separate path (`computed_key_temps` + + // a `delete` on the already-built rest object, #6153) that + // never touches this field. So every element built below is + // guaranteed to be a literal `Expr::String`, not merely + // "usually" one. + // + // Build the excluded-key array FIRST, the same way a literal + // array of the same keys (`[k1, k2, ...]`) is already built: + // one inline bump allocation plus N `store double` (the + // all-literal shape `lower_array_literal` takes when every + // element is `Expr::String` — pooled interned-string handles, + // never an allocation of their own, so no operand rooting is + // needed around them either; see #6951's "emits nothing for + // the all-literal / all-local shapes"). That replaces one + // `js_array_alloc_with_length` call plus one + // `js_array_set_f64_unchecked` call PER excluded key: each of + // those per-key calls re-derived and re-bounds-checked a + // receiver this site had just allocated itself, so every + // check inside (`frozen?`, `has index descriptors?`, `index + // in range?`) was statically true here. + // + // Doing this before lowering `object` — rather than after, as + // the call-by-call version did — also means `object`'s + // pointer is derived AFTER the only allocation left in this + // expression, not cached across it. + let key_exprs: Vec = exclude_keys .iter() - .map(|k| { - let idx = ctx.strings.intern(k); - format!("@{}", ctx.strings.entry(idx).handle_global) - }) + .map(|k| Expr::String(k.clone())) .collect(); + let keys_arr_boxed = lower_array_literal(ctx, &key_exprs)?; + let keys_arr = { + let blk = ctx.block(); + let bits = blk.bitcast_double_to_i64(&keys_arr_boxed); + blk.and(I64, &bits, POINTER_MASK_I64) + }; + let obj_box = lower_expr(ctx, object)?; let blk = ctx.block(); let obj_handle = { let bits = blk.bitcast_double_to_i64(&obj_box); blk.and(I64, &bits, POINTER_MASK_I64) }; - let n_str = (exclude_keys.len() as u32).to_string(); - let keys_arr = blk.call(I64, "js_array_alloc_with_length", &[(I32, &n_str)]); - for (i, handle_global) in key_handle_globals.iter().enumerate() { - let idx_str = i.to_string(); - let key_box = blk.load(DOUBLE, handle_global); - blk.call_void( - "js_array_set_f64_unchecked", - &[(I64, &keys_arr), (I32, &idx_str), (DOUBLE, &key_box)], - ); - } let rest_ptr = blk.call( I64, "js_object_rest", diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 1ed8982196..25a3b55ec4 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -28,6 +28,11 @@ pub(crate) mod lower_call; pub(crate) mod lower_conditional; pub(crate) mod lower_string_concat; pub(crate) mod lower_string_method; +/// #463/#512 dispatch-table/manifest drift check — see the module docs +/// for why this moved here from an integration test (#10668's 15-row +/// drift, which nothing caught until it surfaced on an unrelated PR). +#[cfg(test)] +mod manifest_consistency; pub mod module; pub mod nanbox; #[cfg(feature = "llvm-inprocess")] diff --git a/crates/perry-codegen/src/manifest_consistency.rs b/crates/perry-codegen/src/manifest_consistency.rs new file mode 100644 index 0000000000..a2ebd94b40 --- /dev/null +++ b/crates/perry-codegen/src/manifest_consistency.rs @@ -0,0 +1,83 @@ +//! In-crate coverage for the #463/#512 manifest-drift check that +//! `crates/perry-codegen/tests/manifest_consistency.rs` used to own alone. +//! +//! # Why this exists (a gate that could not fail on the change that broke it) +//! +//! `every_dispatch_entry_has_manifest_counterpart` compares two tables: +//! `NATIVE_MODULE_TABLE` (this crate's dispatch table, walked through +//! [`crate::iter_native_method_signatures`]) against +//! `perry_api_manifest::API_MANIFEST`. By construction it can be tripped by +//! an edit to EITHER table — but as an integration test under +//! `crates/perry-codegen/tests/`, CI's `e2e-scoped` only runs an integration +//! suite per-PR when the diff names it (CLAUDE.md: "Integration suites under +//! `crates/*/tests/*.rs` run per-PR only when the diff names them"). The one +//! file a drifting PR will almost never touch is `manifest_consistency.rs` +//! itself — a PR that adds `NATIVE_MODULE_TABLE` rows has no reason to edit +//! the test file that checks them. #10668 (node:http client response +//! surface + net.Socket surface cluster) landed 15 such rows, and nothing +//! caught the drift until it happened to surface on a later, unrelated PR +//! that ran in the same CI job. +//! +//! This is a fifth way a gate can be unable to fail, distinct from the four +//! CLAUDE.md already tracks under "Four ways a gate can be unable to fail": +//! there the gate itself is broken (`continue-on-error`, not required, +//! cancelled, or its subject never runs). Here the gate is fine — it runs, +//! it can go red, it's required — and the change class it exists to guard +//! simply never triggers it, because trigger condition and subject are +//! disjoint by construction. +//! +//! Moving the assertion into a `#[cfg(test)]` unit test inside this crate +//! puts it on the `cargo-test`-visible per-PR gate unconditionally +//! (CLAUDE.md again: "Prefer putting acceptance coverage in +//! `cargo-test`-visible unit tests (#5960)"). `perry-codegen` already +//! depends on `perry-api-manifest` as an ordinary (non-dev) dependency — see +//! this crate's `Cargo.toml` — so reaching `API_MANIFEST` from here adds no +//! new dependency edge. +//! +//! The integration test's copy of this same check was removed rather than +//! kept as a duplicate: its trigger condition is a strict subset of this +//! module's (this module runs on every PR; the integration test ran only on +//! PRs that touched its own file), so a passing integration-test copy could +//! never catch anything this module doesn't already catch first. The +//! integration file's other checks — `manifest_param_counts_match_dispatch_table`, +//! the reverse-direction module/binding checks — are unaffected and stay +//! where they are; see that file's header for why. + +use perry_api_manifest::{ApiKind, API_MANIFEST}; + +#[test] +fn every_dispatch_entry_has_manifest_counterpart() { + let mut missing: Vec = Vec::new(); + + for sig in crate::iter_native_method_signatures() { + // Look for a manifest entry on the same (module, name) where + // the kind is Method with matching has_receiver. class_filter + // mismatches across rows of the same (module, method) pair are + // expected — the dispatch table specializes by class, the + // manifest does not. + let hit = API_MANIFEST.iter().any(|e| { + e.module == sig.module + && e.name == sig.method + && matches!( + e.kind, + ApiKind::Method { has_receiver, .. } if has_receiver == sig.has_receiver + ) + }); + if !hit { + let cls = sig.class_filter.unwrap_or("-"); + missing.push(format!( + "{}::{} (has_receiver={}, class_filter={})", + sig.module, sig.method, sig.has_receiver, cls + )); + } + } + + assert!( + missing.is_empty(), + "API_MANIFEST is missing {} entry/entries that exist in NATIVE_MODULE_TABLE:\n {}\n\n\ + Add the missing rows to crates/perry-api-manifest/src/entries.rs — \ + drift here would make the unimplemented-API check (#463) error on real implementations.", + missing.len(), + missing.join("\n ") + ); +} diff --git a/crates/perry-codegen/tests/manifest_consistency.rs b/crates/perry-codegen/tests/manifest_consistency.rs index 043a61cf82..c8db1816e1 100644 --- a/crates/perry-codegen/tests/manifest_consistency.rs +++ b/crates/perry-codegen/tests/manifest_consistency.rs @@ -3,15 +3,31 @@ //! Every row of `NATIVE_MODULE_TABLE` (the static dispatch table in //! `lower_call.rs`) must have a counterpart entry in `API_MANIFEST`, //! otherwise the unimplemented-API check would error on a real -//! implementation. This file covers two drifts: +//! implementation. //! -//! 1. `every_dispatch_entry_has_manifest_counterpart` — by name only; -//! catches new dispatch rows that nobody added to the manifest. -//! 2. `manifest_param_counts_match_dispatch_table` (#512) — for +//! **`every_dispatch_entry_has_manifest_counterpart` moved** to +//! `perry_codegen::manifest_consistency`, a `#[cfg(test)]` unit test in +//! `crates/perry-codegen/src/manifest_consistency.rs` — see that module's +//! doc comment for the full reasoning. Short version: as an integration test +//! here, it only ran per-PR when the diff named this file, which is the one +//! file a drifting PR (one that only adds `NATIVE_MODULE_TABLE` rows) has no +//! reason to touch. #10668 landed 15 such rows and nothing caught it until an +//! unrelated PR happened to run this suite. The unit test runs on every +//! `cargo-test` invocation regardless of which files the diff touches. +//! +//! This file still covers: +//! +//! 1. `manifest_param_counts_match_dispatch_table` (#512) — for //! auto-derivable rows (`has_receiver: false`, no class filter) the //! manifest's `params.len()` must match the dispatch table's args //! arity, so the generated `.d.ts` doesn't claim a different shape -//! than what codegen actually accepts. +//! than what codegen actually accepts. (Same disjoint-trigger gap as +//! the moved check applies here too — left as an integration test for +//! now, out of scope for this pass.) +//! 2. `every_native_module_has_at_least_one_manifest_entry` (#513) — the +//! reverse-direction structural check. +//! 3. `cjs_style_node_builtins_have_default_entries`. +//! 4. `every_well_known_binding_has_manifest_entry` (#513). //! //! Class-filtered duplicates collapse to one manifest entry — the //! manifest tracks "is this method known on this module?", not the @@ -20,43 +36,6 @@ use perry_api_manifest::{ApiKind, ParamSpec, TypeSpec, API_MANIFEST}; use perry_codegen::iter_native_method_signatures; -#[test] -fn every_dispatch_entry_has_manifest_counterpart() { - let mut missing: Vec = Vec::new(); - - for sig in iter_native_method_signatures() { - // Look for a manifest entry on the same (module, name) where - // the kind is Method with matching has_receiver. class_filter - // mismatches across rows of the same (module, method) pair are - // expected — the dispatch table specializes by class, the - // manifest does not. - let hit = API_MANIFEST.iter().any(|e| { - e.module == sig.module - && e.name == sig.method - && matches!( - e.kind, - ApiKind::Method { has_receiver, .. } if has_receiver == sig.has_receiver - ) - }); - if !hit { - let cls = sig.class_filter.unwrap_or("-"); - missing.push(format!( - "{}::{} (has_receiver={}, class_filter={})", - sig.module, sig.method, sig.has_receiver, cls - )); - } - } - - assert!( - missing.is_empty(), - "API_MANIFEST is missing {} entry/entries that exist in NATIVE_MODULE_TABLE:\n {}\n\n\ - Add the missing rows to crates/perry-api-manifest/src/entries.rs — \ - drift here would make the unimplemented-API check (#463) error on real implementations.", - missing.len(), - missing.join("\n ") - ); -} - /// #512: for every auto-derivable dispatch row (no receiver, no class /// filter) the manifest's `params` length must match the dispatch /// table's args length, AND each `NA_STR` in the dispatch table must diff --git a/crates/perry-runtime/Cargo.toml b/crates/perry-runtime/Cargo.toml index 8b5e6d75ce..ed59532439 100644 --- a/crates/perry-runtime/Cargo.toml +++ b/crates/perry-runtime/Cargo.toml @@ -133,7 +133,13 @@ bun-cli-utils = [ # pulled only by them, which dead-strips when unreferenced): GC cycle telemetry # (`PERRY_GC_DIAG`), typed-feedback trace dump (`PERRY_TYPED_FEEDBACK`), the v8 # heap-snapshot builder (`v8.getHeapSnapshot`/`writeHeapSnapshot`), and -# `process.report`. None are on a hot path. The env-driven dev diagnostics +# `process.report`. None of THOSE are on a hot path. Note this feature does +# NOT gate `hot_diag`, which is declared unconditionally (`pub mod hot_diag;` +# in lib.rs, no cfg) and whose `enum_on()` runs per string concat +# (`string/concat.rs`) and per property enumeration in every build, shipped +# included. Measured at under 0.4% of a short concat, i.e. below probe +# resolution -- so this is a scope correction to the comment, not a +# performance claim. The env-driven dev diagnostics # degrade gracefully when off (auto-optimize leaves this off unless the program # uses a heap-snapshot / `process.report` API, which the compiler detects). diagnostics = [] diff --git a/crates/perry-runtime/src/closure/dispatch/bound.rs b/crates/perry-runtime/src/closure/dispatch/bound.rs index c73f17a5bb..60b7c356ff 100644 --- a/crates/perry-runtime/src/closure/dispatch/bound.rs +++ b/crates/perry-runtime/src/closure/dispatch/bound.rs @@ -306,14 +306,38 @@ pub unsafe fn dispatch_bound_function(closure: *const ClosureHeader, args: &[f64 // Collect the partial-applied (bound) leading args, then append the // call-time args. `g = f.bind(obj, 2); g(3)` calls `f` with `(2, 3)`. - let mut combined: Vec = Vec::with_capacity(args.len() + 4); - if !bound_args_ptr.is_null() { + // + // The overwhelmingly common shape is `.bind(thisArg)` with NO partial + // args at all — a plain method reference (`arr.forEach(obj.method.bind( + // obj))`), the callback shape `direct.rs` cannot hoist out of a loop + // (BoundFunction is deliberately excluded from `resolve_direct_func_ptr` + // — see that module's doc), so this function runs on every element. + // `js_function_bind` leaves capture slot 2 (`bound_args_ptr`) null + // whenever `bound_arg_count == 0`, so that's exactly the free-to-detect + // case: skip the allocate-copy-free `Vec` and hand `js_native_call_value` + // the caller's own `args` slice directly. Only the actual + // partial-application shape (`.bind(obj, extra)`) still needs a combined + // buffer. + let mut combined: Vec; + let (call_ptr, call_len): (*const f64, usize) = if bound_args_ptr.is_null() { + if args.is_empty() { + (std::ptr::null(), 0) + } else { + (args.as_ptr(), args.len()) + } + } else { let n = crate::array::js_array_length(bound_args_ptr) as usize; + combined = Vec::with_capacity(n + args.len()); for i in 0..n { combined.push(crate::array::js_array_get_f64(bound_args_ptr, i as u32)); } - } - combined.extend_from_slice(args); + combined.extend_from_slice(args); + if combined.is_empty() { + (std::ptr::null(), 0) + } else { + (combined.as_ptr(), combined.len()) + } + }; // A bound concise/object-literal method reads `this` from its baked capture // slot, not IMPLICIT_THIS — rebind it to the bound receiver so the bound @@ -321,11 +345,6 @@ pub unsafe fn dispatch_bound_function(closure: *const ClosureHeader, args: &[f64 let target = rebind_explicit_this(target, bound_this); let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445 let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(bound_this)); - let (call_ptr, call_len) = if combined.is_empty() { - (std::ptr::null::(), 0usize) - } else { - (combined.as_ptr(), combined.len()) - }; let result = js_native_call_value(target, call_ptr, call_len); crate::object::js_implicit_this_set(prev_this.get_nanbox_f64()); result diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 7236eebea4..d4b76b895f 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -821,9 +821,23 @@ fn is_safe_numeric_key(bits: u64) -> bool { // Pre-fix `Map.set("key_" + i, …)` over 500k inserts was O(N²) because // each `set` did a linear `find_key_index` to dedup-check; with this // table the dedup probe is O(1) amortized. +// +// The inner map is keyed by `u64`, but that key is not raw input — it is +// already the FNV-1a content hash above, a well-avalanched 64-bit value. +// `std::collections::HashMap`'s default `RandomState` (SipHash) is built to +// resist adversarial *byte* input; hashing an already-mixed hash through it +// a second time buys nothing here and was costing every `Map.get`/`set`/ +// `has`/`delete` on a string-keyed map past `SIDE_TABLE_THRESHOLD` a second, +// unrelated hash computation. `NumericIndex.hashed` next door already uses +// `PtrHasher` for exactly this reason (u64-keyed, no adversarial input); this +// table gets the same treatment. `PtrHasher::write_u64` is one multiply plus +// an xorshift avalanche step — see `fast_hash.rs`'s `mix` doc comment for why +// the avalanche still matters even though FNV-1a is already well-distributed +// (HashMap reads bucket indices from the LOW bits, which a pure multiply +// under-mixes for some input distributions). crate::perry_thread_local! { static MAP_STRING_INDEX: RefCell< - crate::fast_hash::PtrHashMap>>, + crate::fast_hash::PtrHashMap>>, > = RefCell::new(crate::fast_hash::new_ptr_hash_map()); } @@ -1597,7 +1611,7 @@ pub extern "C" fn js_map_alloc(capacity: u32) -> *mut MapHeader { // and reached directly through the header above. MAP_STRING_INDEX.with(|idx| { idx.borrow_mut() - .insert(ptr as usize, std::collections::HashMap::new()); + .insert(ptr as usize, crate::fast_hash::new_ptr_hash_map()); }); MAP_PTR_INDEX.with(|idx| { idx.borrow_mut() @@ -2131,7 +2145,7 @@ unsafe fn map_set_string_key_value( let mut idx = idx.borrow_mut(); let slot = idx .entry(map as usize) - .or_insert_with(std::collections::HashMap::new); + .or_insert_with(crate::fast_hash::new_ptr_hash_map); slot.entry(h).or_insert_with(Vec::new).push(used); }); } @@ -2253,7 +2267,7 @@ fn map_set_resolved(map: *mut MapHeader, key: f64, value: f64) { let mut idx = idx.borrow_mut(); let slot = idx .entry(map as usize) - .or_insert_with(std::collections::HashMap::new); + .or_insert_with(crate::fast_hash::new_ptr_hash_map); slot.entry(h).or_insert_with(Vec::new).push(used); }); } @@ -3688,6 +3702,82 @@ mod tests { } } + /// MAP_STRING_INDEX's inner table switched from `std::collections:: + /// HashMap` (SipHash) to `PtrHashMap` (a cheap multiplicative hasher) so + /// every string-keyed `Map.get`/`set`/`has`/`delete` past + /// `SIDE_TABLE_THRESHOLD` stops paying for a second, redundant hash of + /// an already-hashed FNV-1a value. `HashMap::get` re-checks + /// `K: Eq` on every candidate regardless of `S`, so a hasher swap cannot + /// change *which* key a lookup resolves to -- only how fast it gets + /// there -- but this pins that down empirically at a scale (10,000+ + /// distinct keys, forced far past `SIDE_TABLE_THRESHOLD` and any small + /// std-HashMap capacity) where bucket collisions in BOTH the outer + /// (map-pointer-keyed) and inner (content-hash-keyed) tables are a + /// certainty, not a contrived edge case. If a bucket collision at either + /// level silently returned the wrong entry, or if switching hashers + /// somehow let two live keys shadow each other, this test fails. + #[test] + fn string_index_resolves_every_key_correctly_past_the_hashed_threshold() { + let map = js_map_alloc(4); + const COUNT: usize = 10_000; + let mut keys: Vec<*const StringHeader> = Vec::with_capacity(COUNT); + for i in 0..COUNT { + let content = format!("string-index-key-{i}"); + let key = js_string_from_bytes(content.as_ptr(), content.len() as u32); + js_map_set_string_number(map, key, i as f64); + keys.push(key); + } + assert_eq!(js_map_size(map), COUNT as u32); + assert!(COUNT as u32 > SIDE_TABLE_THRESHOLD); + + // Every inserted key still resolves to its OWN distinct value, in + // reverse-insertion order (exercises the hashed side table, not + // append-order luck). + for i in (0..COUNT).rev() { + assert_eq!( + js_map_get_string_key(map, keys[i]), + i as f64, + "key {i} resolved to the wrong value -- a bucket collision \ + returned a neighbor's entry instead of missing or matching" + ); + assert_eq!(js_map_has_string_key(map, keys[i]), 1); + } + + // A content-equal-but-freshly-allocated key (distinct pointer from + // the one stored at insert time) must still resolve by content -- + // the outer hasher change must not have started keying by identity. + for i in [0usize, COUNT / 2, COUNT - 1] { + let content = format!("string-index-key-{i}"); + let fresh = js_string_from_bytes(content.as_ptr(), content.len() as u32); + assert_ne!(fresh as usize, keys[i] as usize); + assert_eq!(js_map_get_string_key(map, fresh), i as f64); + } + + // Absent keys that share a long common prefix with real entries + // (adversarial-ish for a byte-at-a-time hash) must still miss. + for i in 0..50 { + let content = format!("string-index-key-{i}-absent"); + let missing = js_string_from_bytes(content.as_ptr(), content.len() as u32); + assert_eq!(js_map_get_string_key(map, missing).to_bits(), TAG_UNDEFINED); + assert_eq!(js_map_has_string_key(map, missing), 0); + } + + // Delete half the keys, then confirm the survivors are still exact + // and the deleted ones are definitively gone (forces + // `compact_map_entries`'s side-table rebuild at this scale too). + for i in (0..COUNT).step_by(2) { + assert_eq!(js_map_delete_string_key(map, keys[i]), 1); + } + assert_eq!(js_map_size(map), (COUNT / 2) as u32); + for i in 0..COUNT { + if i % 2 == 0 { + assert_eq!(js_map_has_string_key(map, keys[i]), 0); + } else { + assert_eq!(js_map_get_string_key(map, keys[i]), i as f64); + } + } + } + #[test] fn clear_resets_every_index_whatever_the_key_kinds() { // Numeric-only map: cleared without touching the side-tables; the diff --git a/crates/perry-runtime/src/node_stream_constructors.rs b/crates/perry-runtime/src/node_stream_constructors.rs index 5476ca631f..1dae81e4f5 100644 --- a/crates/perry-runtime/src/node_stream_constructors.rs +++ b/crates/perry-runtime/src/node_stream_constructors.rs @@ -362,11 +362,11 @@ mod web_adapter; pub use builders::{ js_array_subclass_init, js_event_emitter_async_resource_subclass_init, js_event_emitter_subclass_init, js_node_stream_duplex_new, js_node_stream_duplex_subclass_init, - js_node_stream_passthrough_new, js_node_stream_readable_from, - js_node_stream_readable_from_options, js_node_stream_readable_new, - js_node_stream_readable_subclass_init, js_node_stream_transform_new, - js_node_stream_transform_subclass_init, js_node_stream_writable_new, - js_node_stream_writable_subclass_init, + js_node_stream_legacy_subclass_init, js_node_stream_passthrough_new, + js_node_stream_readable_from, js_node_stream_readable_from_options, + js_node_stream_readable_new, js_node_stream_readable_subclass_init, + js_node_stream_transform_new, js_node_stream_transform_subclass_init, + js_node_stream_writable_new, js_node_stream_writable_subclass_init, }; pub use introspection::{ diff --git a/crates/perry-runtime/src/node_stream_constructors/builders.rs b/crates/perry-runtime/src/node_stream_constructors/builders.rs index bae862622e..0f9a5bff6a 100644 --- a/crates/perry-runtime/src/node_stream_constructors/builders.rs +++ b/crates/perry-runtime/src/node_stream_constructors/builders.rs @@ -93,6 +93,37 @@ pub extern "C" fn js_event_emitter_subclass_init(this: f64) -> f64 { this } +/// #10798: install the legacy `node:stream` `Stream` base surface onto +/// `this` for a source-compiled `class X extends Stream` (the bare +/// `node:stream` base — NOT one of its Readable/Writable/Duplex/Transform +/// subclasses). In Node, `Stream` is EventEmitter plus exactly one added +/// prototype method: `pipe()` (`lib/internal/streams/legacy.js`). It has no +/// `_readableState`/`_writableState`/etc, so — unlike the stream-state +/// inits above — there is no option-driven state to seed here either; this +/// is `js_event_emitter_subclass_init` plus the one extra method. `ns_pipe2` +/// is the same generic, receiver-keyed pipe implementation +/// Readable/Duplex/Transform install (`readable_methods`/ +/// `duplex_methods` in `node_stream_readwrite.rs` / +/// `node_stream_duplex_methods.rs`); it drives itself entirely off `on`/ +/// `emit` on the source and destination, so it works unmodified on a plain +/// EventEmitter-shaped receiver that never went through a stream +/// constructor. +#[no_mangle] +pub extern "C" fn js_node_stream_legacy_subclass_init(this: f64) -> f64 { + let raw = raw_ptr_from_value(this); + if raw == 0 { + return this; + } + if unsafe { gc_type_for_ptr(raw) } != Some(crate::gc::GC_TYPE_OBJECT) { + return this; + } + let obj = raw as *mut ObjectHeader; + let mut methods: Vec<(&str, StubFn)> = emitter_methods().to_vec(); + methods.push(("pipe", cast2(ns_pipe2))); + install_methods_on_existing_object(obj, this, &methods, &[]); + this +} + /// Initialize a source-compiled subclass of EventEmitterAsyncResource on its /// already-allocated `this` object. The listener surface remains the generic /// object-backed EventEmitter implementation; a hidden AsyncResource supplies 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 bedfd46ad9..9820df8c63 100644 --- a/crates/perry-runtime/src/object/class_registry/parent_static.rs +++ b/crates/perry-runtime/src/object/class_registry/parent_static.rs @@ -221,13 +221,31 @@ pub extern "C" fn js_register_class_parent_dynamic(class_id: u32, mut parent_val // #10430: the legacy `Stream` constructor extends EventEmitter, so a // `class X extends require('stream')` subclass inherits the same // EventEmitter parent edge (`new X() instanceof EventEmitter`). - let parent = match method.as_str() { - "EventEmitter" | "Stream" => 0xFFFF0076, - "EventEmitterAsyncResource" => 0xFFFF0077, - _ => 0, - }; - if parent != 0 { - register_class(class_id, parent); + // + // #10798: `Stream` gets its OWN hop in the chain — the reserved id + // `instanceof/static_dispatch.rs` already uses to NAME 0xFFFF0070 + // as "Stream" — rather than collapsing straight onto EventEmitter's + // id. `js_instanceof` walks the full class-id chain + // (`subclass_of_builtin_reaches` / `class_chain_reaches`), so + // registering `class_id -> CLASS_ID_STREAM -> CLASS_ID_EVENT_EMITTER` + // keeps `instanceof EventEmitter` true transitively while making + // `instanceof Stream` true ONLY for a genuine `extends Stream` + // subclass — a plain `extends EventEmitter` class (registered + // directly on 0xFFFF0076, no Stream hop) must NOT satisfy + // `instanceof Stream`, and collapsing both onto the same id would + // have made it. The Stream->EventEmitter edge is registered on + // every call; `register_class` no-ops when the edge already + // matches, so this is idempotent. + match method.as_str() { + "EventEmitter" => register_class(class_id, 0xFFFF0076), + "Stream" => { + const CLASS_ID_STREAM: u32 = 0xFFFF0070; + const CLASS_ID_EVENT_EMITTER: u32 = 0xFFFF0076; + register_class(CLASS_ID_STREAM, CLASS_ID_EVENT_EMITTER); + register_class(class_id, CLASS_ID_STREAM); + } + "EventEmitterAsyncResource" => register_class(class_id, 0xFFFF0077), + _ => {} } } return; diff --git a/crates/perry-runtime/src/object/global_this/fetch_globals.rs b/crates/perry-runtime/src/object/global_this/fetch_globals.rs index b8e6d4a3bd..ca5d1051f2 100644 --- a/crates/perry-runtime/src/object/global_this/fetch_globals.rs +++ b/crates/perry-runtime/src/object/global_this/fetch_globals.rs @@ -778,6 +778,29 @@ pub unsafe extern "C" fn js_fetch_or_value_super( // shim reads is never pre-seeded for ANY `PassThrough` heritage shape — // that's a separate, deeper HIR-level gap needing its own fix; adding an // arm here alone was confirmed (empirically) to change nothing. + // + // #10798: `Stream` (the legacy `node:stream` base that `Readable` and + // friends themselves derive from) is a DIFFERENT shape than + // `PassThrough`: it carries no hidden per-instance state at all — in + // Node it is literally `EventEmitter` plus a `pipe()` prototype method + // (`lib/internal/streams/legacy.js`: `Stream(opts) { EventEmitter.call(this, + // opts); }`), so there is no `_readableState`/`_transform`-shaped field + // that needs pre-seeding, and no `js_node_stream_stream_subclass_init` + // is needed (there isn't one, and adding one would duplicate + // `js_event_emitter_subclass_init` for no reason). `canonical_native_parent_name` + // does not list `Stream` either, so — unlike Readable/Writable/Duplex/ + // Transform, which have a fast STATIC path for a plain `import` and only + // fall here for the aliased/namespace/CJS-destructured shapes — every + // `extends Stream` heritage shape (bare ident, namespace member, + // destructured CJS `require`) already reaches this dynamic dispatch + // uniformly. Reuse the existing EventEmitter shim rather than adding a + // stream-specific one: it installs the identical `.on`/`.emit`/`.once`/… + // surface Stream needs, and `pipe()` resolves through the ordinary + // prototype chain once the parent edge is wired (unaffected by this + // arm). `Stream` IS a real constructor with a usable prototype in + // Perry's runtime (`bound_native_callable_export_value("stream", + // "Stream")`, #10430's `new Stream()` fix), so — unlike `PassThrough` — + // this one-line dispatch arm is not a no-op. if let Some((module, method)) = bound_native_parent.as_ref() { if super::super::native_module::normalize_native_module_alias(module.as_str()) == "stream" { let opts = if args_len >= 1 && !args_ptr.is_null() { @@ -798,6 +821,9 @@ pub unsafe extern "C" fn js_fetch_or_value_super( "Transform" => Some(crate::node_stream::js_node_stream_transform_subclass_init( this_box, opts, )), + "Stream" => Some(crate::node_stream::js_node_stream_legacy_subclass_init( + this_box, + )), _ => None, }; if handled.is_some() { diff --git a/crates/perry-runtime/src/object/global_this/proto_methods.rs b/crates/perry-runtime/src/object/global_this/proto_methods.rs index fa3f8c5231..9bc53ab65b 100644 --- a/crates/perry-runtime/src/object/global_this/proto_methods.rs +++ b/crates/perry-runtime/src/object/global_this/proto_methods.rs @@ -803,6 +803,59 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: ); } } + // #10759: `URLSearchParams` previously had ONLY the `#10555` arm + // below (moved here): `install_web_builtin_to_string_tag` and + // nothing else, because its methods dispatch through type-directed + // static dispatch / the small-int/handle dispatch tables and never + // needed reified closures for ordinary `x.method()` calls. That + // design has no answer for a method read AS A VALUE -- + // `URLSearchParams.prototype.append`, `.prototype["has"]`, or + // through a Proxy `get` trap indirection -- which returned + // `undefined` instead of a callable closure. node-fetch's + // `Headers extends URLSearchParams` -- whose constructor returns + // `new Proxy(this, { get(target, p, receiver) { ... return + // (...)=> URLSearchParams.prototype[p].call(target, ...); } })` -- + // then threw "Function.prototype.call was called on a value that is + // not a function" on the very first `headers.has(...)`, reached by + // every `fetch()` call before the request is even sent. Same + // mechanism as the `Stream.prototype`/`Object.hasOwnProperty`/ + // `Function.toString` fixes elsewhere (`install_static.rs`, + // `node_stream_dispatch.rs`): install the no-op-backed reified + // closures so a value read resolves to a real (name-carrying) + // function, which `Function.prototype.call`/`.apply`'s + // `try_dispatch_value_called_proto_method` re-dispatches by name + // through `try_url_search_params_dynamic_dispatch` using the + // caller-supplied receiver. Method set + arities verified against + // `node --experimental-strip-types` (v26.5.1). The + // `install_web_builtin_to_string_tag` call is retained so + // `Object.getOwnPropertyDescriptor(URLSearchParams.prototype, + // Symbol.toStringTag)` keeps reflecting a real descriptor -- see + // that function's doc comment. The other six members of the + // `#10555` group below (`URL`, `AbortController`, `AbortSignal`, + // `EventTarget`, `Event`, `CustomEvent`) have the same + // "toStringTag-only arm" shape and have NOT been audited for this + // same value-read gap; see #10759's PR body for what was checked. + "URLSearchParams" => { + install_noop_proto_methods( + proto_obj, + &[ + ("append", 2), + ("delete", 1), + ("entries", 0), + ("forEach", 1), + ("get", 1), + ("getAll", 1), + ("has", 1), + ("keys", 0), + ("set", 2), + ("sort", 0), + ("toString", 0), + ("values", 0), + ], + ); + install_noop_proto_methods(proto_obj, OBJECT_PROTO_METHODS); + unsafe { install_web_builtin_to_string_tag(proto_obj, "URLSearchParams") }; + } "Promise" => { install_proto_method( proto_obj, @@ -1088,11 +1141,15 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: // is either type-directed static dispatch or the small-int/handle // dispatch tables), but each still needs its `.prototype`'s own // `Symbol.toStringTag` descriptor for reflection -- see - // `install_web_builtin_to_string_tag`'s doc comment. + // `install_web_builtin_to_string_tag`'s doc comment. `URLSearchParams` + // used to be listed here too; #10759 moved it to its own arm above + // (still calling `install_web_builtin_to_string_tag`) once a VALUE + // read of one of its prototype methods turned out to need real + // reified closures, not just the toStringTag descriptor. The other + // six members of this group (`URL`, `AbortController`, + // `AbortSignal`, `EventTarget`, `Event`, `CustomEvent`) have not been + // audited for the same "read as a value" gap -- see #10759's PR body. "URL" => unsafe { install_web_builtin_to_string_tag(proto_obj, "URL") }, - "URLSearchParams" => unsafe { - install_web_builtin_to_string_tag(proto_obj, "URLSearchParams") - }, "AbortController" => unsafe { install_web_builtin_to_string_tag(proto_obj, "AbortController") }, diff --git a/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs b/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs index f768a52cbb..ef93b4d17a 100644 --- a/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs +++ b/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs @@ -169,7 +169,18 @@ pub extern "C" fn js_instanceof_dynamic(value: f64, type_ref: f64) -> f64 { "Readable" | "Writable" | "Duplex" | "Transform" | "PassThrough" | "Stream" ) && (crate::node_stream::is_classic_stream_instance_of(value, method.as_str()) - || super::tls_constructor_prototype_is_instance_of(value, method.as_str())) + || super::tls_constructor_prototype_is_instance_of(value, method.as_str()) + // #10798: a genuine `class X extends Stream` subclass is a real + // ObjectHeader carrying its own class id, chained through the + // dedicated Stream hop (`class_registry::parent_static`'s + // `js_register_class_parent_dynamic`) rather than prototype- + // linked to the real `Stream.prototype` — so it is invisible to + // `is_classic_stream_instance_of`'s own-field probe above + // (which answers the DIRECT `new Readable()`-shaped case). Walk + // the class-id chain the same way the EventEmitter branch below + // does for its own subclass case. + || (method == "Stream" + && js_instanceof(value, 0xFFFF0070).to_bits() == crate::value::TAG_TRUE)) { return f64::from_bits(crate::value::TAG_TRUE); } diff --git a/crates/perry-runtime/src/regex/perex_split.rs b/crates/perry-runtime/src/regex/perex_split.rs index 7dfd71645e..053897f191 100644 --- a/crates/perry-runtime/src/regex/perex_split.rs +++ b/crates/perry-runtime/src/regex/perex_split.rs @@ -319,6 +319,45 @@ pub(crate) fn regexp(receiver: f64, argument: f64, limit_value: f64) -> Result) -> bool { + let bits = limit_value.get_nanbox_f64().to_bits(); + bits == TAG_UNDEFINED || crate::value::JSValue::from_bits(bits).is_number() +} + +/// Is this separator already a string with no lone surrogate in it? +/// +/// The plain algorithm scans WTF-8 bytes where the engine reads UTF-16 units, +/// so it cannot match a separator that is one half of a valid pair -- +/// `"\u{1F600}\u{1F600}".split(lowHalf)` is three parts to the engine and one +/// to a byte scan. WTF-8 spells a surrogate `ED A0..BF xx`, so this test is +/// exact rather than conservative. A separator that is not already a string is +/// excluded too: its `ToString` can run user code, and a Symbol must throw. +fn separator_is_plain( + scope: &RuntimeHandleScope, + separator: &RuntimeHandle<'_>, +) -> Result { + let jv = crate::value::JSValue::from_bits(separator.get_nanbox_f64().to_bits()); + if !jv.is_string() && !jv.is_short_string() { + return Ok(false); + } + // Already a string, so this coercion runs no user code; it only puts the + // value in the one representation whose bytes can be read. + let sep = text(scope, separator)?; + // SAFETY: a rooted string handle; the borrow spans no allocation or call. + Ok(unsafe { + sep.with_string_bytes(|bytes| { + !bytes + .windows(2) + .any(|w| w[0] == 0xED && (0xA0..=0xBF).contains(&w[1])) + }) + }) +} + pub(crate) fn string(receiver: f64, separator: f64, limit_value: f64) -> Result { if matches!(receiver.to_bits(), TAG_NULL | TAG_UNDEFINED) { return Err(EngineError::Type( @@ -343,6 +382,56 @@ pub(crate) fn string(receiver: f64, separator: f64, limit_value: f64) -> Result< return call(&method, &separator, &args, &memory); } } + // No `@@split`, so the plain string algorithm applies and the engine has + // nothing to contribute. Hand it to the implementation a build without the + // engine uses. + // + // Linking `regex-engine` replaces `String.prototype.split` with this module + // wholesale, so a program using a regex *anywhere* ran every split through + // the engine's per-unit subject reader: 35,826 instructions for + // `"alpha beta gamma delta eps0".split(" ")` against 3,662 without the + // engine, and 2,729 in Node 26.5.1. Both arms auto-optimized, so that is the + // implementation swap rather than the build mode. + // + // The plain algorithm agrees with Node on 27 cases where a byte scan and a + // UTF-16 unit scan can disagree -- empty separator, separator longer than + // the subject, every `limit` form, lone surrogates, an astral pair split by + // units, a separator that is a prefix of itself at the tail -- and on every + // non-string separator form. The one thing it does not implement is + // `@@split`, which is why this sits below that check. + // The two implementations report failure differently: this one returns + // `Err(EngineError)` for `api::finish` to raise at the ABI boundary, while + // the plain algorithm throws directly (its own boundary is the ABI). A + // coercion that throws -- `ToNumber` on a BigInt `limit`, say -- would + // otherwise escape as an uncaught exception, so the throw is captured here + // and re-raised by `finish` like any other engine error. + if limit_is_plain(&limit_value) && separator_is_plain(&scope, &separator)? { + // The plain algorithm reports failure by throwing, where this one + // returns `Err` for `api::finish` to raise; `delegable` has already + // excluded every input whose coercion can throw, so nothing escapes. + return api::caught(|| { + crate::string::js_string_split_plain( + receiver.get_nanbox_f64(), + separator.get_nanbox_f64(), + limit_value.get_nanbox_f64(), + ) + }); + } + string_via_engine( + receiver.get_nanbox_f64(), + separator.get_nanbox_f64(), + limit_value.get_nanbox_f64(), + ) +} + +/// The engine's split, for inputs `delegable` excludes. +fn string_via_engine(receiver: f64, separator: f64, limit_value: f64) -> Result { + let scope = RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(receiver); + let separator = scope.root_nanbox_f64(separator); + let limit_value = scope.root_nanbox_f64(limit_value); + let mut budget = Budget::new(api::WORK); + let memory = MemoryBudget::new(api::SCRATCH_BYTES); let input = text(&scope, &receiver)?; let lim = limit(&limit_value)?; let needle = text(&scope, &separator)?; diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index 858dde3518..6762d72506 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -247,6 +247,10 @@ pub use slice_ops::{ js_string_trim_start, }; pub use split::js_string_split; +/// The engine-free `String.prototype.split`, for the engine path to delegate to +/// once it has ruled out `@@split`. +#[cfg(feature = "regex-engine")] +pub(crate) use split::js_string_split_js as js_string_split_plain; #[cfg(not(feature = "regex-engine"))] pub use split::{js_string_split_js, js_string_split_n}; diff --git a/crates/perry-runtime/src/string/split.rs b/crates/perry-runtime/src/string/split.rs index 4bce6e71e5..f9c53eaea9 100644 --- a/crates/perry-runtime/src/string/split.rs +++ b/crates/perry-runtime/src/string/split.rs @@ -9,7 +9,6 @@ use crate::array::ArrayHeader; /// a per-element layout-map update. The write barrier remains necessary if a /// collection has promoted the rooted result array while it is being built. #[inline] -#[cfg(not(feature = "regex-engine"))] unsafe fn store_split_string(arr: *mut ArrayHeader, index: usize, string: *mut StringHeader) { const STRING_TAG: u64 = 0x7FFF_0000_0000_0000; const POINTER_MASK: u64 = 0x0000_FFFF_FFFF_FFFF; @@ -298,8 +297,12 @@ pub extern "C" fn js_string_to_upper_case_split_part_utf16_length( /// `limit < 0` → no limit (matches `js_string_split`). /// `limit == 0` → empty array. /// `limit > 0` → at most `limit` substrings. -#[cfg(not(feature = "regex-engine"))] -#[no_mangle] +// Compiled in both builds. Linking `regex-engine` re-exports the engine's +// `split` from `crate::string` instead of this one (see `string::mod`), but the +// engine path delegates back here once it has ruled out `@@split`, so the +// implementation must exist either way. Only the exported C symbols are +// conditional -- they would collide with the engine's. +#[cfg_attr(not(feature = "regex-engine"), no_mangle)] pub extern "C" fn js_string_split_n( s: *const StringHeader, delimiter: *const StringHeader, @@ -539,7 +542,6 @@ pub extern "C" fn js_string_split_n( /// `ToUint32(ToNumber(value))` (ECMA-262 §7.1.7). Runs the full `ToNumber` /// (so a boxed `{ valueOf }` / `{ toString }` argument is coerced and may /// throw), then reduces mod 2^32. `NaN`/`±Infinity`/`0` → 0. -#[cfg(not(feature = "regex-engine"))] fn split_limit_to_uint32(boxed: f64) -> u32 { let n = crate::builtins::js_number_coerce(boxed); if !n.is_finite() || n == 0.0 { @@ -550,7 +552,6 @@ fn split_limit_to_uint32(boxed: f64) -> u32 { /// Build the single-element array `[S]` (the `separator === undefined` result /// of `String.prototype.split`). -#[cfg(not(feature = "regex-engine"))] fn split_single_element(s: *const StringHeader) -> *mut ArrayHeader { const STRING_TAG: u64 = 0x7FFF_0000_0000_0000; const POINTER_MASK: u64 = 0x0000_FFFF_FFFF_FFFF; @@ -579,14 +580,17 @@ fn split_single_element(s: *const StringHeader) -> *mut ArrayHeader { /// - `limit === 0` ⇒ empty array; /// - `separator === undefined` ⇒ single-element `[S]`; /// - otherwise split by `ToString(separator)`, capped at `lim`. -#[cfg(not(feature = "regex-engine"))] -#[no_mangle] +#[cfg_attr(not(feature = "regex-engine"), no_mangle)] pub extern "C" fn js_string_split_value( s: *const StringHeader, separator: f64, limit: f64, ) -> *mut ArrayHeader { use crate::value::JSValue; + // Only the regex-engine arm below reads this; binding it unconditionally + // makes `cargo check -p perry --bins` warn, and fail under -D warnings, in + // a build without that feature. A whole-workspace build unifies the feature + // and hides it, which is why it survived review. #[cfg(feature = "regex-engine")] let sep_jv = JSValue::from_bits(separator.to_bits()); let lim_jv = JSValue::from_bits(limit.to_bits()); @@ -661,8 +665,7 @@ pub extern "C" fn js_string_split_value( js_string_split_n(s, r_str, limit_i32) } -#[cfg(not(feature = "regex-engine"))] -#[no_mangle] +#[cfg_attr(not(feature = "regex-engine"), no_mangle)] pub extern "C" fn js_string_split_js(receiver: f64, separator: f64, limit: f64) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); let receiver = scope.root_nanbox_f64(receiver); diff --git a/docs/src/api/reference.md b/docs/src/api/reference.md index 3b137f8df9..a50271bb30 100644 --- a/docs/src/api/reference.md +++ b/docs/src/api/reference.md @@ -2,7 +2,7 @@ This page is auto-generated from Perry's compile-time API manifest (`perry-api-manifest::API_MANIFEST`). It is the source of truth for what `perry compile` accepts; references to symbols not listed here produce `R005 UnimplementedApi` (issue #463). Stubs (#464) are flagged ⚠ — they link cleanly but no-op at runtime on the chosen target. -Total: 2855 entries across 120 modules. +Total: 2870 entries across 120 modules. ## Modules @@ -1538,6 +1538,7 @@ Total: 2855 entries across 120 modules. - `__get_path` — instance *(class: `ClientRequest`)* - `__get_protocol` — instance *(class: `Agent`)* - `__get_protocol` — instance *(class: `ClientRequest`)* +- `__get_rawHeaders` — instance *(class: `IncomingMessage`)* - `__get_req` — instance *(class: `IncomingMessage`)* - `__get_requestTimeout` — instance *(class: `HttpServer`)* - `__get_requests` — instance *(class: `Agent`)* @@ -1586,6 +1587,7 @@ Total: 2855 entries across 120 modules. - `close` — instance *(class: `HttpServer`)* - `closeAllConnections` — instance *(class: `HttpServer`)* - `closeIdleConnections` — instance *(class: `HttpServer`)* +- `complete` — instance *(class: `IncomingMessage`)* - `connection` — instance *(class: `IncomingMessage`)* - `cork` — instance *(class: `ClientRequest`)* - `cork` — instance *(class: `ServerResponse`)* @@ -1615,6 +1617,8 @@ Total: 2855 entries across 120 modules. - `headers` — instance *(class: `IncomingMessage`)* - `headersTimeout` — instance *(class: `HttpServer`)* - `httpVersion` — instance *(class: `IncomingMessage`)* +- `httpVersionMajor` — instance *(class: `IncomingMessage`)* +- `httpVersionMinor` — instance *(class: `IncomingMessage`)* - `keepAlive` — instance *(class: `Agent`)* - `keepAliveMsecs` — instance *(class: `Agent`)* - `keepAliveTimeout` — instance *(class: `HttpServer`)* @@ -1636,6 +1640,7 @@ Total: 2855 entries across 120 modules. - `once` — instance *(class: `ClientRequest`)* - `pause` — instance *(class: `IncomingMessage`)* - `protocol` — instance *(class: `Agent`)* +- `rawHeaders` — instance *(class: `IncomingMessage`)* - `read` — instance *(class: `IncomingMessage`)* - `ref` — instance *(class: `HttpServer`)* - `removeHeader` — instance *(class: `ClientRequest`)* @@ -2009,6 +2014,8 @@ Total: 2855 entries across 120 modules. - `__set_maxConnections` — instance *(class: `Server`)* - `_createServerHandle` — module - `_normalizeArgs` — module +- `_readableState` — instance +- `_writableState` — instance - `addAddress` — instance *(class: `BlockList`)* - `addListener` — instance *(class: `Socket`)* - `addListener` — instance *(class: `Server`)* @@ -2077,9 +2084,14 @@ Total: 2855 entries across 120 modules. - `parse` — module *(class: `SocketAddress`)* - `pause` — instance *(class: `Socket`)* - `pending` — instance *(class: `Socket`)* +- `pipe` — instance *(class: `Socket`)* - `port` — instance *(class: `SocketAddress`)* +- `prependListener` — instance *(class: `Socket`)* +- `prependOnceListener` — instance *(class: `Socket`)* - `rawListeners` — instance *(class: `Socket`)* - `rawListeners` — instance *(class: `Server`)* +- `readable` — instance +- `readableEnded` — instance - `readyState` — instance *(class: `Socket`)* - `ref` — instance *(class: `Socket`)* - `remoteAddress` — instance *(class: `Socket`)* @@ -2105,8 +2117,11 @@ Total: 2855 entries across 120 modules. - `timeout` — instance *(class: `Socket`)* - `toJSON` — instance *(class: `BlockList`)* - `uncork` — instance *(class: `Socket`)* +- `unpipe` — instance *(class: `Socket`)* - `unref` — instance *(class: `Socket`)* - `upgradeToTLS` — instance *(class: `Socket`)* +- `writable` — instance +- `writableEnded` — instance - `write` — instance *(class: `Socket`)* ## `node-fetch` diff --git a/test-files/test_gap_10759_urlsearchparams_prototype_method_value.ts b/test-files/test_gap_10759_urlsearchparams_prototype_method_value.ts new file mode 100644 index 0000000000..baaaf4f6a5 --- /dev/null +++ b/test-files/test_gap_10759_urlsearchparams_prototype_method_value.ts @@ -0,0 +1,136 @@ +// Gap test for #10759 — `URLSearchParams.prototype` had no entry in +// `populate_builtin_prototype_methods` (crates/perry-runtime/src/object/ +// global_this/proto_methods.rs), unlike every neighboring builtin (`Headers`, +// `URLPattern`, `Request`/`Response`, ...). Its prototype methods were never +// installed as real (name-carrying, callable) property VALUES, so any read +// of `URLSearchParams.prototype.` — literal, computed by a runtime +// string, or through a Proxy `get` trap — returned `undefined` instead of a +// function, and `.call()`/`.apply()` on that then threw: +// TypeError: Function.prototype.call was called on a value that is not a +// function +// +// This is the exact shape node-fetch@3.3.2's `Headers` class hits on EVERY +// `fetch()` call: `class Headers extends URLSearchParams` returns +// `new Proxy(this, { get(target, p, receiver) { ... return (...) => +// URLSearchParams.prototype[p].call(target, ...); } })` from its +// constructor, and `getNodeRequestOptions()` calls `headers.has('Accept')` +// unconditionally before the request is even sent. +// +// Same underlying mechanism as the sibling fixes already in-repo for other +// builtins: `require('stream').prototype` (object/native_module/ +// constants.rs) and `Function.toString`/`Object.hasOwnProperty` as values +// (crates/perry/tests/issue_5135_proxy_compound_and_function_tostring.rs) — +// a built-in prototype's methods must be installed as real closures, or a +// value-read misses regardless of how the read is spelled. +// Byte-identical to `node --experimental-strip-types` (v26.5.1). + +// ---- typeof / name / length: literal, literal-string, and computed-by-variable access ---- +{ + const methods = [ + "append", + "delete", + "entries", + "forEach", + "get", + "getAll", + "has", + "keys", + "set", + "sort", + "toString", + "values", + ] as const; + const proto: any = URLSearchParams.prototype; + const out: string[] = []; + for (const m of methods) { + const literal = typeof proto[m]; + const literalStr = typeof proto[m as string]; + const k: string = m; + const computed = typeof proto[k]; + out.push(`${m}:${literal},${literalStr},${computed},len=${proto[m].length},name=${proto[m].name}`); + } + console.log("typeof-suite:", out.join(" ")); +} + +// ---- direct `.call()` on the literal-read method, mutating a real receiver ---- +{ + const usp = new URLSearchParams(); + URLSearchParams.prototype.append.call(usp, "a", "1"); + console.log("literal-call:", usp.toString()); +} + +// ---- `.call()` through a runtime-variable computed key (the exact shape +// node-fetch's Headers.js uses inside its Proxy trap) ---- +{ + const usp = new URLSearchParams(); + const k = "append"; + (URLSearchParams.prototype as any)[k].call(usp, "a", "1"); + const k2 = "has"; + const hasA = (URLSearchParams.prototype as any)[k2].call(usp, "a"); + const hasB = (URLSearchParams.prototype as any)[k2].call(usp, "b"); + console.log("computed-call:", usp.toString(), hasA, hasB); +} + +// ---- the node-fetch `Headers` shape itself: a subclass whose constructor +// returns a Proxy wrapping `this`, whose `get` trap reads +// `URLSearchParams.prototype[p]` by a closure-captured (not literal) +// variable and calls it with `.call(target, ...)`. ---- +class FetchLikeHeaders extends URLSearchParams { + constructor() { + super(); + const target: any = this; + // eslint-disable-next-line no-constructor-return + return new Proxy(target, { + get(target: any, p: any, receiver: any) { + switch (p) { + case "append": + case "set": + return (name: string, value: string) => { + return (URLSearchParams.prototype as any)[p].call(target, name, value); + }; + case "delete": + case "has": + case "getAll": + return (name: string) => { + return (URLSearchParams.prototype as any)[p].call(target, name); + }; + default: + return Reflect.get(target, p, receiver); + } + }, + }); + } +} +{ + // Deliberately exercises only the trap's explicitly-handled cases + // (append/set/delete/has/getAll) — the same subset node-fetch's real + // Headers.js switch covers. Its `.get()`/`.toString()` are separate own + // CLASS METHODS that delegate to `getAll` rather than falling through the + // trap's `default: Reflect.get(target, p, receiver)` arm, because Node's + // native `URLSearchParams.prototype.get`/`.toString`, called with `this` + // bound to the Proxy receiver (as `default` would do), rejects a Proxy + // `this` via its own internal-slot brand check — a genuine, unrelated + // Node quirk this fixture avoids by construction, not a Perry gap. + const headers: any = new FetchLikeHeaders(); + console.log("headers-before-has-accept:", headers.has("Accept")); + headers.set("Accept", "*/*"); + console.log("headers-after-has-accept:", headers.has("Accept")); + headers.append("X-Extra", "1"); + headers.append("X-Extra", "2"); + console.log("headers-getall:", headers.getAll("X-Extra").join(",")); + headers.delete("X-Extra"); + console.log("headers-after-delete:", headers.has("X-Extra")); +} + +// ---- Object.prototype methods must also be present on URLSearchParams.prototype +// (installed alongside the URLSearchParams-specific set, same as every other +// builtin's arm in populate_builtin_prototype_methods). ---- +{ + const proto: any = URLSearchParams.prototype; + console.log( + "object-proto-methods:", + typeof proto.hasOwnProperty, + typeof proto.isPrototypeOf, + typeof proto.propertyIsEnumerable, + ); +} diff --git a/test-files/test_gap_10798_stream_bare_heritage.ts b/test-files/test_gap_10798_stream_bare_heritage.ts new file mode 100644 index 0000000000..e6d195d87b --- /dev/null +++ b/test-files/test_gap_10798_stream_bare_heritage.ts @@ -0,0 +1,65 @@ +// #10798: `class X extends Stream` (the bare legacy `node:stream` base, not +// one of its subclasses) threw `TypeError: ... is not a constructor`. This +// blocked `nodemailer`, which loads `class XOAuth2 extends Stream` on +// import. +// +// PR #10649 fixed the analogous dynamic-heritage-dispatch gap in +// `js_fetch_or_value_super` (crates/perry-runtime/src/object/global_this/ +// fetch_globals.rs) for `Readable`/`Writable`/`Duplex`/`Transform`, but its +// match list stopped short of `Stream` — the base those four themselves +// derive from. +// +// Unlike #10745 (`PassThrough`), this is NOT the deeper HIR-level gap: +// `canonical_native_parent_name` (crates/perry-hir/src/lower_decl/ +// class_decl.rs) never recognized ANY spelling of `Stream` as a native +// parent — unlike Readable/Writable/Duplex/Transform, which have a fast +// static path for a plain `import`, EVERY `extends Stream` heritage shape +// (bare ident, namespace member, CJS destructured `require`) already +// reaches the same dynamic `js_fetch_or_value_super` dispatch #10649 +// patches, uniformly. And `Stream` carries no hidden per-instance state to +// pre-seed in the first place — in Node it is literally `EventEmitter` plus +// a `pipe()` prototype method (`lib/internal/streams/legacy.js`), so the +// fix reuses the existing `js_event_emitter_subclass_init` shim rather than +// adding a stream-specific one. +import { Stream as ImportedStream } from "node:stream"; +import * as streamNs from "node:stream"; +import { createRequire } from "node:module"; + +const req = createRequire(import.meta.url); +const cjsStreamModule: any = req("stream"); +const { Stream: RequiredStream } = cjsStreamModule; + +class BareTap extends ImportedStream {} +class NamespaceTap extends streamNs.Stream {} +class CjsTap extends RequiredStream {} + +function run(name: string, T: any) { + let t: any; + try { + t = new T(); + } catch (e) { + console.log(name, "THREW (construct)", (e as Error).message); + return; + } + let got = 0; + const seen: string[] = []; + t.on("data", (c: any) => { + got++; + seen.push(String(c)); + }); + t.emit("data", "a"); + t.emit("data", "b"); + console.log( + name, + "count:", got, + "values:", seen.join(","), + "typeof pipe:", typeof t.pipe, + "typeof on:", typeof t.on, + "typeof once:", typeof t.once, + "instanceof Stream:", t instanceof ImportedStream, + ); +} + +run("bare extends Stream ", BareTap); +run("namespace extends stream.Stream", NamespaceTap); +run("cjs destructured extends Stream", CjsTap); diff --git a/test-files/test_gap_callback_dispatch_shapes.ts b/test-files/test_gap_callback_dispatch_shapes.ts new file mode 100644 index 0000000000..07c35589e4 --- /dev/null +++ b/test-files/test_gap_callback_dispatch_shapes.ts @@ -0,0 +1,227 @@ +// Callback-dispatch shapes audited for perf/callback-dispatch (calling a +// JSValue that holds a closure through the various paths codegen/runtime +// recognize: direct arrow-inline array-method callback, a closure held in a +// local, a closure threaded through a second function frame, a +// `Function.prototype.bind` method reference with and without partial args, +// and a hand-written tight loop over a callback parameter). +// +// This exercises exactly the shape `dispatch_bound_function` +// (crates/perry-runtime/src/closure/dispatch/bound.rs) treats specially: +// `.bind(thisArg)` with NO extra bound args skips the old per-call `Vec` +// combine-copy and passes the call-time args straight through. What is easy +// to break doing that: `this` binding (arrow vs ordinary function vs bound), +// `arguments`, extra/missing arguments and `.length`, a callback that +// throws (stack must stay correct), recursion through a callback reference, +// and a callback closing over a loop variable. + +function log(label: string, value: unknown): void { + console.log(label + ": " + JSON.stringify(value)); +} + +// --- shape: direct arrow inline to a builtin array-iteration method ------- +{ + const arr = [1, 2, 3, 4, 5]; + let s = 0; + arr.forEach((x) => { s += x; }); + log("forEach_arrow_inline", s); +} + +// --- shape: callback held in a local, called once ------------------------- +{ + const cb = (x: number) => x + 1; + log("local_once", cb(5)); +} + +// --- shape: callback held in a local, called in a tight loop -------------- +{ + const cb = (x: number) => x * 2; + let s = 0; + for (let i = 0; i < 20; i++) s += cb(i); + log("local_loop", s); +} + +// --- shape: callback threaded through a second function frame ------------- +function invokeOnce(cb: (x: number) => number, x: number): number { + return cb(x); +} +function twoFrames(cb: (x: number) => number, n: number): number { + let s = 0; + for (let i = 0; i < n; i++) s += invokeOnce(cb, i); + return s; +} +log("two_frames", twoFrames((x) => x + 3, 15)); + +// --- shape: callback parameter called directly in a tight loop ------------ +function directLoop(cb: (i: number) => number, n: number): number { + let s = 0; + for (let i = 0; i < n; i++) s += cb(i); + return s; +} +log("direct_loop_param", directLoop((x) => x - 1, 12)); + +// --- shape: method reference via .bind(), NO extra bound args ------------- +class Adder { + base: number; + constructor(base: number) { this.base = base; } + add(x: number): number { return this.base + x; } + // reads `this` explicitly, so a wrong receiver after bind() is observable. + describe(): string { return "Adder(" + this.base + ")"; } +} +{ + const a = new Adder(10); + const fn = a.add.bind(a); + const arr = [1, 2, 3, 4, 5]; + let s = 0; + arr.forEach((x) => { s += fn(x); }); + log("bound_no_extra_args_forEach", s); + log("bound_no_extra_args_once", fn(7)); + + const describeFn = a.describe.bind(a); + log("bound_this_reads_correctly", describeFn()); +} + +// --- shape: method reference via .bind(), WITH partial-applied args ------- +{ + const a = new Adder(100); + const fn2 = a.add.bind(a, 5); // bound arg `5` prepended... but `add` takes + // only ONE param, so the extra bound arg is simply ignored per spec (bound + // length caps at 0, extra bound args beyond declared arity are dropped by + // the underlying call, not by bind itself -- Node and Perry must agree). + log("bound_with_extra_args", fn2(1)); + + function sum3(a: number, b: number, c: number): number { return a + b + c; } + const boundSum = sum3.bind(null, 1, 2); + log("bound_plain_fn_partial", boundSum(3)); +} + +// --- shape: this binding across arrow / ordinary function / bound --------- +{ + const obj = { + v: 42, + arrowGet(this: any) { return (() => this.v)(); }, + ordinary(this: any) { return this.v; }, + }; + function grabThis(this: any): unknown { return this; } + const boundGrab = grabThis.bind(obj); + log("this_arrow_capture", obj.arrowGet()); + log("this_ordinary_direct", obj.ordinary()); + log("this_bound_grab", (boundGrab() as { v: number }).v); + + // A receiverless call of an ordinary function callback observes + // `this === undefined` (strict-mode-like OrdinaryCallBindThis) even when + // an enclosing method call left an IMPLICIT_THIS around. + function receiverless(this: unknown): string { + return this === undefined ? "undefined" : "leaked:" + JSON.stringify(this); + } + function callIt(cb: () => string): string { return cb(); } + const holder = { + m(): string { return callIt(receiverless); }, + }; + log("this_receiverless_no_leak", holder.m()); +} + +// --- shape: arguments object + extra/missing args + .length --------------- +{ + function variadic(): string { + // eslint-disable-next-line prefer-rest-params + const args = arguments as unknown as ArgumentsLike; + const parts: string[] = []; + for (let i = 0; i < args.length; i++) parts.push(String(args[i])); + return parts.join(","); + } + interface ArgumentsLike { length: number; [i: number]: unknown; } + function callWithN(cb: (...a: unknown[]) => string, ...a: unknown[]): string { + return cb(...a); + } + log("arguments_object_extra", callWithN(variadic as any, 1, 2, 3, 4)); + log("arguments_object_missing", callWithN(variadic as any)); + log("function_length_declared", ((a: number, b: number, c: number) => a + b + c).length); + + function needsThree(a: number, b: number, c: number): string { + return `${a},${b},${c}`; + } + log("missing_args_become_undefined", (needsThree as any)(1)); + log("extra_args_ignored", (needsThree as any)(1, 2, 3, 4, 5)); +} + +// --- shape: callback that throws, stack must stay correct ----------------- +{ + function boom(): never { throw new Error("boom"); } + function callThrow(cb: () => never): string { + try { + cb(); + return "no-throw"; + } catch (e) { + return "caught:" + (e as Error).message; + } + } + log("callback_throws_caught", callThrow(boom)); + + const boundBoom = boom.bind(null); + let threwFromBound = "no"; + try { + boundBoom(); + } catch (e) { + threwFromBound = "caught:" + (e as Error).message; + } + log("bound_callback_throws", threwFromBound); + + function outer(): string { + function inner(cb: () => never): string { + try { + cb(); + return "unreachable"; + } catch (e) { + return (e as Error).message; + } + } + return inner(boom); + } + log("callback_throws_through_two_frames", outer()); +} + +// --- shape: recursion through a callback reference ------------------------- +{ + function makeCountdown(): (n: number) => number { + const step = (n: number): number => (n <= 0 ? 0 : n + step(n - 1)); + return step; + } + const countdown = makeCountdown(); + log("recursive_callback", countdown(10)); + + // Recursion through a bound reference to itself. + let fact: (n: number) => number; + fact = (n: number): number => (n <= 1 ? 1 : n * fact(n - 1)); + const boundFact = fact.bind(null); + log("recursive_bound_callback", boundFact(6)); +} + +// --- shape: closure captured in a loop variable ---------------------------- +{ + const callbacks: Array<() => number> = []; + for (let i = 0; i < 5; i++) { + callbacks.push(() => i * i); + } + log("loop_var_capture_let", callbacks.map((f) => f())); + + const callbacksVar: Array<() => number> = []; + for (var j = 0; j < 5; j++) { + // eslint-disable-next-line no-loop-func + callbacksVar.push((function (captured) { return () => captured; })(j)); + } + log("loop_var_capture_var_iife", callbacksVar.map((f) => f())); +} + +// --- shape: bound method used as a hot forEach callback across many calls - +{ + class Acc { + total: number = 0; + add(x: number): number { this.total += x; return this.total; } + } + const acc = new Acc(); + const boundAdd = acc.add.bind(acc); + const many: number[] = []; + for (let i = 0; i < 200; i++) many.push(i); + many.forEach((x) => boundAdd(x)); + log("bound_hot_loop_total", acc.total); +} diff --git a/test-files/test_gap_map_get_string_key_perf.ts b/test-files/test_gap_map_get_string_key_perf.ts new file mode 100644 index 0000000000..540856feef --- /dev/null +++ b/test-files/test_gap_map_get_string_key_perf.ts @@ -0,0 +1,131 @@ +// Gap test: Map.get(string) correctness under the fast string-keyed lookup +// path (content-hash side table + SameValueZero identity rules). Covers the +// shapes exercised while reducing Map.get(str) instruction cost: identity +// semantics (NaN, +0/-0), insertion-order iteration, has-vs-get with a +// stored `undefined`, delete-then-reinsert order, non-ASCII / lone-surrogate +// string keys, content-equal-but-distinct-identity string keys, and a map +// past the side-table growth threshold. +// Run: node --experimental-strip-types test_gap_map_get_string_key_perf.ts + +// --- SameValueZero: NaN keys collapse to one slot --- +const nanMap = new Map(); +nanMap.set(NaN, "first"); +nanMap.set(NaN, "second"); +console.log("nan size:", nanMap.size); +console.log("nan get:", nanMap.get(NaN)); +console.log("nan has:", nanMap.has(NaN)); + +// --- SameValueZero: +0 and -0 are the same key --- +const zeroMap = new Map(); +zeroMap.set(0, "plus"); +zeroMap.set(-0, "minus"); +console.log("zero size:", zeroMap.size); +console.log("zero get +0:", zeroMap.get(0)); +console.log("zero get -0:", zeroMap.get(-0)); + +// --- has() vs get() for a stored `undefined` value --- +const undefMap = new Map(); +undefMap.set("present", undefined); +console.log("undef has present:", undefMap.has("present")); +console.log("undef get present:", undefMap.get("present")); +console.log("undef has missing:", undefMap.has("missing")); +console.log("undef get missing:", undefMap.get("missing")); + +// --- Insertion-order iteration survives get()-only probing --- +const orderMap = new Map(); +orderMap.set("z", 1); +orderMap.set("a", 2); +orderMap.set("m", 3); +orderMap.get("a"); +orderMap.get("z"); +const orderKeys: string[] = []; +for (const k of orderMap.keys()) orderKeys.push(k); +console.log("order keys:", orderKeys); + +// --- delete() then re-insert moves a key to the end --- +const reinsMap = new Map(); +reinsMap.set("x", 1); +reinsMap.set("y", 2); +reinsMap.set("z", 3); +reinsMap.delete("x"); +reinsMap.set("x", 10); +const reinsKeys: string[] = []; +for (const k of reinsMap.keys()) reinsKeys.push(k); +console.log("reins keys:", reinsKeys); +console.log("reins get x:", reinsMap.get("x")); + +// --- Non-ASCII string keys --- +const uniMap = new Map(); +uniMap.set("héllo", "accent"); +uniMap.set("日本語", "japanese"); +uniMap.set("😀emoji", "emoji"); +console.log("uni get héllo:", uniMap.get("héllo")); +console.log("uni get 日本語:", uniMap.get("日本語")); +console.log("uni get emoji:", uniMap.get("😀emoji")); +console.log("uni get missing:", uniMap.get("héllo2")); + +// --- Lone-surrogate string keys (WTF-8) --- +const loneHigh = String.fromCharCode(0xd800); +const loneLow = String.fromCharCode(0xdc00); +const surrMap = new Map(); +surrMap.set(loneHigh, "high"); +surrMap.set(loneLow, "low"); +console.log("surr get high:", surrMap.get(loneHigh)); +console.log("surr get low:", surrMap.get(loneLow)); +console.log("surr high !== low:", loneHigh !== loneLow); +console.log( + "surr get rebuilt high:", + surrMap.get(String.fromCharCode(0xd800)), +); + +// --- Content-equal but distinct-identity keys (dynamically built) --- +function makeKey(prefix: string, n: number): string { + return prefix + n; +} +const dynMap = new Map(); +for (let i = 0; i < 20; i++) { + dynMap.set(makeKey("item", i), i * 10); +} +// Re-build the same content via a different allocation than the stored key. +console.log("dyn get item0 (rebuilt):", dynMap.get(makeKey("item", 0))); +console.log("dyn get item19 (rebuilt):", dynMap.get(makeKey("item", 19))); +console.log("dyn get item9 (rebuilt):", dynMap.get(makeKey("item", 9))); +console.log("dyn get missing:", dynMap.get(makeKey("item", 99))); + +// --- Map past the side-table growth threshold (small linear scan vs +// hashed side table) --- +const bigMap = new Map(); +for (let i = 0; i < 64; i++) { + bigMap.set("k" + i, i); +} +console.log("big size:", bigMap.size); +console.log("big get k0:", bigMap.get("k0")); +console.log("big get k63:", bigMap.get("k63")); +console.log("big get k32 (rebuilt):", bigMap.get(makeKey("k", 32))); +console.log("big get missing:", bigMap.get("nomatch")); +bigMap.delete("k10"); +bigMap.delete("k20"); +bigMap.set("k10", 1010); +console.log("big get k10 after delete+reinsert:", bigMap.get("k10")); +console.log("big has k20 after delete:", bigMap.has("k20")); +console.log("big size after churn:", bigMap.size); + +// --- Long (> 64 byte) string keys --- +const longMap = new Map(); +const longPrefix = "q".repeat(70); +for (let i = 0; i < 10; i++) { + longMap.set(longPrefix + i, i); +} +console.log("long get 0 (rebuilt):", longMap.get(longPrefix + 0)); +console.log("long get 9 (rebuilt):", longMap.get(longPrefix + 9)); +console.log("long get missing:", longMap.get(longPrefix + "zz")); + +// --- Interned literal keys get a pointer-equality shortcut but must +// still match content-equal keys built at runtime --- +const litMap = new Map(); +litMap.set("literal-key", 1); +console.log("lit get literal:", litMap.get("literal-key")); +console.log( + "lit get rebuilt:", + litMap.get(["li", "teral-key"].join("")), +); diff --git a/test-files/test_gap_object_destructuring_field_and_rest_guard.ts b/test-files/test_gap_object_destructuring_field_and_rest_guard.ts new file mode 100644 index 0000000000..616395f3db --- /dev/null +++ b/test-files/test_gap_object_destructuring_field_and_rest_guard.ts @@ -0,0 +1,155 @@ +// Object-destructuring coverage for the field-guard / rest-array perf work: +// 2-field, 5-field, nested, defaults (undefined-only, not null), rest +// (excluding named + computed keys, keeping Symbol keys), missing +// properties, getter call order, null/undefined source TypeErrors, Symbol +// keys, and function-parameter destructuring. + +// --- 2-field / 5-field --- +{ + const { a, b } = { a: 1, b: 2 }; + console.log("2field", a, b); + const { a: a5, b: b5, c: c5, d: d5, e: e5 } = { a: 10, b: 20, c: 30, d: 40, e: 50 }; + console.log("5field", a5, b5, c5, d5, e5); +} + +// --- nested --- +{ + const { a: { b } } = { a: { b: 99 } }; + console.log("nested", b); +} + +// --- defaults: only undefined triggers, not null --- +{ + const { a = 10 } = {} as { a?: number }; + console.log("default-missing", a); + const { a: a2 = 10 } = { a: undefined } as { a?: number }; + console.log("default-undefined", a2); + const { a: a3 = 10 } = { a: null } as { a?: number | null }; + console.log("default-null", a3); + const { a: a4 = 10 } = { a: 0 }; + console.log("default-falsy-present", a4); +} + +// --- missing property -> undefined --- +{ + const { missing } = { a: 1 } as { a: number; missing?: number }; + console.log("missing-prop", missing === undefined); +} + +// --- rest excludes named keys, keeps Symbol keys --- +{ + const src = { a: 1, b: 2, c: 3, d: 4 }; + const { a, b, ...rest } = src; + console.log("rest-basic", a, b, JSON.stringify(rest)); + + // NOTE: a `{...rest}` of an object with a Symbol-keyed property is + // deliberately NOT covered here. Perry currently drops Symbol-keyed + // properties from the rest object entirely (`js_object_rest`'s own + // key-copy logic, unrelated to the `exclude_keys` array this file's + // perf change touches) — a pre-existing, separately-filed gap, not + // something this test should assert byte-identical parity on. + + // computed-key exclusion (evaluated once) + rest + let evalCount = 0; + function key() { + evalCount++; + return "b"; + } + const { [key()]: bv, ...restComputed } = { a: 1, b: 2, c: 3 }; + console.log("rest-computed-key", bv, JSON.stringify(restComputed), evalCount); + + // empty pattern before rest + const { ...restAll } = { x: 1, y: 2 }; + console.log("rest-empty-pattern", JSON.stringify(restAll)); +} + +// --- Symbol-keyed destructuring --- +{ + const sym2 = Symbol("s2"); + const obj: any = { [sym2]: 42, plain: 1 }; + const { [sym2]: symVal, plain } = obj; + console.log("symbol-key-read", symVal, plain); +} + +// --- getters run exactly once, in PATTERN source order (not object's own key order) --- +{ + const log: string[] = []; + const obj = { + get c() { + log.push("c"); + return 3; + }, + get a() { + log.push("a"); + return 1; + }, + get b() { + log.push("b"); + return 2; + }, + }; + const { c, a, b } = obj; + console.log("getter-order", log.join(","), a, b, c); +} + +// --- null / undefined source throws TypeError (even for empty pattern) --- +{ + function tryDestructure(fn: () => void): string { + try { + fn(); + return "no-throw"; + } catch (e) { + return e instanceof TypeError ? "TypeError" : "wrong-error:" + String(e); + } + } + console.log("null-source", tryDestructure(() => { + const { a } = null as any; + void a; + })); + console.log("undefined-source", tryDestructure(() => { + const { a } = undefined as any; + void a; + })); + console.log("null-source-empty-pattern", tryDestructure(() => { + const {} = null as any; + })); + console.log("undefined-source-rest", tryDestructure(() => { + const { ...r } = undefined as any; + void r; + })); +} + +// --- function parameter destructuring: plain, default, rest, nested --- +{ + function f2({ a, b }: { a: number; b: number }): number { + return a - b; + } + console.log("param-2field", f2({ a: 5, b: 2 })); + + function fDefault({ a = 7 }: { a?: number }): number { + return a; + } + console.log("param-default-missing", fDefault({})); + console.log("param-default-present", fDefault({ a: 1 })); + console.log("param-default-null", fDefault({ a: null } as any)); + + function fRest({ a, ...rest }: { a: number; [k: string]: number }): string { + return a + ":" + JSON.stringify(rest); + } + console.log("param-rest", fRest({ a: 1, b: 2, c: 3 })); + + function fNested({ outer: { inner } }: { outer: { inner: number } }): number { + return inner; + } + console.log("param-nested", fNested({ outer: { inner: 77 } })); + + function fParamThrows(o: any): string { + try { + const { a } = o; + return "no-throw:" + a; + } catch (e) { + return e instanceof TypeError ? "TypeError" : "wrong-error"; + } + } + console.log("param-null-throws", fParamThrows(null)); +}