ReviewOS

zig-utils/zig-js

VM: preserve object-literal function metadata and canonical keys

#815
Closed chris opened this 9 days ago ·
9 days ago

Parent: #465. Related: #806, #808. Class NamedEvaluation before static effects is tracked separately in #814.

At main 209201a, four retained required-bytecode Test262 failures reproduce unchanged: object/method-definition/fn-name-fn.js, fn-name-gen.js, generator-name-prop-symbol.js and name-name-prop-symbol.js. Forced tree-walker passes, required bytecode misses the Symbol-derived function names.

The wider audit confirms three object-literal metadata defects:

  1. Computed data-property lowering omits the source-level anonymous function/method naming step. String and described/empty Symbol keys fail for concise methods, functions, arrows and generators.
  2. Both static and computed VM/native property initialization set HomeObject whenever the stored callable is a method, even when the syntax only copies an existing method. Copying a.read into another literal changes super in the original function. The source-level MethodDefinition distinction must survive lowering.
  3. Computed accessor lowering leaves an unconverted key for vmInitAccessor, which performs keyOf and keyDisplayName on the original object. Its @@toPrimitive is called twice. ToPropertyKey must occur once, before function creation/publication, and the canonical key must be retained.

Examples:

var a = { __proto__: { x: 7 }, read() { return super.x; } };
var b = { __proto__: { x: 9 }, read: a.read };
// Both calls must remain 7; copying is not MakeMethod.
a.read() === 7 && b.read() === 7;
var calls = 0;
var key = { [Symbol.toPrimitive]() { calls++; return 'x'; } };
var object = { get [key]() { return 7; } };
// calls must be 1, with accessor name 'get x'.

Acceptance:

  • Carry explicit source-derived naming/MakeMethod intent through canonical bytecode and native operation metadata; never infer it from the runtime callable's flags or rename copied values.
  • Preserve once-only ToPropertyKey, accessor prefixes, exact Symbol/string names and function-name descriptors, HomeObject identity, named/existing callable controls and ordinary/generator/async function forms.
  • Force tree-walker, required VM and native paths; cover key-coercion abrupt/moving boundaries and allocation replay, shared no-GIL/TSan, full units and seeded fuzz.
  • Measure the same pinned upstream scopes before/after with nonzero denominators, and link conventional issue-specific commits and complete pushed CI.

Current local metadata probes excluding #814's class-static-name case: 10/32 passing entry forms, versus Node 32/32. These are ordinary automatic IIFEs and generator entries, not claimed as a forced-tree matrix. Four upstream paired witnesses independently prove the required-tier mismatch. Exact sources, logs, current-main runner and hashes are retained at /Users/chris/.cache/zig-js-vm-naming.7Wj8Ri.

No blanket runtime renaming, method rebinding on copies, duplicate coercion, source rewrite, fallback coverage substitution, disabled GC or native stack/queue/job lifecycle redesign.

9 days ago

The full positive Object-literal paired census now measures eight required-tier gains: 998/1,007 -> 1,006/1,007 forced-tree/required paired passes, with no regressions. The 163 negative/fixture files remain outside this positive witness denominator. Default-tier Object and Function reports remain byte-identical (1,677 pass, two existing Function realm failures), so these gains are explicitly VM-only.

The remaining Object-literal paired failure is fn-name-class.js, linked to #814. It is not being hidden with late class renaming.

The initial focused build passed ten checks (including actual native execution, syntax lowering, allocation replay and shared no-GIL lanes), but a moving-Symbol-key check failed in forced tree-walker. Diagnosis found a pre-existing description-storage ownership bug, now tracked as #816: the Symbol's identity and getter result survive a verified evacuation, but description bytes retained from a String cell become stale. The fix belongs at Symbol construction, not in the name consumer. Validation remains in progress; this is not a completion claim.

Inactive compiler caches have been removed after preserving before/after corpus executables, compressed failing unit binaries, hashes, sources and logs. The broader census overlapped a compile/list-only Debug build, never an active unit run.

9 days ago

The separate Symbol storage fix #816 is now pushed as 61310f6. Continued ownership validation also confirmed #817: the public name String was copied, but internal Function.name retained borrowed computed-key bytes. The shared naming-boundary fix passes both new direct/moving witnesses and the full focused Debug metadata suite (13/13). It will accompany the #815 metadata change; final sanitizer/conformance/fuzz builds are running, and sharded units plus pushed CI remain pending.

9 days ago

Landed as fix(vm): preserve literal function metadata and ownership (#815), authored/committed as Chris, no trailers. This also implements the shared diagnostic-name ownership fix tracked in #817. #816's separate Symbol storage fix is already in its ancestry.

Final saved-candidate evidence (61310f6 + exact name-fixed.patch):

  • Forced-tree/required-VM Object positives: 998/1,007 -> 1,006/1,007, eight required-tier gains, zero measured regressions. The 163 negative/fixture files remain outside that denominator. The remaining class-name failure belongs to #814.
  • Default Object/Function/Symbol reports are byte-identical: 1,775 passing and two existing Function realm failures, zero default-tier flips. Local #815 entry forms pass 32/32; #814's two static-name forms remain visibly failing in the wider 34-form report.
  • Debug metadata 13/13; unsuppressed ReleaseSafe+TSan metadata 13/13 and Symbol 24/24, zero failures/leaks/race reports. Coverage includes actual native execution, immutable lowering flags/root maps, five moving fixtures in both forced tiers, allocation replay, copied shared methods without the GIL, and direct/moving diagnostic-name ownership.
  • Seed-1 Debug threadfuzz: 400 default + 300 exact-value verifier cases, zero failures.
  • Public C API and GC relocation inventory audits pass.

The native four-shard full Debug suite is running against the retained exact candidate binary, with 2,132 linked tests and historical timing-based scheduling. Its result is still pending. The commit rebased cleanly over six independently pushed commits (error diagnostics and the new native --vm-witness-subtree driver) before publication. Local measurements above are explicitly pre-rebase; complete pushed CI remains the combined-main gate. This issue and #817 stay open until terminal evidence is complete.

Retained logs, hashes, patches and final executables are under /Users/chris/.cache/zig-js-vm-naming.7Wj8Ri. The ordinary command interface recovered after a secondary-runtime reset interrupted the first full-suite startup; that empty attempt is not counted. The restarted native suite is the authoritative local full run. Superseded caches and duplicate binaries were removed; the successfully restored temporary stash is archived as a patch and dropped after landing, with all user work and older stashes preserved.

9 days ago

The native full Debug suite for the exact saved #815/#817 candidate completed: 2,131 passed, one platform skip, zero failed/leaked, 2,132 linked tests, 744.0 seconds across four shards. Provenance is 61310f6 plus name-fixed.patch (before rebasing the independent error-message changes into 6bb4b77), retained under /Users/chris/.cache/zig-js-vm-naming.7Wj8Ri with binary hashes and shard logs. Current combined-main CI is not green: the no-GIL 400/401 recurrence is tracked in #722; the stale exact JSON RangeError spelling is tracked in #818. Neither gate is silently waived.

9 days ago

Closing with terminal combined-main integration evidence: CI 33211154123 at e24c4fa completed successfully: 53 successful checks, one skipped nightly-only check, zero failed/cancelled checks. The implementation fix(vm): preserve literal function metadata and ownership (#815) is in that revision. This completes the pending integration condition alongside the scoped local/conformance evidence recorded above. Separately discovered Error-intrinsic, Array-realm, prototype-type and restriction-path defects remain open in #823, #824, #825 and #826; this closure does not claim those are fixed.

Sign in to comment on this issue.