test(test262): #5346 — run Node oracle as a global script (vm.runInThisContext) - #5511
Merged
Merged
Conversation
…isContext) The annexB subset radar invoked the Node oracle as `node case.js`, which evaluates the assembled case as a CommonJS module: top-level `var`/`function` declarations land on the module wrapper, not the global object. A real Test262 host (and Perry) run each case as a global script, so the two diverge wherever harness intrinsics must be reachable from global scope. That misattributed 72 `annexB/language/eval-code/indirect` cases as "Perry ran clean; Node rejected (missed negative)": the indirect `(0,eval)(...)` evaluates in global scope and so couldn't see the module-scoped `assert`, making Node throw `ReferenceError: assert is not defined` on cases Perry runs correctly. These were never Perry leniency bugs — they were an oracle artifact. Route the oracle through a committed `host-run.cjs` shim that evaluates the assembled script via `vm.runInThisContext`, restoring global-script semantics (top-level decls become globals, top-level `this` is `globalThis`, syntax errors still throw at compile so negative parse cases keep exiting non-zero). The `.cjs` extension is required because the repo's package.json sets `"type": "module"`. Effect on annexB (--all-features, node v26): the 72 eval-indirect false failures flip to pass, and the corrected oracle simultaneously unmasks ~74 genuine B.3.3 bugs the CommonJS wrapper had been hiding (Perry does not reflect top-level `var`/`function`/B.3.3 block-function bindings as own properties of the global object — a global-environment-record gap, tracked separately). The headline parity is ~flat (it was slightly inflated by the masked bugs) but now reflects reality. Default class-dir slice unaffected (95.7%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA new ChangesTest262 Node Oracle Global-Script Shim
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
proggeramlug
added a commit
that referenced
this pull request
Jun 23, 2026
…78-case global prop-desc cluster) (#5608) * fix(hir): #5579 — global-script mode top-level `this` is globalThis (78-case global prop-desc cluster) The 78 newly-failing `built-ins/*/prop-desc.js` (and friends) test262 cases report `parseFloat should be a function` / `decodeURI should be an own property`. They run the standard `verifyProperty(this, NAME, ...)` / `verifyPrimordialCallableProperty(this, NAME, 1)` shape against the global object via top-level `this`. Root cause: not a runtime regression. #4868 lowered module top-level `this` to a CJS `module.exports` stand-in (a fresh `{}`) explicitly "to match the node oracle" — which at the time ran each assembled case as a CommonJS module (`this === module.exports === {}`). In-window commit #5346/#5511 (`84eea0c20`, host-run.cjs) switched that oracle to a conforming *global script* via `vm.runInThisContext`, where top-level `this === globalThis`. Perry kept emitting the `{}` stand-in, so `this["parseFloat"]` was `undefined` and `Object.getOwnPropertyDescriptor(this, "decodeURI")` was `undefined` — the cluster regressed against the new oracle. This cannot be fixed by flipping Perry's default: the gap suite compares byte-for-byte against `node --experimental-strip-types <file>`, which runs the file as a CommonJS module where top-level `this` is NOT `globalThis` (`this.parseFloat === undefined`). A default change would diverge there. Fix: add an opt-in global-script mode (`PERRY_GLOBAL_SCRIPT_THIS`). When set, module top-level `this` lowers to `globalThis` instead of `Expr::ModuleTopThis`, and a direct `eval("this")` folds to the same (keeping `eval("this") === this` true under a conforming host). The default stays CJS-`{}`. The test262 harness sets the flag so Perry matches the script-mode node oracle. Validation: - New `issue_5579_global_script_this_prop_desc` (3 tests: script-mode this===globalThis + verifyProperty descriptor shape; default-mode this stays CJS `{}`; script-mode `eval("this") === this`) — pass. - Focused test262 (parseFloat/decodeURI/parseInt/isNaN/isFinite/ encodeURIComponent + Object/getOwnPropertyDescriptor/global/eval-code/direct/ global-code): every `prop-desc.js` now passes; A/B flag-off-vs-on over the `this`-sensitive dirs shows 12+ fixed and 0 newly-failing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: cargo fmt (#5579 global-script this) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ralph <ralph@skelpo.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
proggeramlug
added a commit
that referenced
this pull request
Jun 24, 2026
…onto globalThis (43 asyncTest cases) (#5607) * fix(codegen): #5579 — reflect top-level Script function declarations onto globalThis A non-ESM entry program is a Script: per GlobalDeclarationInstantiation its bare top-level `function` declarations become own properties of the global object. Perry bound them only in module scope, so `Object.prototype.hasOwnProperty.call(globalThis, name)` returned false. The Test262 async harness (asyncHelpers.js) gates on exactly that: `asyncTest` throws "asyncTest called without async flag" unless globalThis owns `$DONE` (installed by doneprintHandle.js as a top-level `function`). 43 `language/.../async-function` (+ friends) cases regressed to that error once #5511 switched the Node oracle to run cases as a Script (vm.runInThisContext), which — correctly, matching a conforming Test262 host — exposes top-level declarations on the global object. The oracle change was right; it merely unmasked this pre-existing global-environment-record gap in Perry (the #5511 commit message flagged it as "tracked separately"). Fix: - Lowering records bare top-level function declarations (name + FuncId) in a new `Module::script_global_functions`, deduped last-declaration-wins. This excludes nested closures and object-literal methods (which also live in `hir.functions`) so they never pollute the global object. - The entry-module codegen branch, for a non-ESM program only, emits `globalThis[name] = <closure>` for each recorded name before user init runs (honoring hoisting). The closure value is built exactly as `Expr::FuncRef` (`js_closure_alloc_singleton(@__perry_wrap_<sym>)`), so it is callable and `typeof globalThis[name] === "function"`. ESM modules (import/export syntax or top-level await) do not reflect. - `script_global_functions` participates in the module stable hash so the compile cache can't serve an object that omits the reflection. `var` reflection (the other half of GlobalDeclarationInstantiation) is a larger follow-up and not needed for this cluster. Regression test: crates/perry/tests/issue_5579_script_global_function_reflection.rs asserts the compiled program's own stdout (top-level functions are globalThis own properties, callable through globalThis, nested fns/object methods are not, duplicate decls last-wins). It does NOT diff against `node`, because `node <file>` runs as a module (no reflection); the authority here is the Test262 Script oracle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(codegen): #5579 — gate globalThis fn reflection on globalThis use; fix CI Refines the previous commit after CI feedback: 1. Narrow the reflection gate. Emitting `globalThis[name] = <fn>` for every non-ESM program's top-level functions added dynamic-property-helper calls to module init even for pure programs that never read the global object — which (a) is unobservable dead work and (b) tripped the native-region-proof compiler-output-regression gate (`h1_native_rep_equivalence`: "optimized IR has no dynamic property helper calls"). Now also require the source to reference `globalThis` (new `Module::references_global_this`, set at lowering from the installed module source). The reflection is unobservable without a `globalThis` reference, so this is sound; every Test262 async case still qualifies because asyncHelpers.js spells `globalThis` in its `$DONE` gate. 2. Propagate the two new `Module` fields (`script_global_functions`, `references_global_this`) to the hand-written `Module { .. }` literals in the perry-codegen / perry-codegen-arkts test helpers (fixes the cargo-test / harmonyos-smoke E0063 "missing field" build break). 3. Add an ESM-entry negative test (CodeRabbit nitpick): a module with a named export must NOT reflect top-level functions onto globalThis. 4. Drive-by (UNRELATED to #5579): fix the stale `init_body_function_name` matcher in shadow_slot_hygiene.rs. `__init_body` is emitted with `external` linkage (worker_threads call it directly, bypassing the `__init` once-guard), but the helper still grepped for `define internal void` and panicked. This test fails identically on pristine origin/main (verified) — it is not caused by this PR, but it blocks cargo-test, so the matcher is made linkage-agnostic. Verified: integration test (3/3) passes; test262 language/statements/async-function has 0 `asyncTest called without async flag` failures; the h1 fixture compiles with 0 dynamic-property-helper call instructions; cargo fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ralph <ralph@skelpo.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
proggeramlug
added a commit
that referenced
this pull request
Jun 24, 2026
… body to its global completion value (#5627) * fix(hir): #5579 — fold module-top indirect eval of a declaration-free body to its global completion value Residual of the #5579 regression batch: the `language/eval-code/indirect` completion-value cluster. Indirect eval `(0, eval)('<const>')` runs as global code, but Perry's general indirect-eval path deferred every valid body to the runtime global-`eval` thunk, which only models the `this`/`globalThis` idiom and returns `undefined` for everything else. So `(0,eval)('x = 1')` yielded undefined instead of 1 and never mutated the global `x` — diverging from the script-mode Node oracle (`vm.runInThisContext`, #5346/#5511). Perry already models a constant *direct* eval body with a scope-capturing completion IIFE (#1679). For indirect eval that IIFE is only sound where the captured enclosing scope already IS the global scope: module top level (`scope_depth == 0`, no enclosing class / `with` / ESM) under `PERRY_GLOBAL_SCRIPT_THIS`, where module-top `var`s and `this` are the global bindings (#5608/#5609). There `try_indirect_eval_general` now folds the body via the shared `build_eval_completion_iife` (sloppy unless the body opens with its own Use Strict Directive — indirect eval never inherits the caller's strictness). The fold is restricted to *declaration-free* bodies. A scope-capturing IIFE places any `var`/`function`/`class`/`let`/`const` the body declares inside the wrapper, but real global eval routes `var`/`function` to the global var environment and `let`/`const`/`class` to the eval's own fresh lexical environment — and Perry additionally registers class names at module scope, so a folded `class C {}` would leak `C` to the top level (breaking `indirect/lex-env-distinct-cls`). A declaration-free body has no such binding to misplace; it only reads/assigns the globals it names. A new recursive `eval_body_declares_bindings` scan (through blocks, loops, `try`, `switch`, `with`, labeled and `if` statements where `var`/`function` hoist) gates this, so any declaration defers to the runtime thunk as before. Test262 `language/eval-code/indirect`: +2 (`cptn-nrml-expr-prim`, `cptn-nrml-expr-obj`) with zero regressions across `language/eval-code` and `annexB/language/eval-code` (declaration-bearing bodies, incl. the annexB function-hoisting set, are unchanged). Outside global-script mode (the default, and every standalone build) behavior is byte-for-byte unchanged. Regression test: crates/perry/tests/issue_5579_indirect_eval_global_completion.rs — completion values + global mutation, object identity, always-sloppy `with`/ undeclared-assignment side effects, and that a nested-scope indirect eval does NOT capture the enclosing function's locals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test+style: address CodeRabbit nitpicks (#5627) - stmt_declares_binding: replace the `_ => false` catch-all with explicit no-declaration arms (Expr/Empty/Debugger/Return/Break/Continue/Throw) so a future ast::Stmt variant that can nest a declaration is a compile error here rather than a silent miss in the eval-fold safety gate. - indirect_eval_nested_does_not_capture_locals: add a positive `typeof: undefined` + `DONE` assertion (not just the negative `!number` check) and align the doc comment with the clean-exit expectation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ralph <ralph@skelpo.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
proggeramlug
added a commit
that referenced
this pull request
Sep 2, 2026
….length` throws for a non-writable descriptor (#9422) (#9458) * fix(runtime): a rejected strict `arr.length = n` throws for a non-writable descriptor too (#9422) "use strict"; const a = [1, 2]; Object.defineProperty(a, "length", { writable: false }); a.length = 0; // node: TypeError Perry: silent (length stayed 2) a.length = 2; // node: TypeError Perry: silent (same-value writes reject too) ES2024 6.2.5.7 (PutValue) calls Set(O, "length", n, Throw) with Throw = IsStrictReference, and OrdinarySet consults `length`'s own descriptor and reports false BEFORE it looks at `n` — so a non-writable `length` rejects even a write of the value it already holds. `js_array_set_length_strict` recognised only ONE of the two ways `length` becomes non-writable. It tested OBJ_FLAG_FROZEN, which Object.freeze sets; an explicit Object.defineProperty(arr, "length", { writable: false }) records the attribute in the descriptor side table WITHOUT freezing the array, and that shape fell straight through to the sloppy body — whose own non-writable arm is a silent `return`, annotated "strict-mode throw is handled by the caller's PutValue". This entry IS that caller. The throw set and the no-op set had drifted, and nothing tied them together. The predicate is not new. `array_length_is_non_writable` is what push/pop/shift/unshift have guarded with since test262 Array.prototype.{push,pop,shift,unshift}/set-length-*-non-writable — those mutators perform the same Set(O,"length",...,true). This was the one such site not using it. It is checked BEFORE the zero-truncate fast path, so a write the spec rejects cannot reach a shortcut that stores. Scope, stated because the neighbouring cases look identical and are NOT fixed: Object.seal and Object.preventExtensions leave `length` writable, so they are not this rejection and do not throw here. Perry's handling of those two is wrong in a different, non-strictness way — it refuses the length change outright in BOTH modes where node performs it (preventExtensions then `a.length = 5` gives 5 in node, 2 in Perry) — and a sealed shrink should reject through ArraySetLength's deletion walk, which Perry does not model. Making the strict entry mirror the sloppy body wholesale would have turned both of those wrong answers into wrong TypeErrors. WHAT #9422 AS FILED CLAIMED, AND WHAT IS ACTUALLY TRUE. The issue reported that `"use strict"; const o={x:1}; Object.freeze(o); o.x=9;` is silent, and located the cause as codegen emitting `js_put_value_set(..., strict = 0)` at EVERY property-set site. Neither holds on main. That program throws correctly, and so does every other ordinary-object shape: frozen own/new, sealed new, non-writable own and INHERITED, getter-only own and INHERITED, preventExtensions new, computed key, class field, compound assignment and update. The emitted IR shows why: the strict arm lowers to `js_class_field_set_fallback` (which throws), while the two `strict = 0` literals in expr/property_set.rs sit inside try_lower_sloppy_class_field_store / ..._boxed_store, which proxy_reflect.rs reaches only under `if !*strict` — where 0 is the correct constant. The array-`length` lane above is the one place a rejected strict write really was silent. test-files/test_gap_9422_strict_object_store_strictness.cts is a `.cts`, so it is a CommonJS script in BOTH runtimes, with a sloppy arm and a "use strict" arm. BOTH ARMS ARE ASSERTED across all seven rejection shapes plus the over-throw controls (sealed / preventExtensions writes to an EXISTING property, and an inherited setter, which succeed in both modes). A compiler built from unfixed origin/main reports `strict non-writable array length: silent 2` where node reports `TypeError 2`; with this change the file is byte-identical to node 26.5.1. Unit test `set_length_rejection_throws_only_in_strict_mode` sits beside #9394's `element_store_rejection_throws_only_in_strict_mode` and asserts both arms, the same-value write, the frozen shape that already worked, and a writable-`length` control. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp * fix(codegen): ES module top-level code is lowered as strict code (#9423) // any .ts under "type": "module" — an ES module, strict with no directive const a = [1, 2]; Object.freeze(a); for (a[0] of [7]) {} // node: TypeError Perry: silent ES2024 11.2.2: a Module IS strict mode code, with no "use strict" prologue needed. Lowering already knows this — LoweringContext::module_strict is computed from the file's module goal and feeds current_strict, so every HIR node that carries its own `strict` flag (PutValueSet, PropertyUpdate, IndexUpdate) was already right. That is why a plain `frozenObject.x = 9` at module top level threw correctly and this stayed hidden. Codegen could not see it. Module init is lowered as a synthetic function, and FnCtx::is_strict_fn was hardcoded false for it at both codegen/entry.rs sites (entry module and per-module __init), and again for every outlined entry chunk in codegen/entry_outline.rs — whose comment said so and asked the next person to match it. So every lane keyed on the CONTEXT's strictness rather than on a node-carried flag ran module top-level code sloppy: - Expr::IndexSet (expr/dispatch.rs passes ctx.is_strict_fn straight into index_set::lower) — the node a `for` head or a destructuring target with a computed member lowers to. A rejected `for (frozenArray[0] of ...)` was a silent no-op. This is the shape #9423 predicted and the one the fixture catches. - Expr::This (expr/this_super_call.rs) and `delete` (expr/instance_misc1.rs, expr/proxy_reflect.rs via js_delete_result), which also read the context flag. The module's strictness now rides on the HIR module as Module::init_is_strict, set next to ctx.module_strict at the top of lowering so a later early return cannot ship a module claiming to be sloppy, and read by both entry.rs sites and threaded into entry_outline.rs's chunk functions — a chunk is module top-level code that merely moved into a function, so relaxing its mode would reopen the same hole. It also joins the module's stable hash. That is load-bearing, not tidiness: the flag changes emitted code, so without it a cached object from a sloppy compile would be reused for a strict module. The exhaustive destructure in stable_hash/module.rs is what forced the decision to be made rather than defaulted. NOT FIXED, and deliberately not asserted by the fixture: module top-level `this`. Node gives `undefined` for an ES module; Perry gives a CommonJS `module.exports` stand-in. That lowers to its own HIR node, Expr::ModuleTopThis, chosen in lower_expr's ast::Expr::This arm and switched only by PERRY_GLOBAL_SCRIPT_THIS (#5579/#5346/#5511). It never consults strictness, so no is_strict_fn change can move it — it is a separate module-goal decision (Perry compiles a standalone program as CJS on purpose) and changing it does not belong in a strictness fix. test-files/test_gap_9423_module_init_strictness.ts is a plain `.ts`, which under this repo's "type": "module" package is strict-mode ESM in BOTH runtimes, so every write in it sits at module top level where the spec says strict. It covers an undeclared-name assignment and a rejected write through each lowering that reaches a store at module top level — static name, computed key, `for`-of head (named and computed), destructuring target (named and computed), array element and arr.length — plus the over-throw controls that must still succeed (sealed / preventExtensions writes to an existing property, and the same `for`-of head and destructure on an unfrozen receiver). A compiler built from unfixed origin/main reports `module frozen array for-of head: silent 1` where node reports `TypeError 1`; with this change the file is byte-identical to node 26.5.1. The sloppy control for the same shapes is #9422's `.cts` fixture. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp * docs(hir): init_is_strict doc stops claiming module-top this was fixed (review) The field never governed Expr::ModuleTopThis -- that is a module-goal decision that never consults strictness, and it still diverges from node. The doc listed it among the fixed lanes, which overclaimed. --------- Co-authored-by: Ralph Küpper <ralph3@skelpo.com> Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up on the
annexB/languagetail (#5346). The remeasure after #5394/#5356 shows the72× missed-negativecluster the issue flagged as "Annex B early-error semantics Perry is too lenient on" is not a Perry bug at all — it's a flaw in the subset radar's Node oracle.The runner invoked the oracle as
node case.js, which evaluates the assembled case as a CommonJS module. That scopes the harness's top-levelassert/Test262Error/etc. to the module wrapper instead of the global object. A conforming Test262 host — and Perry — run each case as a global script, so the two diverge wherever a harness intrinsic must be reachable from global scope.Concretely, all 72 mis-bucketed cases are
annexB/language/eval-code/indirect/*, where an indirect(0,eval)(...)evaluates in global scope and so can't see a module-scopedassert:Node threw on cases Perry runs correctly, and the runner scored them
Perry ran clean; Node rejected (missed negative).Fix
Route the oracle through a committed
test-compat/test262/host-run.cjsshim that runs the assembled script viavm.runInThisContext, restoring global-script semantics:assert),thisisglobalThis,The
.cjsextension is deliberate — the repo'spackage.jsonsets"type": "module", so a.jsshim would load as ESM andrequirewould be undefined.Effect (annexB,
--all-features, node v26)eval-code/indirectfalse failures flip to pass.f should be an own property,binding is not reinitialized,SameValue("string","function"), …). Root cause: Perry does not reflect top-levelvar/function(incl. B.3.3 block functions) as own properties of the global object —Object.getOwnPropertyDescriptor(globalThis, "a")isundefinedfor a top-levelvar a, where Node reports an own enumerable, writable, non-configurable property. That's a global-environment-record gap, not a contained fix; it should be its own ticket.Re-diagnosis of the remaining
annexB/languagetailWith the oracle corrected, the tail decomposes into three well-defined items:
eval/B.3.3 bindings not reflected as own global-object properties. One architectural feature; recommend a dedicated ticket.eval()— genuinely runtime-built strings; a real AOT limit to document/scope (not fixable in-process).\c/ named-group gaps, the known categorical regex limits.This PR is test tooling only — no compiler/runtime change, no version bump, no changelog (per maintainer convention for non-shipping infra changes).
Validation
scripts/test262_subset.py --dir annexB --all-features --jobs 8before/after — 72 eval-indirect cases move fail→pass; reasons re-bucketed as above.host-run.cjson a previously-passing case (Date.prototype.getYear/length) and a previously-mis-bucketed one (global-block-decl-eval-global-skip-early-err): both exit 0.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Tests
Documentation