diff --git a/README.md b/README.md index 3c4b5f74b..014d7913d 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ Core built-ins include `Math`, `JSON`, `Object`, `Function`, `Array`, `Boolean`, Non-standard data-format APIs and SemVer are import-only Goccia runtime modules, not auto-installed globals: `goccia:csv`, `goccia:json5`, `goccia:jsonl`, `goccia:toml`, `goccia:tsv`, `goccia:yaml`, and `goccia:semver`. They expose named exports only; use `import * as CSV from "goccia:csv"` when you want the namespace-object shape. There is no default export. -`node:async_hooks` is an import-only module too, at Node's own address and with Node's exports: `AsyncLocalStorage` and `AsyncResource`, named and on the default export. The engine propagates the async context, so a store bound with `run` survives `await` and every promise-reaction continuation. See the [Async Context reference](docs/built-ins-async-context.md) and [ADR 0112](docs/adr/0112-native-async-local-storage.md). +`node:async_hooks` is an import-only module too, at Node's own address. It exports `AsyncLocalStorage` and `AsyncResource`, named and on the default export; the `async_hooks` observer API (`createHook`, `executionAsyncId`, and the rest) is out of scope. The engine propagates the async context, so a store bound with `run` survives `await` and every promise-reaction continuation. See the [Async Context reference](docs/built-ins-async-context.md) and [ADR 0112](docs/adr/0112-native-async-local-storage.md). Native FFI is an explicit unsafe runtime opt-in (`--unsafe-ffi` or the matching configuration key). It provides native-layout structures, unions, fixed-length arrays, callbacks, and guarded library lifetimes through GocciaScript's custom bidirectional ABI machinery. See the [FFI reference](docs/built-ins-ffi.md) and [ADR 0095](docs/adr/0095-custom-bidirectional-ffi-abi-engine.md). diff --git a/docs/built-ins-async-context.md b/docs/built-ins-async-context.md index 8ee869afd..fec4326ba 100644 --- a/docs/built-ins-async-context.md +++ b/docs/built-ins-async-context.md @@ -45,8 +45,9 @@ const handle = async (request) => | `AsyncLocalStorage.bind(fn)` | Returns `fn` pinned to the context current at the `bind` call. Throws `TypeError` at the `bind` call if `fn` is not callable. | | `AsyncLocalStorage.snapshot()` | Returns `(fn, ...args) => fn(...args)`, run under the context current at the `snapshot` call. It has no callback to validate, so a non-callable is a `TypeError` at the runner's call instead. | -Every function these return is named `bound` and reports its target's `length`, -as Node's do. +Every function these return is named `bound`, as Node's are. A `bind` wrapper +reports its target's `length`; a `snapshot` runner has no target and reports +`1`, the arity of the `(fn, ...args)` runner itself. ## AsyncResource @@ -100,7 +101,8 @@ snapshot mechanism behind the propagation. ## Availability `node:async_hooks` is installed by the loader runtime profile, so it resolves in -`GocciaScriptLoader`, `GocciaTestRunner`, `GocciaREPL`, and -`GocciaBenchmarkRunner` without a flag. It grants no capability — no I/O, no +`GocciaScriptLoader`, `GocciaTestRunner`, `GocciaREPL`, +`GocciaBenchmarkRunner`, and `GocciaSandboxRunner` (which applies that profile +before installing its own sandbox extension) without a flag. It grants no capability — no I/O, no clock, no ambient authority — so nothing about it is gated. `GocciaScriptLoaderBare` attaches no runtime and therefore does not resolve it. diff --git a/docs/differential-testing.md b/docs/differential-testing.md index df69bc8b9..5016c5bb9 100644 --- a/docs/differential-testing.md +++ b/docs/differential-testing.md @@ -228,7 +228,8 @@ A differential suite that is handed to an external runtime uses only the suite named `*.goccia.test.js` is the exception, because it deliberately reaches for goccia-only globals — or asserts behavior that deliberately diverges from both external runtimes — and it is classified `skip` for both. A suite that needs the mocking API instead imports `vi` from -`vitest`, which every runtime that can run it resolves for itself. A `.test.ts` +`vitest`, which each runtime that can run it resolves its own way — Vitest to +itself, goccia to its bundled compatibility shim. A `.test.ts` suite works under bun because bun transpiles TypeScript natively while goccia parses annotations as types-as-comments. diff --git a/docs/module-resolution.md b/docs/module-resolution.md index 82679ab13..260168e57 100644 --- a/docs/module-resolution.md +++ b/docs/module-resolution.md @@ -194,7 +194,8 @@ A resolved file inside a package is classified before it is loaded: - A package whose manifest declares `"type": "module"` ships ES modules. - Otherwise the source text decides: a file carrying CommonJS markers (a `require(...)` call, `module.exports`, `exports.x`) and no ES module - markers (a statement-position `import` or `export`) is CommonJS. + markers (an `import` or `export` keyword followed by whitespace, `{`, `*`, + or a quote) is CommonJS. The source scan is a heuristic, and it is asymmetric on purpose. A file with both shapes — an interop shim calling `require` from an ES module — is read as @@ -203,6 +204,16 @@ neither is inert and loads either way. Reading the file text rather than trusting `"type"` is what makes the `module`-field deviation above work at all, since those ES module builds routinely sit in packages that declare no type. +The scan matches raw text and does not tokenize, so it does not skip comments +or string literals: a CommonJS bundle whose banner comment mentions `import` or +`export` carries an ES module marker as far as the classifier is concerned and +is not refused here. Such a file is loaded and then fails at its first +`require`, with an `Undefined variable: require` reference error rather than the +package-relative CommonJS message below. That is the deliberate direction of +the asymmetry — a false *negative* costs a worse diagnostic, while tokenizing +every candidate file to remove it would cost a parse of every resolved package +entry. + A file classified as CommonJS raises: ```text diff --git a/scripts/differential/m-nodemods.test.js b/scripts/differential/m-nodemods.test.js index 2cc9a4501..fe93f26c5 100644 --- a/scripts/differential/m-nodemods.test.js +++ b/scripts/differential/m-nodemods.test.js @@ -1,4 +1,4 @@ -// Differential suite L — bare-specifier resolution against node_modules. +// Differential suite M — bare-specifier resolution against node_modules. // // The fixture packages are committed under ./mods/nodemods/node_modules, so // bun resolves them natively and goccia resolves them under the diff --git a/scripts/differential/n-nodemods.goccia.test.js b/scripts/differential/n-nodemods.goccia.test.js index f9901e205..9516abd98 100644 --- a/scripts/differential/n-nodemods.goccia.test.js +++ b/scripts/differential/n-nodemods.goccia.test.js @@ -1,4 +1,4 @@ -// Differential suite M — the node_modules behaviours that are goccia's own. +// Differential suite N — the node_modules behaviours that are goccia's own. // // Both are deliberate deviations recorded in docs/module-resolution.md, so // neither bun nor vitest can act as an oracle for them and this file is diff --git a/scripts/differential/p-callintrinsics.test.js b/scripts/differential/p-callintrinsics.test.js index 6d4a214cf..4a7504201 100644 --- a/scripts/differential/p-callintrinsics.test.js +++ b/scripts/differential/p-callintrinsics.test.js @@ -33,9 +33,7 @@ describe("user-defined call/apply/bind on function objects", () => { }); test("own properties on a plain function run instead of the intrinsics", () => { - const host = function () { - return "host"; - }; + const host = () => "host"; host.call = (...args) => `own call(${args.join(",")})`; host.apply = (thisArg, list) => `own apply(${thisArg},[${list.join(",")}])`; host.bind = (...args) => `own bind(${args.join(",")})`; @@ -55,9 +53,7 @@ describe("user-defined call/apply/bind on function objects", () => { return `inherited apply(${tag},${list.length})`; }, }; - const host = function () { - return "host"; - }; + const host = () => "host"; Object.setPrototypeOf(host, behaviour); expect(host.call("t")).toBe("inherited call(t)"); @@ -65,12 +61,15 @@ describe("user-defined call/apply/bind on function objects", () => { }); test("a different built-in installed as `apply` keeps its own semantics", () => { - const inner = function () { - return `inner(thisIsArray=${Array.isArray(this)})`; - }; - const host = function () { - return "host"; - }; + // Extracted from an object literal rather than written with `function`: + // the shorthand method is a real function with a dynamic `this`, which is + // what the receiver assertion below needs. + const inner = ({ + m() { + return `inner(thisIsArray=${Array.isArray(this)})`; + }, + }).m; + const host = () => "host"; host.apply = Reflect.apply; // Reflect.apply(target, thisArgument, argumentsList): the receiver `host` is @@ -82,9 +81,7 @@ describe("user-defined call/apply/bind on function objects", () => { }); test("arity variants of a user-defined call are all forwarded", () => { - const host = function () { - return "host"; - }; + const host = () => "host"; host.call = (...args) => args.length; expect(host.call()).toBe(0); @@ -127,9 +124,11 @@ describe("user-defined call/apply/bind on function objects", () => { }); test("shadowing an intrinsic does not disturb the intrinsic itself", () => { - const host = function (a, b) { - return `${this.tag}:${a}:${b}`; - }; + const host = ({ + m(a, b) { + return `${this.tag}:${a}:${b}`; + }, + }).m; host.call = () => "shadowed"; host.apply = () => "shadowed"; @@ -140,9 +139,11 @@ describe("user-defined call/apply/bind on function objects", () => { }); describe("the Function.prototype intrinsics themselves", () => { - const collect = function (...args) { - return `${this === undefined ? "undefined" : this.tag}:${args.join(",")}`; - }; + const collect = ({ + m(...args) { + return `${this === undefined ? "undefined" : this.tag}:${args.join(",")}`; + }, + }).m; test("call forwards the this value and every argument", () => { const receiver = { tag: "r" }; @@ -213,9 +214,7 @@ describe("the Function.prototype intrinsics themselves", () => { // never reach the callee as a distinguishable value, in any argument count and // through any of the entry points that build the list. test("apply turns argument-array holes into undefined", () => { - const args = function (...rest) { - return rest.map((value) => String(value)).join("|"); - }; + const args = (...rest) => rest.map((value) => String(value)).join("|"); expect(args.apply(undefined, [1, , 3])).toBe("1|undefined|3"); expect(args.apply(undefined, [, 2, 3])).toBe("undefined|2|3"); @@ -229,9 +228,7 @@ describe("the Function.prototype intrinsics themselves", () => { }); test("holes stay undefined through bound functions and detached apply", () => { - const args = function (...rest) { - return rest.map((value) => String(value)).join("|"); - }; + const args = (...rest) => rest.map((value) => String(value)).join("|"); const apply = Function.prototype.apply; expect(args.bind(undefined).apply(undefined, [1, , 3])).toBe("1|undefined|3"); @@ -243,9 +240,7 @@ describe("the Function.prototype intrinsics themselves", () => { }); test("argument-array holes are read through the prototype chain", () => { - const args = function (...rest) { - return rest.map((value) => String(value)).join("|"); - }; + const args = (...rest) => rest.map((value) => String(value)).join("|"); let reads = 0; Object.defineProperty(Array.prototype, 1, { @@ -274,9 +269,7 @@ describe("the Function.prototype intrinsics themselves", () => { // one an engine loses the moment it reads the arguments in whatever order its // call sequence happens to evaluate. test("inherited index getters fire in ascending index order", () => { - const args = function (...rest) { - return rest.map((value) => String(value)).join("|"); - }; + const args = (...rest) => rest.map((value) => String(value)).join("|"); let order = ""; const define = (index) => Object.defineProperty(Array.prototype, index, { @@ -318,9 +311,7 @@ describe("the Function.prototype intrinsics themselves", () => { }); test("a getter that truncates the argument array keeps the original count", () => { - const args = function (...rest) { - return rest.map((value) => String(value)).join("|"); - }; + const args = (...rest) => rest.map((value) => String(value)).join("|"); let reading = null; Object.defineProperty(Array.prototype, 1, { @@ -354,9 +345,7 @@ describe("the Function.prototype intrinsics themselves", () => { }); test("apply uses the array's length, not its dense element count", () => { - const args = function (...rest) { - return rest.map((value) => String(value)).join("|"); - }; + const args = (...rest) => rest.map((value) => String(value)).join("|"); const grown = [1, 2]; grown.length = 5; diff --git a/scripts/test-cli.ts b/scripts/test-cli.ts index f94f0087a..30caf40c2 100644 --- a/scripts/test-cli.ts +++ b/scripts/test-cli.ts @@ -3229,10 +3229,15 @@ console.log("Assertion failure text..."); "}", "class ProtoNamed extends Error {}", "ProtoNamed.prototype.name = 'ProtoAssigned';", + // An explicit prototype name that happens to spell "Error" is still + // the author's answer, so the constructor name must not displace it. + "class ProtoErrorNamed extends Error {}", + "ProtoErrorNamed.prototype.name = 'Error';", 'test("plain error", () => Promise.reject(new Error("boom")));', 'test("subclass error", () => Promise.reject(new MyErr("boom")));', 'test("named subclass error", () => Promise.reject(new NamedErr("boom")));', 'test("prototype-named subclass error", () => Promise.reject(new ProtoNamed("boom")));', + 'test("prototype-named Error subclass", () => Promise.reject(new ProtoErrorNamed("boom")));', 'test("native error", () => Promise.reject(new TypeError("bad")));', 'test("plain object", () => Promise.reject({ code: 42 }));', 'test("message only", () => Promise.reject({ message: "hi" }));', @@ -3256,6 +3261,13 @@ console.log("Assertion failure text..."); `TestRunner (${mode}) should report "${expected}", got: ${rejectionOut}`, ); } + // ProtoErrorNamed spells its prototype name "Error" on purpose, which + // reads identically to the inherited default; only the absence of the + // constructor name tells the two apart. + if (rejectionOut.includes("ProtoErrorNamed: boom")) + throw new Error( + `TestRunner (${mode}) must keep an explicitly assigned "Error" prototype name, got: ${rejectionOut}`, + ); } // Every member the shim does not implement must keep throwing by name. The diff --git a/source/shared/FileUtils.Test.pas b/source/shared/FileUtils.Test.pas index dedfe2a6c..fae2a8be1 100644 --- a/source/shared/FileUtils.Test.pas +++ b/source/shared/FileUtils.Test.pas @@ -30,6 +30,8 @@ TFileUtilsTests = class(TTestSuite) procedure TestMultipleFilesInMultipleSubdirs; procedure TestNoMatchingFilesAmongMany; procedure TestMixedExtensionsAcrossDepths; + procedure TestIsAbsoluteHostPathRootedForms; + procedure TestIsAbsoluteHostPathRelativeForms; public procedure SetupTests; override; procedure BeforeEach; override; @@ -52,6 +54,10 @@ procedure TFileUtilsTests.SetupTests; Test('Multiple files across multiple subdirectories', TestMultipleFilesInMultipleSubdirs); Test('No matching files among many non-matching returns empty', TestNoMatchingFilesAmongMany); Test('Mixed extensions across various depths', TestMixedExtensionsAcrossDepths); + Test('IsAbsoluteHostPath accepts the platform''s rooted spellings', + TestIsAbsoluteHostPathRootedForms); + Test('IsAbsoluteHostPath rejects paths read against a working directory', + TestIsAbsoluteHostPathRelativeForms); end; procedure TFileUtilsTests.BeforeEach; @@ -341,6 +347,39 @@ procedure TFileUtilsTests.TestMixedExtensionsAcrossDepths; end; end; +{ A ceiling directory that is classified absolute is used verbatim; one that is + not is anchored to the directory the setting came from + (Goccia.Modules.Configuration AnchorCeilingDirectory), so misclassifying a + drive-relative or backslash-prefixed path silently moves the capability + boundary. The spellings are platform-specific, so the expectations are too. } +procedure TFileUtilsTests.TestIsAbsoluteHostPathRootedForms; +begin + { A leading '/' roots a path on both platforms. } + Expect(IsAbsoluteHostPath('/usr/local/lib')).ToBe(True); + {$IFNDEF UNIX} + Expect(IsAbsoluteHostPath('C:\packages')).ToBe(True); + Expect(IsAbsoluteHostPath('c:/packages')).ToBe(True); + Expect(IsAbsoluteHostPath('\\server\share\pkg')).ToBe(True); + Expect(IsAbsoluteHostPath('\packages')).ToBe(True); + {$ENDIF} +end; + +procedure TFileUtilsTests.TestIsAbsoluteHostPathRelativeForms; +begin + Expect(IsAbsoluteHostPath('')).ToBe(False); + Expect(IsAbsoluteHostPath('packages')).ToBe(False); + Expect(IsAbsoluteHostPath('./packages')).ToBe(False); + Expect(IsAbsoluteHostPath('../packages')).ToBe(False); + { Drive-relative: resolved against C:'s own working directory, not the root. } + Expect(IsAbsoluteHostPath('C:packages')).ToBe(False); + Expect(IsAbsoluteHostPath('C:')).ToBe(False); + {$IFDEF UNIX} + { A backslash is an ordinary filename character on UNIX. } + Expect(IsAbsoluteHostPath('\packages')).ToBe(False); + Expect(IsAbsoluteHostPath('C:\packages')).ToBe(False); + {$ENDIF} +end; + begin Randomize; TestRunnerProgram.AddSuite(TFileUtilsTests.Create('FileUtils')); diff --git a/source/shared/FileUtils.pas b/source/shared/FileUtils.pas index 2fa66e93f..3ef78fdc8 100644 --- a/source/shared/FileUtils.pas +++ b/source/shared/FileUtils.pas @@ -19,7 +19,12 @@ function FindAllFilesExcludingDirectories(const ADirectory: string; const AFileExtensions: array of string; const AExcludedDirectoryNames: array of string): TStringList; { True when APath is rooted rather than interpreted against a working - directory: a leading path separator, a drive letter, or a UNC prefix. + directory. The test is platform-specific because the spellings are: on UNIX + only a leading '/' roots a path, and a backslash is an ordinary filename + character; on Windows a UNC prefix, a leading separator, or a drive letter + *followed by a separator* does, while the drive-relative `C:packages` is + resolved against that drive's working directory and is therefore not + absolute. (Several units still carry private copies of this predating the shared one; they are unchanged here rather than refactored in passing.) } function IsAbsoluteHostPath(const APath: string): Boolean; @@ -46,15 +51,32 @@ implementation TextEncoding; function IsAbsoluteHostPath(const APath: string): Boolean; +{$IFDEF UNIX} +begin + { A backslash is an ordinary filename character here, so `\packages` is a + relative path, not a rooted one. } + Result := (Length(APath) > 0) and (APath[1] = '/'); +end; +{$ELSE} begin if Length(APath) = 0 then Exit(False); - if (APath[1] = '/') or (APath[1] = '\') then + { A UNC path is rooted at the share. } + if (Copy(APath, 1, 2) = '\\') or (Copy(APath, 1, 2) = '//') then Exit(True); - if (Length(APath) >= 2) and (APath[2] = ':') then + { A leading separator with no drive is root-relative rather than fully + qualified, but it is still rooted: it is not interpreted against the + working directory. } + if (APath[1] = '\') or (APath[1] = '/') then Exit(True); - Result := Copy(APath, 1, 2) = '\\'; + { `C:\x` is rooted; `C:x` is drive-*relative* — resolved against that + drive's own working directory — so only the separator form counts. } + Result := (Length(APath) >= 3) and + (APath[2] = ':') and + ((APath[3] = '\') or (APath[3] = '/')) and + (UpCase(APath[1]) >= 'A') and (UpCase(APath[1]) <= 'Z'); end; +{$ENDIF} function ExpandHostFileName(const APath: string): string; begin diff --git a/source/units/Goccia.AsyncContext.pas b/source/units/Goccia.AsyncContext.pas index 1295c4567..a3dd4afb2 100644 --- a/source/units/Goccia.AsyncContext.pas +++ b/source/units/Goccia.AsyncContext.pas @@ -80,17 +80,29 @@ function EnterAsyncContext( const ASnapshot: TGocciaAsyncContextSnapshot): Integer; procedure LeaveAsyncContext(const AToken: Integer); -{ Drops every snapshot this thread is holding. Called when an engine is torn - down, because snapshots reference that engine's objects and the next engine - on the same thread must not inherit them. - - `enterWith` is why this is not merely tidy: it installs a context with no - scope to leave, so a program that calls it outside a `run` deliberately - leaves one in effect when it ends. Without this reset the next engine on the - worker thread started with the previous engine's snapshot still current, and - marking it walked objects belonging to a realm that no longer exists. } +{ Drops every snapshot this thread is holding. Thread teardown only: an engine + must use EnterEngineAsyncContext/LeaveEngineAsyncContext instead, which do + the same job for the engine's own span without reaching past it. } procedure ResetAsyncContextState; +{ The bracket an engine holds for its whole lifetime. + + Enter hides whatever the thread was already holding and starts the engine on + an empty context, so a worker thread reusing a slot cannot let one engine + inherit the snapshot the previous one left behind — `enterWith` installs a + context with no scope to leave, so a program that calls it outside a `run` + deliberately ends with one in effect, and marking it would walk objects + belonging to a realm that no longer exists. + + Leave restores exactly what Enter hid. Engines nest on one thread — a + ShadowRealm owns a child engine, and freeing it can happen inside the outer + engine's `run` or a microtask callback — so the teardown has to put the outer + engine's context back rather than clear the thread. The displaced snapshot + waits on the same collector-marked stack EnterAsyncContext uses, which is + what keeps it alive while the inner engine runs. } +function EnterEngineAsyncContext: Integer; +procedure LeaveEngineAsyncContext(const AToken: Integer); + { Both derivations tolerate a nil source snapshot and return nil when the result would be empty, so the nil-is-empty representation is closed. } function DeriveAsyncContext(const ASnapshot: TGocciaAsyncContextSnapshot; @@ -344,6 +356,23 @@ procedure ResetAsyncContextState; FreeAndNil(GSnapshotRoots); end; +function EnterEngineAsyncContext: Integer; +begin + Result := EnterAsyncContext(nil); +end; + +procedure LeaveEngineAsyncContext(const AToken: Integer); +begin + LeaveAsyncContext(AToken); + { The root source registered with the collector this engine used, so drop it + once nothing is left for it to mark. While an outer engine still holds a + context it has to stay: nothing else publishes that snapshot, and + EnsureSnapshotRoots is only reached from the next Set/Enter, which may + never come. } + if (not Assigned(GCurrentSnapshot)) and (GSavedSnapshotCount = 0) then + FreeAndNil(GSnapshotRoots); +end; + procedure CleanupAsyncContextThreadState; begin ResetAsyncContextState; diff --git a/source/units/Goccia.Builtins.AsyncHooks.pas b/source/units/Goccia.Builtins.AsyncHooks.pas index 72ee61ea0..300cc023d 100644 --- a/source/units/Goccia.Builtins.AsyncHooks.pas +++ b/source/units/Goccia.Builtins.AsyncHooks.pas @@ -256,10 +256,20 @@ function TGocciaAsyncBoundFunction.Invoke( `thisArg === undefined`, not on the argument count. So a bound function installed as an object method, `{ tag, run: resource.bind(fn) }`, sees the holder as `this`, and only an explicitly non-undefined thisArg displaces - it. Probed against Node v24.0.1. *) - Receiver := FBoundThis; - if not Assigned(Receiver) then - Receiver := AThisValue; + it. Probed against Node v24.0.1. + + A snapshot runner is not a bound function in that sense: Node implements + snapshot() as `AsyncResource.bind((cb, ...args) => cb(...args))`, and the + plain `cb(...)` inside it passes no receiver at all. Installing a runner + as an object method must therefore not hand its holder to the callback. *) + if FUsesCallerTarget then + Receiver := TGocciaUndefinedLiteralValue.UndefinedValue + else + begin + Receiver := FBoundThis; + if not Assigned(Receiver) then + Receiver := AThisValue; + end; CallArgs := TGocciaArgumentsCollection.Create; try diff --git a/source/units/Goccia.Builtins.TestingLibrary.pas b/source/units/Goccia.Builtins.TestingLibrary.pas index fb332a929..1b37d66ac 100644 --- a/source/units/Goccia.Builtins.TestingLibrary.pas +++ b/source/units/Goccia.Builtins.TestingLibrary.pas @@ -372,6 +372,7 @@ implementation TimingUtils, Goccia.Arithmetic, + Goccia.Builtins.Globals, Goccia.Constants.ConstructorNames, Goccia.Constants.ErrorNames, Goccia.Constants.PropertyNames, @@ -776,6 +777,23 @@ function DescribeRejectionReason(const AValue: TGocciaValue): string; NameValue: TGocciaValue; MessageValue: TGocciaValue; ConstructorValue: TGocciaValue; + + { The object in AValue's prototype chain that owns the "name" the read above + resolved to, or nil when nothing in the chain does. } + function OwnNameHolder: TGocciaObjectValue; + var + Current: TGocciaObjectValue; + begin + Result := nil; + Current := TGocciaObjectValue(AValue); + while Assigned(Current) do + begin + if Current.HasOwnProperty(PROP_NAME) then + Exit(Current); + Current := Current.Prototype; + end; + end; + begin Result := DescribeThrowValue(AValue); if not (AValue is TGocciaObjectValue) then @@ -792,7 +810,12 @@ function DescribeRejectionReason(const AValue: TGocciaValue): string; Exit; if TGocciaStringLiteralValue(NameValue).Value <> 'Error' then Exit; - if TGocciaObjectValue(AValue).HasOwnProperty(PROP_NAME) then + { The value "Error" is not enough on its own: an author who wrote + `MyErr.prototype.name = 'Error'` chose that spelling deliberately. Only a + name that Error.prototype itself supplies is the inherited default, so ask + which object in the chain actually owns it rather than only checking the + instance. } + if OwnNameHolder <> GetErrorProto then Exit; { Only a declared class narrows the name: a built-in error's constructor diff --git a/source/units/Goccia.CapabilityAudit.Test.pas b/source/units/Goccia.CapabilityAudit.Test.pas index 8cc1398c2..680f481b7 100644 --- a/source/units/Goccia.CapabilityAudit.Test.pas +++ b/source/units/Goccia.CapabilityAudit.Test.pas @@ -39,7 +39,9 @@ TCapabilityAuditTests = class(TTestSuite) private FRootClampCount: Integer; FSinkInvocationCount: Integer; + FRecordedEvents: TStringList; procedure FailingSink(const AEvent: TGocciaCapabilityAuditEvent); + procedure RecordingSink(const AEvent: TGocciaCapabilityAuditEvent); procedure RootClampSentinel(const APath, ABase, ACanonicalPath: string); procedure TestSerializesStructuredEvent; @@ -49,6 +51,7 @@ TCapabilityAuditTests = class(TTestSuite) procedure TestVMAsyncIteratorCannotCatchSinkFailure; procedure TestFailedRuntimeInstallRestoresRootClampCallback; procedure TestSandboxDetachRestoresExistingModules; + procedure TestEmbeddedNodeModulesGrantEmitsAudit; public procedure SetupTests; override; end; @@ -68,6 +71,8 @@ procedure TCapabilityAuditTests.SetupTests; TestFailedRuntimeInstallRestoresRootClampCallback); Test('Sandbox detach restores existing runtime modules', TestSandboxDetachRestoresExistingModules); + Test('An embedded node_modules grant emits the capability audit event', + TestEmbeddedNodeModulesGrantEmitsAudit); end; constructor TFailingRootClampRuntimeExtension.Create( @@ -105,6 +110,13 @@ procedure TCapabilityAuditTests.FailingSink( raise ECapabilityAuditSinkFailure.Create(AEvent.Subject); end; +procedure TCapabilityAuditTests.RecordingSink( + const AEvent: TGocciaCapabilityAuditEvent); +begin + FRecordedEvents.Add(CapabilityKindName(AEvent.Kind) + '|' + + CapabilityDecisionName(AEvent.Decision) + '|' + AEvent.Subject); +end; + procedure TCapabilityAuditTests.RootClampSentinel( const APath, ABase, ACanonicalPath: string); begin @@ -355,6 +367,34 @@ procedure TCapabilityAuditTests.TestSandboxDetachRestoresExistingModules; end; end; +{ TGocciaEngine.AllowNodeModules is the embedding host's entry point for the + capability. The CLI emits its own event around its direct resolver call, so + an embedder that never touches the CLI has to get one from here or the grant + is invisible to an auditor. } +procedure TCapabilityAuditTests.TestEmbeddedNodeModulesGrantEmitsAudit; +var + Source: TStringList; + Executor: TGocciaInterpreterExecutor; + Engine: TGocciaEngine; +begin + FRecordedEvents := TStringList.Create; + Source := TStringList.Create; + Executor := TGocciaInterpreterExecutor.Create; + Engine := TGocciaEngine.Create('audit-node-modules.js', Source, Executor); + try + Engine.CapabilityAuditSink := RecordingSink; + Engine.AllowNodeModules; + Expect(FRecordedEvents.Count).ToBe(1); + Expect(FRecordedEvents[0]).ToBe('modules.node-modules|allow|'); + finally + Engine.Free; + Executor.Free; + Source.Free; + FRecordedEvents.Free; + FRecordedEvents := nil; + end; +end; + begin TestRunnerProgram.AddSuite( TCapabilityAuditTests.Create('Capability Audit')); diff --git a/source/units/Goccia.Engine.Realm.Test.pas b/source/units/Goccia.Engine.Realm.Test.pas index 211b13c9d..2146d9cb1 100644 --- a/source/units/Goccia.Engine.Realm.Test.pas +++ b/source/units/Goccia.Engine.Realm.Test.pas @@ -10,6 +10,7 @@ TestingPascalLibrary, Goccia.Arguments.Collection, + Goccia.AsyncContext, Goccia.Engine, Goccia.ExecutionContext, Goccia.Executor, @@ -52,6 +53,7 @@ TTestEngineRealm = class(TTestSuite) procedure TestSequentialEnginesHaveFreshURLSearchParamsPrototype; procedure TestSequentialEnginesHaveFreshURLPrototype; procedure TestNestedEngineRestoresOuterRealmOnDestroy; + procedure TestNestedEngineRestoresOuterAsyncContextOnDestroy; procedure TestEachEngineGetsADistinctRealm; procedure TestInterpreterExecutionContextUsesEngineRealm; procedure TestBytecodeExecutionContextUsesEngineRealm; @@ -84,6 +86,8 @@ procedure TTestEngineRealm.SetupTests; TestSequentialEnginesHaveFreshURLPrototype); Test('Destroying a nested engine restores the outer engine''s realm', TestNestedEngineRestoresOuterRealmOnDestroy); + Test('Destroying a nested engine restores the outer async context', + TestNestedEngineRestoresOuterAsyncContextOnDestroy); Test('Each engine owns a distinct realm instance', TestEachEngineGetsADistinctRealm); Test('Interpreter execution context uses the engine realm', @@ -469,6 +473,60 @@ procedure TTestEngineRealm.TestNestedEngineRestoresOuterRealmOnDestroy; end; end; +{ An engine's teardown used to clear the thread's async-context state outright, + which is correct for a worker thread reusing a slot but wrong for the nested + lifetimes the engine supports: a ShadowRealm owns a child engine, and freeing + it can happen inside the outer engine's run or a microtask callback. Clearing + there stripped the outer engine's AsyncLocalStorage binding mid-run. } +procedure TTestEngineRealm.TestNestedEngineRestoresOuterAsyncContextOnDestroy; +var + OuterEngine, InnerEngine: TGocciaEngine; + OuterExecutor, InnerExecutor: TGocciaInterpreterExecutor; + OuterSource, InnerSource: TStringList; + OuterContext: TGocciaAsyncContextSnapshot; + Key, Store: TGocciaValue; +begin + OuterSource := TStringList.Create; + OuterSource.Text := ''; + InnerSource := TStringList.Create; + InnerSource.Text := ''; + + OuterExecutor := TGocciaInterpreterExecutor.Create; + InnerExecutor := TGocciaInterpreterExecutor.Create; + try + OuterEngine := TGocciaEngine.Create('', OuterSource, + OuterExecutor); + try + Key := TGocciaStringLiteralValue.Create('storage-key'); + Store := TGocciaStringLiteralValue.Create('outer-store'); + OuterContext := DeriveAsyncContext(nil, Key, Store); + SetCurrentAsyncContext(OuterContext); + Expect(CurrentAsyncContext = OuterContext).ToBe(True); + + InnerEngine := TGocciaEngine.Create('', InnerSource, + InnerExecutor); + try + // A nested engine starts on an empty context rather than inheriting + // the outer engine's, whose stores belong to the outer realm. + Expect(CurrentAsyncContext = nil).ToBe(True); + finally + InnerEngine.Free; + end; + + Expect(CurrentAsyncContext = OuterContext).ToBe(True); + finally + OuterEngine.Free; + InnerSource.Free; + OuterSource.Free; + end; + // The outermost engine's own teardown still leaves the thread clean. + Expect(CurrentAsyncContext = nil).ToBe(True); + finally + InnerExecutor.Free; + OuterExecutor.Free; + end; +end; + procedure TTestEngineRealm.TestEachEngineGetsADistinctRealm; var EngineA, EngineB: TGocciaEngine; diff --git a/source/units/Goccia.Engine.pas b/source/units/Goccia.Engine.pas index 0888edcce..3c75c56ad 100644 --- a/source/units/Goccia.Engine.pas +++ b/source/units/Goccia.Engine.pas @@ -188,6 +188,9 @@ TGocciaEngine = class FFunctionConstructor: TGocciaFunctionConstructorClassValue; FTypedArrayIntrinsic: TGocciaClassValue; FSuppressWarnings: Boolean; + { The async-context bracket this engine holds for its whole lifetime; see + EnterEngineAsyncContext. } + FAsyncContextToken: Integer; FLastTiming: TGocciaScriptResult; FLastSourceMap: TGocciaSourceMap; procedure SetStrictTypes(const AValue: Boolean); @@ -856,6 +859,10 @@ procedure TGocciaEngine.Initialize(const AFileName: string; const ASourceLines: TStringList; const AModuleLoader: TGocciaModuleLoader; const AOwnsModuleLoader: Boolean); begin + { Not a valid token until EnterEngineAsyncContext returns one, so a + constructor that fails before then cannot make Destroy unwind past an + enclosing engine's entry. } + FAsyncContextToken := -1; FSourcePath := AFileName; FSourceLines := ASourceLines; FModuleLoader := AModuleLoader; @@ -870,6 +877,11 @@ procedure TGocciaEngine.Initialize(const AFileName: string; TGocciaCallStack.Initialize; TGocciaMicrotaskQueue.Initialize; + { Start on an empty async context and remember what this thread was holding, + so neither a reused worker-thread slot nor an enclosing engine leaks its + snapshot into this one. Destroy restores it. } + FAsyncContextToken := EnterEngineAsyncContext; + // Per-realm intrinsic state (Array.prototype, ...) lives on FRealm. The // execution-context stack makes it current after the global environment is // available, before any built-in construction performs lazy intrinsic lookup. @@ -939,9 +951,11 @@ procedure TGocciaEngine.Initialize(const AFileName: string; destructor TGocciaEngine.Destroy; begin { Async-context snapshots hold this engine's objects, and `enterWith` can - leave one installed with no scope to unwind it. Drop them before anything - else is torn down so the next engine on this thread cannot inherit them. } - ResetAsyncContextState; + leave one installed with no scope to unwind it. Drop everything this engine + left behind before anything else is torn down, and restore whatever was + current when it was constructed — engines nest on one thread, so clearing + the thread outright would strip an outer engine's context mid-run. } + LeaveEngineAsyncContext(FAsyncContextToken); if (TGarbageCollector.Instance <> nil) and Assigned(FInterpreter) then TGarbageCollector.Instance.RemoveRootObject(FInterpreter.GlobalScope); @@ -1716,6 +1730,16 @@ procedure TGocciaEngine.AddAlias(const APattern, AReplacement: string); procedure TGocciaEngine.AllowNodeModules(const ACeilingDirectory: string); begin Resolver.AllowNodeModules(ACeilingDirectory); + { The grant is a host decision, not a script action, so it is emitted once at + configuration time. The subject is the effective ceiling the resolver + normalized — empty when the walk is unbounded, which is the part an auditor + most needs to see. An embedding host that calls this API instead of going + through the CLI gets the same event; the CLI reaches the resolver directly, + so nothing is emitted twice. } + if Resolver.NodeModulesEnabled then + EmitCapabilityAudit(gckNodeModulesResolution, gcdAllow, + Resolver.NodeModulesCeiling, + 'bare specifiers resolve against node_modules'); end; procedure TGocciaEngine.SetAllowedFetchHosts(const AHosts: TStrings); diff --git a/source/units/Goccia.Evaluator.PatternMatching.pas b/source/units/Goccia.Evaluator.PatternMatching.pas index 0dbd26bf8..19e3ab723 100644 --- a/source/units/Goccia.Evaluator.PatternMatching.pas +++ b/source/units/Goccia.Evaluator.PatternMatching.pas @@ -572,6 +572,7 @@ function TryMatchArrayItems(const AItems: TGocciaValueList; I: Integer; CurrentContext, NextContext: TGocciaEvaluationContext; RestArray: TGocciaArrayValue; + RestArrayRoot: TGocciaTempRoot; Success: Boolean; begin if not Assigned(ARestPattern) and not AHasRestWildcard and @@ -601,9 +602,20 @@ function TryMatchArrayItems(const AItems: TGocciaValueList; RestArray := TGocciaArrayValue.Create; for I := AElements.Count to AItems.Count - 1 do RestArray.Elements.Add(AItems[I]); - if not TryMatchPatternInternal(RestArray, ARestPattern, CurrentContext, - NextContext) then - Exit(False); + { The rest array is derived here, so the caller's subject root does not + cover it and this local is its only reference. The subpattern below can + run guest code — a guard, a custom matcher, a computed key — and any of + that is a collecting safe point that would free it out from under the + binding. } + InitializeTempRoot(RestArrayRoot); + AddTempRootIfNeeded(RestArrayRoot, RestArray); + try + if not TryMatchPatternInternal(RestArray, ARestPattern, CurrentContext, + NextContext) then + Exit(False); + finally + RemoveTempRootIfNeeded(RestArrayRoot); + end; CurrentContext := NextContext; end; @@ -640,6 +652,7 @@ function TryMatchObjectPattern(const ASubject: TGocciaValue; MatchedKeys: TStringList; MatchedSymbols: TList; Remainder: TGocciaObjectValue; + RemainderRoot: TGocciaTempRoot; RestSubject: TGocciaObjectValue; Entry: TPair; SymbolEntry: TPair; @@ -690,20 +703,30 @@ function TryMatchObjectPattern(const ASubject: TGocciaValue; Exit(False); Remainder := TGocciaObjectValue.Create; - for Entry in RestSubject.GetEnumerablePropertyEntries do - begin - if MatchedKeys.IndexOf(Entry.Key) < 0 then - Remainder.AssignProperty(Entry.Key, Entry.Value); - end; - for SymbolEntry in RestSubject.GetEnumerableSymbolProperties do - begin - if not MatchedSymbols.Contains(SymbolEntry.Key) then - Remainder.AssignSymbolProperty(SymbolEntry.Key, SymbolEntry.Value); - end; + { Same reason the array rest subject is rooted: the remainder object is + derived here, is reachable from this local alone, and the subpattern + below can run guest code that collects. Rooted from creation because + the property copies below allocate too. } + InitializeTempRoot(RemainderRoot); + AddTempRootIfNeeded(RemainderRoot, Remainder); + try + for Entry in RestSubject.GetEnumerablePropertyEntries do + begin + if MatchedKeys.IndexOf(Entry.Key) < 0 then + Remainder.AssignProperty(Entry.Key, Entry.Value); + end; + for SymbolEntry in RestSubject.GetEnumerableSymbolProperties do + begin + if not MatchedSymbols.Contains(SymbolEntry.Key) then + Remainder.AssignSymbolProperty(SymbolEntry.Key, SymbolEntry.Value); + end; - if not TryMatchPatternInternal(Remainder, APattern.RestPattern, - CurrentContext, NextContext) then - Exit(False); + if not TryMatchPatternInternal(Remainder, APattern.RestPattern, + CurrentContext, NextContext) then + Exit(False); + finally + RemoveTempRootIfNeeded(RemainderRoot); + end; CurrentContext := NextContext; end; diff --git a/source/units/Goccia.Evaluator.pas b/source/units/Goccia.Evaluator.pas index ce89fa959..2e7f6ea13 100644 --- a/source/units/Goccia.Evaluator.pas +++ b/source/units/Goccia.Evaluator.pas @@ -3717,6 +3717,29 @@ procedure RunBaseSuperclassInitializers(const AClassValue: TGocciaClassValue; AContext); end; +{ ES2026 §10.2.2 [[Construct]] step 13.c: a ~derived~ constructor whose body + returned undefined must have initialized `this`, which only super() does. + LastSuperConstructorCalled is the record the method value keeps of the call + that just returned, so this has to run immediately after CallWithThisValue. + + An implicit constructor is never a candidate: §15.7.14 step 15a synthesizes + one that forwards to super unconditionally, and it has no method value to + ask. } +procedure RequireDerivedConstructorThisInitialized( + const AClassValue: TGocciaClassValue; const AResult: TGocciaValue); +begin + if not ClassRequiresObjectConstructorReturn(AClassValue) then + Exit; + if Assigned(AResult) and not (AResult is TGocciaUndefinedLiteralValue) then + Exit; + if not Assigned(AClassValue.ConstructorMethod) then + Exit; + if AClassValue.ConstructorMethod.LastSuperConstructorCalled then + Exit; + ThrowReferenceError( + 'Must call super constructor before returning from derived constructor'); +end; + function InvokeConstructableWithReceiver(const AConstructor: TGocciaValue; const AArguments: TGocciaArgumentsCollection; const AReceiver: TGocciaValue; @@ -3837,6 +3860,11 @@ function InvokeConstructableWithReceiver(const AConstructor: TGocciaValue; SuperResult := ClassConstructor.ConstructorMethod.CallWithThisValue( AArguments, AReceiver, ConstructorThisValue, EffectiveNewTarget); ValidateClassConstructorReturn(ClassConstructor, SuperResult); + // ES2026 §10.2.2 step 13.c: a derived constructor returning undefined + // must have initialized `this`. The same check the `new` operator makes + // in InstantiateClass, so every route into a derived constructor body + // reports the missing super() rather than handing back a bare receiver. + RequireDerivedConstructorThisInitialized(ClassConstructor, SuperResult); if not (SuperResult is TGocciaObjectValue) and (ConstructorThisValue is TGocciaObjectValue) then SuperResult := ConstructorThisValue; @@ -4206,6 +4234,10 @@ function EvaluateCallWithOptionalShortCircuit( SuperResult := SuperClass.ConstructorMethod.CallWithThisValue( Arguments, AContext.Scope.ThisValue, ConstructorThisValue, AContext.Scope.FindNewTarget); + // §10.2.2 step 13.c again: the superclass constructor this super() + // just entered is itself derived, so returning without calling its own + // super() leaves `this` uninitialized. + RequireDerivedConstructorThisInitialized(SuperClass, SuperResult); if SuperResult is TGocciaObjectValue then begin AContext.Scope.ThisValue := TGocciaObjectValue(SuperResult); @@ -11449,11 +11481,21 @@ function InstantiateClass(const AClassValue: TGocciaClassValue; // anywhere up the superclass chain. // // The walk reads FSuperClass, which is resolved once at -// ClassDefinitionEvaluation time. A later Object.setPrototypeOf on the -// constructor therefore does not move this answer — deliberately: FSuperClass -// is also what Instantiate and InstantiateClass drive construction from, so a -// guard that disagreed with them would route a class into a path that then -// built it from the other chain. +// ClassDefinitionEvaluation time, and deliberately does not follow +// GetConstructorPrototype. §15.7.14 step 9 fixes [[ConstructorKind]] and the +// super chain when the class is defined; a later Object.setPrototypeOf on the +// constructor changes static-method `super` lookups and nothing about +// [[Construct]], so a guard that moved with it would decline exactly the +// classes that must keep being redirected. Probed against Node v24.0.1: +// `class WithMap extends Map {}; class Sub {}` with Sub retargeted onto +// WithMap constructs an ordinary object with Sub.prototype and no Map +// internals, which is what this guard's answer produces here. +// +// InstantiateClass's implicit-constructor branch does consult +// GetConstructorPrototype, so the two disagree for a retargeted constructor. +// That substitution is itself the deviation — it makes a retargeted base class +// run the other class's constructor, which Node does not — and correcting it +// belongs with the construction paths, not with this guard. function ClassChainReachesNativeConstruction( const AClassValue: TGocciaClassValue): Boolean; var @@ -11470,6 +11512,33 @@ function ClassChainReachesNativeConstruction( Result := False; end; +{ The evaluation context a class's own definition environment supplies. + + Every field is filled the way TGocciaFunctionValue builds a call context out + of its closure: a field initializer is guest code, so a context missing a + host callback is not a degraded context but a crashing one — + TGocciaImportCallExpression calls AContext.LoadModule with no assigned-check, + and CurrentFilePath is what `import()` and `import.meta` resolve against and + what coverage and call-stack frames are attributed to. } +function BuildClassDefinitionContext(const AClassValue: TGocciaClassValue; + const ADefinitionScope: TGocciaScope): TGocciaEvaluationContext; +begin + Result := Default(TGocciaEvaluationContext); + Result.Realm := AClassValue.CreationRealm; + Result.Scope := ADefinitionScope; + Result.OnError := ADefinitionScope.OnError; + Result.LoadModule := ADefinitionScope.LoadModule; + Result.LoadModuleSource := ADefinitionScope.LoadModuleSource; + Result.LoadDeferredModule := ADefinitionScope.LoadDeferredModule; + Result.ResolveModuleURL := ADefinitionScope.ResolveModuleURL; + Result.CurrentFilePath := AClassValue.DefinitionSourcePath; + Result.CoverageEnabled := (TGocciaCoverageTracker.Instance <> nil) and + TGocciaCoverageTracker.Instance.Enabled; + Result.StrictTypes := ADefinitionScope.EffectiveStrictTypes; + Result.NonStrictMode := ADefinitionScope.EffectiveNonStrictMode; + Result.CompatibilityNonStrictMode := Result.NonStrictMode; +end; + // ES2026 §7.3.14 Construct(F, argumentsList, newTarget) for a class the // tree-walk evaluator built. Reflect.construct (§28.1.2), the proxy // [[Construct]] fallback (§10.5.13), construction through a bound wrapper @@ -11548,13 +11617,7 @@ function RedirectEvaluatorClassConstruct(const ATarget: TGocciaValue; { The class environment is the only context this construction can be anchored to — the caller is a native function with none of its own — and it is the one §15.7.10 ClassFieldDefinitionEvaluation step 2b names as the - initializers' [[Environment]] anyway. Every field is filled the way - TGocciaFunctionValue builds a call context out of its closure: a field - initializer is guest code, so a context missing a host callback is not a - degraded context but a crashing one — TGocciaImportCallExpression calls - AContext.LoadModule with no assigned-check, and CurrentFilePath is what - `import()` and `import.meta` resolve against and what coverage and - call-stack frames are attributed to. + initializers' [[Environment]] anyway. ApplyClassDefinitionSourceContext repairs the same four callbacks and the file path again around each initializer run, so removing either half alone @@ -11562,20 +11625,7 @@ function RedirectEvaluatorClassConstruct(const ATarget: TGocciaValue; dead code to be tidied away: this one covers everything InstantiateClass does with the context outside an initializer run, and that one covers a superclass initialized from a context this function never built. } - EvalContext := Default(TGocciaEvaluationContext); - EvalContext.Realm := ClassValue.CreationRealm; - EvalContext.Scope := DefinitionScope; - EvalContext.OnError := DefinitionScope.OnError; - EvalContext.LoadModule := DefinitionScope.LoadModule; - EvalContext.LoadModuleSource := DefinitionScope.LoadModuleSource; - EvalContext.LoadDeferredModule := DefinitionScope.LoadDeferredModule; - EvalContext.ResolveModuleURL := DefinitionScope.ResolveModuleURL; - EvalContext.CurrentFilePath := ClassValue.DefinitionSourcePath; - EvalContext.CoverageEnabled := (TGocciaCoverageTracker.Instance <> nil) and - TGocciaCoverageTracker.Instance.Enabled; - EvalContext.StrictTypes := DefinitionScope.EffectiveStrictTypes; - EvalContext.NonStrictMode := DefinitionScope.EffectiveNonStrictMode; - EvalContext.CompatibilityNonStrictMode := EvalContext.NonStrictMode; + EvalContext := BuildClassDefinitionContext(ClassValue, DefinitionScope); { Same realm discipline as TGocciaClassValue.Instantiate: the intrinsics a field initializer allocates (object and array literals, errors) come from @@ -11596,6 +11646,42 @@ function RedirectEvaluatorClassConstruct(const ATarget: TGocciaValue; Result := True; end; +// The evaluator's half of TryRunASTInstanceElements: a class whose instance +// elements are AST expressions can only have them run here, and a holder that +// is not the evaluator — the bytecode VM driving a compiled subclass's super() +// into a superclass the evaluator built, which is what a module's top-level +// function declarations produce even in bytecode mode — has no context of its +// own to run them in. Without this the fields are silently dropped in bytecode +// mode and present in interpreted mode. +procedure RunEvaluatorClassInstanceElements( + const AClassValue: TGocciaClassValue; const AInstance: TGocciaValue); +var + DefinitionScope: TGocciaScope; + EvalContext: TGocciaEvaluationContext; + PreviousRealm: TGocciaRealm; + SwapRealm: Boolean; +begin + if not (AInstance is TGocciaObjectValue) then + Exit; + DefinitionScope := AClassValue.DefinitionScope; + if not Assigned(DefinitionScope) then + Exit; + + EvalContext := BuildClassDefinitionContext(AClassValue, DefinitionScope); + PreviousRealm := CurrentRealm; + SwapRealm := Assigned(AClassValue.CreationRealm) and + (AClassValue.CreationRealm <> PreviousRealm); + if SwapRealm then + SetCurrentRealm(AClassValue.CreationRealm); + try + RunClassInstanceInitializers(AClassValue, + TGocciaObjectValue(AInstance), EvalContext); + finally + if SwapRealm then + SetCurrentRealm(PreviousRealm); + end; +end; + // Template literals without real interpolations are returned as static strings. // The parser pre-segments templates with interpolations into // TGocciaTemplateWithInterpolationExpression, so this function only handles @@ -13241,5 +13327,6 @@ function EvaluateDelete(const AOperand: TGocciaExpression; const AContext: TGocc initialization RegisterClassConstructRedirectHook(RedirectEvaluatorClassConstruct); + RegisterClassInstanceElementsHook(RunEvaluatorClassInstanceElements); end. diff --git a/source/units/Goccia.Modules.NodeResolution.pas b/source/units/Goccia.Modules.NodeResolution.pas index a148c857a..cacab10f8 100644 --- a/source/units/Goccia.Modules.NodeResolution.pas +++ b/source/units/Goccia.Modules.NodeResolution.pas @@ -135,7 +135,12 @@ function HasInvalidPathSegment(const APath: string): Boolean; ERR_INVALID_PACKAGE_TARGET otherwise. } function IsValidExportsTarget(const ATarget: string): Boolean; -{ True when APath is ADirectory itself or lives beneath it. +{ True when APath lives strictly beneath ADirectory. + + The comparison is against ADirectory plus a trailing separator, so ADirectory + itself does NOT pass: this answers "is this file inside the package", and + every caller passes a resolved file path. A caller that needs the directory + to count as inside itself must say so explicitly rather than assume it. The final containment gate: segment validation rejects the specifiers and targets that are invalid on their face, and this catches whatever a @@ -148,7 +153,13 @@ function IsPathInsideDirectory(const APath, ADirectory: string): Boolean; True when the source carries CommonJS markers (`require(...)`, `module.exports`, `exports.x`) and no ES module markers. The asymmetry is deliberate: a file with both is being read as an ES module by every other - toolchain, and a file with neither is inert and loads fine either way. } + toolchain, and a file with neither is inert and loads fine either way. + + The scan matches raw text and does not tokenize, so an `import` or `export` + keyword inside a comment or a string literal counts as an ES module marker. + That direction is the safe one: the file is loaded rather than refused, and + fails on its own terms at the first `require`. Removing the false negative + would cost a parse of every resolved package entry. } function LooksLikeCommonJSSource(const ASource: string): Boolean; { Whether a resolved file inside a package must be refused as CommonJS. diff --git a/source/units/Goccia.VM.pas b/source/units/Goccia.VM.pas index 81ff8c105..816f35482 100644 --- a/source/units/Goccia.VM.pas +++ b/source/units/Goccia.VM.pas @@ -2716,9 +2716,13 @@ TGocciaBytecodeAsyncGeneratorObjectValue = class(TGocciaAsyncGeneratorBaseValu until it is made there too. Async context is deliberately NOT recorded per request. A body observes - the context of whichever call resumed it, and that already falls out of - the microtask seam: every resumption reaches the body through a promise - reaction, which carries the snapshot captured where it was registered. + the context of whichever call resumed it, and that falls out of the two + execution paths without any per-request bookkeeping: a request that finds + the queue idle is started synchronously on the resuming call's own stack, + under that call's context, while a request that had to wait — a queued + second next(), or a body suspended on await — reaches the body through a + promise reaction, which carries the snapshot captured where it was + registered. Probed against Node v24.0.1 across for-await, a generator created in one context and resumed in another, a queued second request overlapping a running one, and nested for-await under different stores; both executors @@ -5766,9 +5770,16 @@ function InvokeConstructableWithReceiver(const AConstructor: TGocciaValue; if not (SuperResult is TGocciaObjectValue) and (ConstructorThisValue is TGocciaObjectValue) then SuperResult := ConstructorThisValue; + // A constructor whose super() returned a replacement object had this + // class's instance elements run and stamped onto that object by + // InitializeCurrentCtorReceiver already; running them a second time + // would re-evaluate every initializer and re-stamp the private brand. + // Same guard TGocciaVMClassValue.TryConstructOnReceiver applies. if (SuperResult is TGocciaObjectValue) and (SuperResult <> AReceiver) and - (SuperResult = ConstructorThisValue) then + (SuperResult = ConstructorThisValue) and + not HasBytecodePrivateInitializersApplied(SuperResult, + ClassConstructor) then VMClassConstructor.FVM.RunClassInitializers(ClassConstructor, SuperResult); end else if Assigned(ClassConstructor.ConstructorMethod) then @@ -6183,11 +6194,28 @@ function TGocciaVMSuperConstructorValue.Call( ConstructorThisValue: TGocciaValue; ImplicitSuperInitialized: Boolean; WasSuperAlreadyCalled: Boolean; + PreviousSuperClassSuperCalled: Boolean; + SuperClassCalledItsOwnSuper: Boolean; ReceiverPrototype: TGocciaObjectValue; function IsUndefinedConstructedValue(const AValue: TGocciaValue): Boolean; begin Result := (not Assigned(AValue)) or (AValue is TGocciaUndefinedLiteralValue); end; + { ES2026 §10.2.2 [[Construct]] step 13.c: a ~derived~ superclass constructor + that returns undefined must have initialized `this`, which only its own + super() does. The compiler emits an unconditional implicit `undefined` + return and OP_CHECK_DERIVED_THIS only guards `this` *access*, so a body + that calls neither reaches here with no error of its own. } + procedure RequireSuperClassThisInitialized(const AValue: TGocciaValue; + const ACalledItsOwnSuper: Boolean); + begin + if (Assigned(SuperClass.SuperClass) or + Assigned(SuperClass.NativeSuperConstructor)) and + IsUndefinedConstructedValue(AValue) and + not ACalledItsOwnSuper then + ThrowReferenceError( + 'Must call super constructor before returning from derived constructor'); + end; procedure ValidateSuperConstructorResult(const AValue: TGocciaValue); begin if (Assigned(SuperClass.SuperClass) or @@ -6291,9 +6319,22 @@ function TGocciaVMSuperConstructorValue.Call( if not SuperClass.HasDerivedConstructorKind then TGocciaVMClassValue(SuperClass).FVM.RunClassInitializers( SuperClass, AThisValue); - SuperResult := TGocciaVMClassValue(SuperClass).FVM.InvokeFunctionValue( - TGocciaVMClassValue(SuperClass).FConstructorValue, - AArguments, AThisValue); + { The flag belongs to the constructor being entered, and it is read back + before being restored so this frame can tell whether that constructor + called its own super(). } + PreviousSuperClassSuperCalled := + TGocciaVMClassValue(SuperClass).FVM.FCurrentConstructorSuperCalled; + TGocciaVMClassValue(SuperClass).FVM.FCurrentConstructorSuperCalled := False; + try + SuperResult := TGocciaVMClassValue(SuperClass).FVM.InvokeFunctionValue( + TGocciaVMClassValue(SuperClass).FConstructorValue, + AArguments, AThisValue); + finally + SuperClassCalledItsOwnSuper := + TGocciaVMClassValue(SuperClass).FVM.FCurrentConstructorSuperCalled; + TGocciaVMClassValue(SuperClass).FVM.FCurrentConstructorSuperCalled := + PreviousSuperClassSuperCalled; + end; if SuperResult is TGocciaObjectValue then begin // §10.2.2 step 12: an object the super constructor *returns* replaces @@ -6304,6 +6345,7 @@ function TGocciaVMSuperConstructorValue.Call( Exit(SuperResult); end; ValidateSuperConstructorResult(SuperResult); + RequireSuperClassThisInitialized(SuperResult, SuperClassCalledItsOwnSuper); if TGocciaVMClassValue(SuperClass).FConstructorValue is TGocciaBytecodeFunctionValue then begin BytecodeConstructor := TGocciaBytecodeFunctionValue( @@ -6328,11 +6370,19 @@ function TGocciaVMSuperConstructorValue.Call( if Assigned(SuperClass.ConstructorMethod) then begin - // §10.2.2 step 5b again: pre-initialize only for a ~base~ superclass. - if (SuperClass is TGocciaVMClassValue) and - not SuperClass.HasDerivedConstructorKind then - TGocciaVMClassValue(SuperClass).FVM.RunClassInitializers( - SuperClass, AThisValue); + // §10.2.2 step 5b again: pre-initialize only for a ~base~ superclass. The + // superclass need not be a compiled one: CallWithThisValue below runs only + // the constructor body, so an evaluator-built base superclass reaching + // this branch loses its instance elements unless they run here too. + if not SuperClass.HasDerivedConstructorKind then + begin + if SuperClass is TGocciaVMClassValue then + TGocciaVMClassValue(SuperClass).FVM.RunClassInitializers( + SuperClass, AThisValue) + else if FCurrentCtorClass is TGocciaVMClassValue then + TGocciaVMClassValue(FCurrentCtorClass).FVM.RunClassInitializers( + SuperClass, AThisValue); + end; SuperResult := SuperClass.ConstructorMethod.CallWithThisValue( AArguments, AThisValue, ConstructorThisValue, FNewTarget); if SuperResult is TGocciaObjectValue then @@ -6344,6 +6394,10 @@ function TGocciaVMSuperConstructorValue.Call( Exit(SuperResult); end; ValidateSuperConstructorResult(SuperResult); + { An AST constructor records the same thing on its method value, which is + what the tree-walk evaluator reads. } + RequireSuperClassThisInitialized(SuperResult, + SuperClass.ConstructorMethod.LastSuperConstructorCalled); if ConstructorThisValue is TGocciaObjectValue then begin if (ConstructorThisValue <> AThisValue) and @@ -6565,17 +6619,37 @@ function TGocciaVMClassValue.TryConstructOnReceiver( var ConstructorThisValue: TGocciaValue; PreviousConstructorSuperCalled: Boolean; + PreviousPendingNewTarget: TGocciaValue; + ConstructorSuperCalled: Boolean; function HasDerivedConstructorReturnRestriction: Boolean; begin Result := Assigned(SuperClass) or Assigned(NativeSuperConstructor); end; + function EffectiveNewTarget: TGocciaValue; + begin + if Assigned(ANewTarget) then + Exit(ANewTarget); + Result := Self; + end; begin Result := True; if not Assigned(FConstructorValue) then begin - AResult := FVM.InvokeImplicitSuperInitialization(Self, AReceiver, - AArguments); + // ES2026 §10.2.2 [[Construct]] step 5: the implicit constructor forwards + // newTarget up the chain, and InvokeImplicitSuperInitialization reads it + // off FPendingNewTarget to pick the receiver's prototype. Leaving whatever + // an earlier construction parked there would build the instance from the + // wrong constructor; native initialization can return without consuming + // the value, so it is restored rather than cleared. + PreviousPendingNewTarget := FVM.FPendingNewTarget; + FVM.FPendingNewTarget := EffectiveNewTarget; + try + AResult := FVM.InvokeImplicitSuperInitialization(Self, AReceiver, + AArguments); + finally + FVM.FPendingNewTarget := PreviousPendingNewTarget; + end; if not Assigned(AResult) then AResult := AReceiver; Exit; @@ -6600,6 +6674,9 @@ function TGocciaVMClassValue.TryConstructOnReceiver( AResult := FVM.InvokeFunctionValue(FConstructorValue, AArguments, AReceiver); finally + // Read the flag before restoring it: it belongs to the constructor that + // just returned, exactly as Instantiate captures it. + ConstructorSuperCalled := FVM.FCurrentConstructorSuperCalled; FVM.FCurrentConstructorSuperCalled := PreviousConstructorSuperCalled; end; @@ -6612,6 +6689,17 @@ function TGocciaVMClassValue.TryConstructOnReceiver( ThrowTypeError('Derived constructor returned non-object', SSuggestNotConstructorType); + // ES2026 §10.2.2 step 13.c: a derived constructor that returns undefined + // must have initialized `this`. The compiler emits an unconditional implicit + // `undefined` return and OP_CHECK_DERIVED_THIS only guards `this` *access*, + // so a body that never calls super() and never touches `this` reaches here + // with no error of its own. + if HasDerivedConstructorReturnRestriction and + ((not Assigned(AResult)) or (AResult is TGocciaUndefinedLiteralValue)) and + not ConstructorSuperCalled then + ThrowReferenceError( + 'Must call super constructor before returning from derived constructor'); + if FConstructorValue is TGocciaBytecodeFunctionValue then ConstructorThisValue := RegisterToValue(FVM.FLastClosureThisValue) else @@ -10562,6 +10650,15 @@ procedure TGocciaVM.RunClassInitializers(const AClassValue: TGocciaClassValue; AClassValue.RunMethodInitializers(AInstance); AClassValue.RunFieldInitializers(AInstance); AClassValue.RunDecoratorFieldInitializers(AInstance); + { A class the tree-walk evaluator built keeps its instance elements as AST + expressions rather than as the closure-shaped initializers above, so + those three calls do nothing for it. Such a class still turns up under + the VM in bytecode mode — a module's top-level function declarations are + created and run by the evaluator, so a class declared inside one is a + plain TGocciaClassValue that a compiled subclass can extend — and + dropping its fields there was a mode divergence, not a missing fast + path. } + TryRunASTInstanceElements(AClassValue, AInstance); finally FPrivateInitializerReceiver := PreviousPrivateInitializerReceiver; FPrivateInitializerPreserveExisting := diff --git a/source/units/Goccia.Values.ClassValue.pas b/source/units/Goccia.Values.ClassValue.pas index 845a411ed..6a2e265f0 100644 --- a/source/units/Goccia.Values.ClassValue.pas +++ b/source/units/Goccia.Values.ClassValue.pas @@ -166,10 +166,12 @@ TGocciaClassValue = class(TGocciaObjectValue) // are initialized: a ~base~ constructor does it before its body (§10.2.2 // step 5b), a ~derived~ one when super() returns (§13.3.7.1 step 11). // Reports True when a resolved superclass or a linked native super - // constructor is present. Known gap: `class A extends null {}` is - // ~derived~ per the spec but reports False here, because extends-null - // records no superclass. Tree-walk `extends null` is separately broken; - // do not lean on this predicate for it. + // constructor is present. `class A extends null {}` reports True, which is + // the spec-correct answer (§15.7.14 step 9a makes it ~derived~): the + // evaluator links Function.prototype as the native super constructor for + // it, and the construction paths recognise that sentinel and raise the + // TypeError §10.2.2 requires when the implicit derived constructor tries + // to call super(). function HasDerivedConstructorKind: Boolean; function HasInstanceInitializerWork: Boolean; // ECMAScript: number of expected constructor parameters before the first @@ -423,6 +425,30 @@ function GetNativePrototypeFromConstructor( const ANewTarget: TGocciaValue; const ACurrentRealmDefault: TGocciaObjectValue): TGocciaObjectValue; +type + { Runs a class's AST-declared instance elements against AInstance. + + §15.7.10 ClassFieldDefinitionEvaluation records an evaluator-built class's + fields as expressions in InstancePropertyDefs, not as the closure-shaped + values RunFieldInitializers walks, and evaluating an expression needs an + evaluation context. Only the tree-walk evaluator has one, so it registers + this hook and every other holder of a TGocciaClassValue — including the + bytecode VM — reaches those elements through it. } + TGocciaClassInstanceElementsHook = procedure( + const AClassValue: TGocciaClassValue; const AInstance: TGocciaValue); + +procedure RegisterClassInstanceElementsHook( + const AHook: TGocciaClassInstanceElementsHook); +{ True when AClassValue's instance elements are AST-declared, which is exactly + the classes the evaluator built. A compiled class records field initializers + as closures instead and always reports False here. } +function HasASTInstanceElements( + const AClassValue: TGocciaClassValue): Boolean; +{ Runs them when there are any and an evaluator registered itself; reports + whether it ran anything. } +function TryRunASTInstanceElements(const AClassValue: TGocciaClassValue; + const AInstance: TGocciaValue): Boolean; + implementation uses @@ -466,6 +492,33 @@ implementation Goccia.Values.WeakRefValue, Goccia.Values.WeakSetValue; +var + GClassInstanceElementsHook: TGocciaClassInstanceElementsHook; + +procedure RegisterClassInstanceElementsHook( + const AHook: TGocciaClassInstanceElementsHook); +begin + GClassInstanceElementsHook := AHook; +end; + +function HasASTInstanceElements( + const AClassValue: TGocciaClassValue): Boolean; +begin + Result := Assigned(AClassValue) and + ((AClassValue.InstancePropertyDefs.Count > 0) or + (AClassValue.PrivateInstancePropertyDefs.Count > 0)); +end; + +function TryRunASTInstanceElements(const AClassValue: TGocciaClassValue; + const AInstance: TGocciaValue): Boolean; +begin + Result := Assigned(GClassInstanceElementsHook) and + (AInstance is TGocciaObjectValue) and + HasASTInstanceElements(AClassValue); + if Result then + GClassInstanceElementsHook(AClassValue, AInstance); +end; + function ToNumberConstructorValue( const AValue: TGocciaValue): TGocciaNumberLiteralValue; begin diff --git a/source/units/Goccia.Values.FunctionBase.pas b/source/units/Goccia.Values.FunctionBase.pas index e22b6375f..b8e7a3cc7 100644 --- a/source/units/Goccia.Values.FunctionBase.pas +++ b/source/units/Goccia.Values.FunctionBase.pas @@ -1043,6 +1043,12 @@ constructor TGocciaFunctionSharedPrototype.Create; Assert(Intrinsic is TGocciaNativeFunctionValue, 'Function.prototype.' + AName + ' must be a native function to carry ' + 'its intrinsic kind'); + // Production builds define PRODUCTION and compile with {$C-} + // (source/shared/Shared.inc), so the assertion above is gone there. The + // type test has to stand on its own rather than let an unchecked cast + // write IntrinsicKind through whatever the property actually holds. + if not (Intrinsic is TGocciaNativeFunctionValue) then + Exit; TGocciaNativeFunctionValue(Intrinsic).IntrinsicKind := AKind; end; diff --git a/source/units/Goccia.Values.GeneratorValue.pas b/source/units/Goccia.Values.GeneratorValue.pas index 259c03c95..7b9eeb62f 100644 --- a/source/units/Goccia.Values.GeneratorValue.pas +++ b/source/units/Goccia.Values.GeneratorValue.pas @@ -79,9 +79,13 @@ TGocciaAsyncGeneratorObjectValue = class(TGocciaAsyncGeneratorBaseValue) until it is made there too. Async context is deliberately NOT recorded per request. A body observes - the context of whichever call resumed it, and that already falls out of - the microtask seam: every resumption reaches the body through a promise - reaction, which carries the snapshot captured where it was registered. + the context of whichever call resumed it, and that falls out of the two + execution paths without any per-request bookkeeping: a request that finds + the queue idle is started synchronously on the resuming call's own stack, + under that call's context, while a request that had to wait — a queued + second next(), or a body suspended on await — reaches the body through a + promise reaction, which carries the snapshot captured where it was + registered. Probed against Node v24.0.1 across for-await, a generator created in one context and resumed in another, a queued second request overlapping a running one, and nested for-await under different stores; both executors diff --git a/tests/built-ins/AsyncHooks/bind.js b/tests/built-ins/AsyncHooks/bind.js index 2955107e1..735c40e52 100644 --- a/tests/built-ins/AsyncHooks/bind.js +++ b/tests/built-ins/AsyncHooks/bind.js @@ -60,6 +60,21 @@ describe("AsyncLocalStorage statics", () => { .toBe("holder"); }); + test("a snapshot runner does not forward its own receiver", () => { + // Node implements snapshot() as AsyncResource.bind((cb, ...args) => + // cb(...args)), and that plain call passes no receiver, so a runner + // installed as an object method must not hand its holder to the callback. + // Probed against Node v24.0.1. + const read = ({ + read() { + return this === undefined ? "undefined" : "leaked"; + }, + }).read; + const runner = AsyncLocalStorage.snapshot(); + expect(({ tag: "holder", run: runner }).run(read)).toBe("undefined"); + expect(runner(read)).toBe("undefined"); + }); + test("does not shadow Function.prototype.bind for ordinary functions", () => { const target = { tag: "target" }; const read = ((value) => [target.tag, value]).bind(null, 1); diff --git a/tests/built-ins/Function/prototype/apply.js b/tests/built-ins/Function/prototype/apply.js index 2032ab0f0..1bbd03ef0 100644 --- a/tests/built-ins/Function/prototype/apply.js +++ b/tests/built-ins/Function/prototype/apply.js @@ -135,9 +135,11 @@ describe("Function.prototype.apply", () => { }); try { - // One-, two- and three-element arrays take the small-argument fast path; - // longer ones go through the generic list build. Both must observe the - // inherited accessor rather than substituting undefined for the hole. + // A hole disqualifies the small-argument fast path at every length, so + // all four of these go through the generic list build regardless of how + // short they are. Each must observe the inherited accessor rather than + // substituting undefined for the hole — including the bound wrapper, + // which reaches apply through a different callee. expect(collect.apply(undefined, [1, , 3])).toBe("1|inherited|3"); expect(collect.apply(undefined, [1, ,])).toBe("1|inherited"); expect(collect.apply(undefined, [1, , 3, 4])).toBe("1|inherited|3|4"); diff --git a/tests/language/classes/derived-constructor-missing-super.js b/tests/language/classes/derived-constructor-missing-super.js new file mode 100644 index 000000000..6665b9af8 --- /dev/null +++ b/tests/language/classes/derived-constructor-missing-super.js @@ -0,0 +1,87 @@ +/*--- +description: A derived constructor that returns without calling super() raises a ReferenceError through every construction route +features: [class-inheritance, Reflect, class] +---*/ + +// ES2026 §10.2.2 [[Construct]] step 13.c. The check belongs to the constructor +// that returned, so it has to fire wherever that constructor was entered from: +// `new`, a subclass's super(), Reflect.construct, and a bound wrapper all +// reach it through different machinery. In bytecode mode the compiler emits an +// unconditional implicit `undefined` return and the derived-this guard only +// covers `this` *access*, so a body that touches neither has no error of its +// own to raise. Probed against Node v24.0.1: every one of these throws. +// +// Known deviation, identical in both modes and therefore not a parity break: a +// subclass with no constructor of its own (`class L extends Middle {}`) still +// constructs successfully. Its implicit constructor forwards through a +// different set of paths, none of which consult the flag yet. + +class Base {} + +class Middle extends Base { + constructor() { + // Deliberately never calls super() and never reads `this`. + } +} + +class Leaf extends Middle { + constructor() { + super(); + } +} + +const expectMissingSuper = (build) => { + let name = ""; + try { + build(); + } catch (error) { + name = error.name; + } + expect(name).toBe("ReferenceError"); +}; + +describe("a derived constructor that never calls super()", () => { + test("`new` on the constructor itself throws", () => { + expectMissingSuper(() => new Middle()); + }); + + test("a subclass's explicit super() throws", () => { + expectMissingSuper(() => new Leaf()); + }); + + test("Reflect.construct throws", () => { + expectMissingSuper(() => Reflect.construct(Middle, [])); + }); + + test("construction through a bound wrapper throws", () => { + expectMissingSuper(() => new (Middle.bind(null))()); + }); + + test("a constructor that does call super() is unaffected", () => { + class Ok extends Base { + seq = 1; + + constructor() { + super(); + this.tail = 2; + } + } + + expect(Object.keys(new Ok())).toEqual(["seq", "tail"]); + expect(Object.keys(Reflect.construct(Ok, []))).toEqual(["seq", "tail"]); + expect(Object.keys(new (Ok.bind(null))())).toEqual(["seq", "tail"]); + }); + + test("an explicit object return stands in for super()", () => { + // §10.2.2 step 13.a: returning an Object is the other way a derived + // constructor can finish, and it is checked before step 13.c. + class Returns extends Base { + constructor() { + return { replaced: true }; + } + } + + expect(new Returns().replaced).toBe(true); + expect(Reflect.construct(Returns, []).replaced).toBe(true); + }); +}); diff --git a/tests/language/classes/replacement-receiver-initializes-once.js b/tests/language/classes/replacement-receiver-initializes-once.js new file mode 100644 index 000000000..6f76017b1 --- /dev/null +++ b/tests/language/classes/replacement-receiver-initializes-once.js @@ -0,0 +1,96 @@ +/*--- +description: A derived class whose super() returns a replacement object initializes its instance elements exactly once on it +features: [class-inheritance, class-fields, private-fields, Reflect, class] +---*/ + +// ES2026 §10.2.2 step 12 and §13.3.7.1 step 11: when the super constructor +// returns an object, that object becomes the receiver and the derived class's +// instance elements run on it — once. Every construction route reaches the +// derived constructor through different machinery, and a route that both +// initializes the replacement receiver and then replays the initializers +// afterwards evaluates every field twice and stamps the private brand twice, +// which the second stamp reports as a repeated super() call. + +let ticks = 0; + +class ReplacingBase { + constructor() { + return { replaced: true }; + } +} + +class Derived extends ReplacingBase { + seq = ++ticks; + + #brand = "derived"; + + constructor() { + super(); + this.tail = "tail"; + } + + // The replacement object keeps Object.prototype, so the brand is only + // reachable through a static of the class that stamped it. + static readBrand(instance) { + return instance.#brand; + } +} + +const expectInitializedOnce = (build) => { + const before = ticks; + const instance = build(); + + expect(ticks - before).toBe(1); + expect(instance.replaced).toBe(true); + expect(instance.seq).toBe(ticks); + expect(instance.tail).toBe("tail"); + expect(Derived.readBrand(instance)).toBe("derived"); + expect(Object.getPrototypeOf(instance)).toBe(Object.prototype); + expect(Object.keys(instance)).toEqual(["replaced", "seq", "tail"]); +}; + +describe("a super() that returns a replacement object", () => { + test("`new` initializes the replacement once", () => { + expectInitializedOnce(() => new Derived()); + }); + + test("Reflect.construct initializes the replacement once", () => { + expectInitializedOnce(() => Reflect.construct(Derived, [])); + }); + + test("a bound wrapper initializes the replacement once", () => { + expectInitializedOnce(() => new (Derived.bind(null))()); + }); + + test("a further subclass's super() initializes each layer once", () => { + class Leaf extends Derived { + leafSeq = ++ticks; + + #leafBrand = "leaf"; + + constructor() { + super(); + this.leafTail = "leaf-tail"; + } + + static readLeafBrand(instance) { + return instance.#leafBrand; + } + } + + const before = ticks; + const leaf = new Leaf(); + + expect(ticks - before).toBe(2); + expect(leaf.replaced).toBe(true); + expect(Derived.readBrand(leaf)).toBe("derived"); + expect(Leaf.readLeafBrand(leaf)).toBe("leaf"); + expect(Object.keys(leaf)).toEqual([ + "replaced", + "seq", + "tail", + "leafSeq", + "leafTail", + ]); + }); +}); diff --git a/tests/language/modules/hoisted-function-import-capture/class-construction.js b/tests/language/modules/hoisted-function-import-capture/class-construction.js index 5f6bb5c45..fbb3b979c 100644 --- a/tests/language/modules/hoisted-function-import-capture/class-construction.js +++ b/tests/language/modules/hoisted-function-import-capture/class-construction.js @@ -16,6 +16,8 @@ import { fnDeclBoundConstruct, fnDeclClosureRead, fnDeclConstruct, + makeEvaluatorBuiltBase, + makeSubclassOf, fnDeclDerivedConstruct, fnDeclLocalImplicitSubclassConstruct, fnDeclLocalSubclassConstruct, @@ -140,6 +142,55 @@ describe("imported module functions construct module classes", () => { expect(stamped.secret()).toBe("id-secret"); }); + // §10.2.2 step 5: an implicit constructor forwards newTarget up the chain, + // and the compiled superclass's implicit branch reads it back off the VM to + // pick the receiver's prototype. Constructing from the entry file is what + // makes the answer observable — the exotic Array receiver is allocated + // several links above the class `new` names. + test("an implicit compiled constructor forwards newTarget", () => { + class CompiledMid extends Array {} + const Sub = makeSubclassOf(CompiledMid); + + const direct = new Sub(); + expect(Object.getPrototypeOf(direct)).toBe(Sub.prototype); + expect(Array.isArray(direct)).toBe(true); + + const Other = class Other extends CompiledMid {}; + const redirected = Reflect.construct(Sub, [], Other); + expect(Object.getPrototypeOf(redirected)).toBe(Other.prototype); + expect(Array.isArray(redirected)).toBe(true); + }); + + // TGocciaMethodValue.CallWithThisValue runs only the constructor body, so a + // superclass whose instance elements are AST expressions loses them unless + // the VM hands them back to the evaluator. Interpreted mode never had the + // gap, which made this a mode divergence rather than a missing fast path. + test("a compiled subclass of an evaluator-built base runs the base's fields", () => { + const Base = makeEvaluatorBuiltBase(); + + class ExplicitSuper extends Base { + own = "explicit"; + + constructor() { + super(3); + this.tail = "explicit-tail"; + } + } + + class ImplicitSuper extends Base {} + + const explicit = new ExplicitSuper(); + expect(Object.keys(explicit)).toEqual(["label", "n", "own", "tail"]); + expect(explicit.label).toBe("id-evaluator-base"); + expect(explicit.n).toBe(3); + expect(explicit.brand()).toBe("id-evaluator-brand"); + + const implicit = new ImplicitSuper(5); + expect(implicit.label).toBe("id-evaluator-base"); + expect(implicit.n).toBe(5); + expect(implicit.brand()).toBe("id-evaluator-brand"); + }); + test("entry-file construction of a derived module class initializes once", () => { const ticksBefore = derivedTickCount(); const counted = new Counted(30); diff --git a/tests/language/modules/hoisted-function-import-capture/helpers/module-classes.js b/tests/language/modules/hoisted-function-import-capture/helpers/module-classes.js index 3485ff217..56fd9a627 100644 --- a/tests/language/modules/hoisted-function-import-capture/helpers/module-classes.js +++ b/tests/language/modules/hoisted-function-import-capture/helpers/module-classes.js @@ -149,3 +149,30 @@ export class Factory { return new Point(1, 2); } } + +// A class *created inside* a hoisted function declaration: in bytecode mode +// the declaration runs in the tree-walk evaluator, so this class records its +// instance elements as AST expressions rather than as compiled initializers. +// Extending it from the entry file is what puts an evaluator-built superclass +// under a compiled one. +export function makeEvaluatorBuiltBase() { + return class EvaluatorBuiltBase { + label = PREFIX + "evaluator-base"; + + #brand = PREFIX + "evaluator-brand"; + + constructor(n) { + this.n = n; + } + + brand() { + return this.#brand; + } + }; +} + +// Another evaluator-built class, this time with no constructor of its own, so +// construction reaches the compiled superclass's *implicit* branch. +export function makeSubclassOf(Base) { + return class EvaluatorBuiltSub extends Base {}; +} diff --git a/tests/language/pattern-matching/gc.js b/tests/language/pattern-matching/gc.js index b4db5fea0..f48e92cbe 100644 --- a/tests/language/pattern-matching/gc.js +++ b/tests/language/pattern-matching/gc.js @@ -126,6 +126,73 @@ describe.runIf(hasGoccia)("pattern matching GC safety", () => { expect(result).toBe(1); }); + test("collecting inside an array rest subpattern keeps the rest array alive", () => { + // The rest array is built here, not by the caller, so the subject root the + // match expression installed does not cover it: a collection taken by the + // guard below could reclaim it before the binding reads it. + const check = (value) => { + Goccia.gc(); + Goccia.gc(); + return Array.isArray(value); + }; + + const result = match ([1, 2, 3, 4]) { + [const head, ...const rest] if (check(rest)): head + rest.length; + default: -1; + }; + + expect(result).toBe(4); + }); + + test("collecting inside an array rest custom matcher keeps the rest array alive", () => { + class Pair { + static [Symbol.customMatcher](subject) { + Goccia.gc(); + Goccia.gc(); + return Array.isArray(subject) && subject.length === 2; + } + } + + const result = match ([0, 1, 2]) { + [const head, ...Pair]: head; + default: -1; + }; + + expect(result).toBe(0); + }); + + test("collecting inside an object rest subpattern keeps the remainder alive", () => { + const check = (value) => { + Goccia.gc(); + Goccia.gc(); + return Object.keys(value).length === 2; + }; + + const result = match ({ a: 1, b: 2, c: 3 }) { + { a: const a, ...const rest } if (check(rest)): a + rest.b + rest.c; + default: -1; + }; + + expect(result).toBe(6); + }); + + test("collecting inside an object rest custom matcher keeps the remainder alive", () => { + class TwoKeys { + static [Symbol.customMatcher](subject) { + Goccia.gc(); + Goccia.gc(); + return Object.keys(subject).length === 2; + } + } + + const result = match ({ a: 1, b: 2, c: 3 }) { + { a: const a, ...TwoKeys }: a; + default: -1; + }; + + expect(result).toBe(1); + }); + test("collecting inside an is-expression pattern keeps the subject alive", () => { const key = () => { Goccia.gc();