ReviewOS

zig-utils/zig-js

Strings: bound variadic concat construction

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

Parents: #473, #493 Related: #480, #777, #781, #782

Problem

String.prototype.concat with two or more arguments still converts each argument to canonical WTF-8 and appends through ArrayListUnmanaged. The growable buffer can copy an expanding prefix repeatedly, flat Latin-1 arguments require whole-input snapshots, and the path does not enforce the engine string-size ceiling before growth.

#782 deliberately fixed binary + and the zero/one-argument concat cases. This child owns the remaining variadic construction path.

Scope

  • Preserve receiver coercion and left-to-right ToString ordering for every argument.
  • Retain exact StringData identity when all other pieces are empty.
  • Compute checked canonical output length, including high/low surrogate pairs spanning any adjacent non-empty pieces.
  • Enforce the engine string-size ceiling before output allocation.
  • Stream every flat Latin-1 and canonical WTF-8 piece into one exact owned output buffer.
  • Reuse one shared construction implementation with the two-value path without changing binary + semantics.

No-workaround rules

  • No rope placeholder, repeated pairwise rebuilding, arbitrary argument cap, lossy surrogate handling, source recognition, cached result, weaker RangeError/OOM behavior, or growable output buffer.
  • No stacks, queues, jobs, Promise/microtask, Worker lifecycle, or Map/Set iterator changes.

Acceptance

  • Multi-argument empty pieces retain the exact sole non-empty StringData cell without output allocation.
  • Non-empty results use checked exact sizing and one canonical output buffer.
  • Flat Latin-1 and surrogate pairs spanning empty/non-empty argument boundaries remain exact.
  • Coercions occur once each in left-to-right order and abrupt completion stops later coercions.
  • Focused normal/TSan/OOM units, exact-parent Test262 concat accounting, threadfuzz, and the full unit gate pass without regressions.

Completed integration

CI run 33113309062 completed successfully on main commit 8a1398ffc04e9e30987f968693667c7271ca3ce1, a descendant containing this implementation. All 53 executed jobs succeeded (unit and TSan-unit shards, Linux no-GIL TSan, functional/no-GIL corpus gates, fuzz profiles, ABI, Wasm, Test262 parallel, and docs); the scheduled-only nightly job was skipped. Together with the measured local evidence in the comments, this completes this issue's acceptance.

9 days ago

Implementation audit update: the variadic path now publishes the receiver and every original argument before the first observable ToString, then replaces each precise-root slot with its completed string. This closes the moving-compaction gap for both generic receivers and later arguments while retaining once-only left-to-right coercion. The audit also found the same raw-local pattern in general binary-operator coercion; that broader, separately testable work is tracked as #784 so this issue remains scoped to String.prototype.concat.

9 days ago

Landed 9f79e91c on main.

Evidence:

  • focused normal variadic suite: 2/2 passed, 0 leaked;
  • focused TSan variadic suite: 2/2 passed, 0 leaked;
  • adjacent two-value regression: 1/1 passed in normal and TSan;
  • exact-parent Test262 test/built-ins/String/prototype/concat: 22 pass, 0 fail (0 flips);
  • deterministic threadfuzz: 400 programs from seed 1, 0 failures.

The focused OOM witness is included in the normal/TSan variadic unit. The final acceptance box remains open for the pushed full CI gate.

9 days ago

Integration update: run 33077388393 was superseded by the #784 push after completing 54 jobs. Its only failure was the exact pre-existing #722 signature in cve/mc-lock-stop-vs-park.js (expected 400 but got 401); no string/concat gate failed. The active descendant run 33078418866 contains both #783 and #784 and is the final integration evidence.

Sign in to comment on this issue.