Skip to content

Fix pi's small compile gaps - #1348

Merged
ctate merged 17 commits into
mainfrom
opt/pi-small-fixes
Oct 20, 2026
Merged

ctate merged 17 commits into
mainfrom
opt/pi-small-fixes

Conversation

@cramforce

Copy link
Copy Markdown
Contributor

The pi coding agent diagnosis turned up a set of small gaps on pi's startup, provider-stream and terminal paths. Each fix is its own commit and lands with Node-differential corpus programs, which pass in both the plain and sanitized lanes. The optional-chain miscompile from the same diagnosis lands separately (opt/optional-chain-fix).

Changes

  1. Assign process.emitWarning. The assignment stores the replacement. Later process.emitWarning(...) calls apply it to their own arguments. Warnings the runtime builds itself (util.deprecate and Node's own modules) reach it as (message, type[, code]), and no report is printed. Assigning process.title already worked; the corpus program pins both.
  2. Type process.getBuiltinModule results as module namespaces. Under @types/node, a string-literal id that names a supported builtin now lowers to the same namespace value an import * as binding has. This includes pi's spelling (process as P).getBuiltinModule?.("node:os") ?? null. A nullable namespace binding that has been narrowed to present uses the same member tables, and a namespace is truthy. Reading process.features as a whole value gives an object with none of the flags set, which matches the existing reads of its members.
  3. for await over class instances whose [Symbol.asyncIterator] is an async generator method. This is pi-ai's EventStream, which every provider stream goes through. An async * declaration annotated with AsyncIterator, AsyncIterableIterator or AsyncIterable maps like AsyncGenerator. Boxing a class instance whose fields hold a promise now links the dynamic async unit. Without that, linking failed on EventStream's this.isComplete(event).
  4. WeakMap- and WeakSet-typed class fields. These are stored in the same checked-dynamic storage that locals of those types already use.
  5. chalk.bold() from TypeScript. TypeScript that imports chalk's JavaScript source types the default export as a plain function, so chalk.bold is an any member on a typed receiver. Compile chalk 5 through the dynamic object model #1309 only handled receivers that are themselves untyped, so this shape was still refused. A member the checker leaves untyped now takes the same path, which still refuses a string receiver at run time. The chalk 5 fixture now covers root, aliased and Chalk-instance bold calls.
  6. Bare core-module names resolve to builtins. An installed npm string_decoder (or events, ...) no longer captures import "string_decoder", because Node checks its builtins first. Builtins that only exist with the node: prefix, such as node:test, leave their bare names to packages.
  7. pi-tui terminal setup. The presence check if (process.stdin.setRawMode) now answers the method on a TTY and undefined otherwise. process.stdin.setEncoding decodes with the shared StringDecoder: a multibyte character split across reads stays whole, and the buffered tail is flushed before 'end'. String 'data' listeners are supported. process.stdout.on/once/off("resize") works, backed by a SIGWINCH listener that does not keep the event loop alive and only emits when a TTY's size actually changed.
  8. delete this.stack on Error subclasses that declare a stack accessor. quickjs-wasi's JSException does this. Compiled errors keep the stack trace internally, so the delete only needs to evaluate its receiver.
  9. Runtime variants for the new runtime behavior. The emitWarning replacement (item 1) and the stdin encoding, string 'data' listeners and 'resize' support (item 7) are compiled only into two new runtime-pack variants: SCR_WARNING_REPLACE of scr_async_dyn.c and SCR_STDIO_EXTENDED of scr_events.c. The link selects them when the program assigns process.emitWarning, or reaches setEncoding, a string 'data' listener or 'resize' (the compiler's warningReplace / stdioExtended link features). The default objects are byte-identical to main's, so the shared warning paths, stdin delivery and TLS layout of every other executable are unchanged. The development and sanitizer builds pass the same defines. The darwin matrix lists the variants too, so the macOS runtime pack has to be rebuilt to provide them.

Not in this PR

  • A computed class field keyed by a well-known symbol ([Symbol.toStringTag] = "LRUCache", from lru-cache). Fields named with sym: need to be materialized for the dynamic view, Object.prototype.toString and inspect, and only methods support that today.
  • node:sea, which pi's extension loader probes with process.getBuiltinModule("node:sea")?.isSea(), is not a supported builtin.
  • this.decoder.decode(...) inside a method compiled with a dynamic this (quickjs-wasi handleHostCall). The class's dynamic view cannot expose a TextDecoder record field. Fixing it needs a dynamic handle that holds the codec record by reference. A repro is in the results directory.
  • Destructuring from globalThis in TypeScript (const { X } = globalThis as unknown as T), and the local typing of evalCode() results in quickjs-wasi callers.
  • Found while testing: "stack" in e is false natively for a class stack accessor, where Node answers true.

Validation

  • Corpus programs 6231–6237 and the extended chalk 5 fixture pass in the plain and sanitized lanes, with 6231 and 6236 re-run against the variant runtime pack. The related existing corpus slices (async iteration, stdin, readline, tty, signals, warnings, weak collections, chalk, npm) also pass. Failures that reproduce identically on the base build were left alone: 5614, 5254, 5372, 5568, 5564 and npm node24-url-authority. These are Node 24.15 vs 24.22 differences in the sandbox.
  • pnpm lint and pnpm format:check are clean. pnpm test:ts7 --baselines-only is complete. The runtime-pack, backend, executable-cache, runtime-features, validate and code-quality unit tests pass (941).
  • Self-hosting: building packages/compiler/src/native/cli.ts with --emit=ir reports no diagnostics. tests/harness/self-hosting-native-driver.test.ts passed on the pre-merge tree.
  • The default runtime-pack objects (.o, all flavors) built from this branch are byte-identical to origin/main's; the only additions are the warning-replace and stdio-extended variant directories (and their -workers twins).
  • Zero-cost gate against origin/main 64e3ecd (benchmarks, the self-hosted compiler and the TypeScript corpus): PASS: all 2395 programs are byte-identical in IR, LLVM and release executables, with no remaining differences. Compile time: bench geomean −0.12% (CI −1.07%/+0.70%), self +0.28% (CI +0.20%/+0.46%), both within tolerance. The gate ran on the tree just before two last edits: normalizing the new link features to booleans in effectiveRuntimeFeatures (needed for self-hosting) and the matching test fixture. Those edits only touch variant selection and give the same answer for programs that do not set the features.

🤖 Generated with Claude Code

cramforce and others added 10 commits October 18, 2026 15:15
pi's CLI setup replaces process.emitWarning to silence warnings. The
assignment now stores the replacement: later process.emitWarning(...)
calls apply it to their arguments, and runtime-built warnings (Node's own
modules call process.emitWarning) reach it as (message, type[, code])
instead of the report and the 'warning' listeners. process.title writes
already lowered; the corpus program pins both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under @types/node, process.getBuiltinModule("node:os") types as the
module namespace, which lowered as an unchecked dynamic value and failed.
A string-literal id naming a supported builtin now lowers to the namespace
value an `import * as` binding has, including pi's cast-and-probe spelling
`(process as P).getBuiltinModule?.("node:os") ?? null`; a nullable
namespace binding narrowed present keys the same member tables, and a
namespace is truthy. process.features read as a whole value (`"sea" in
process.features`) is an object holding none of the flags, matching the
existing member reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pi-ai consumes every provider stream with `for await` over an EventStream
whose `async *[Symbol.asyncIterator](): AsyncIterator<T>` is an async
generator method; that was SC1070. The loop now opens the iterator with
the method call and drives the returned async generator like any other
(break and throw close it). An `async *` declaration annotated with the
protocol interfaces (AsyncIterator, AsyncIterableIterator, AsyncIterable)
maps like AsyncGenerator. Boxing a class instance whose fields hold a
promise now links the dynamic async unit (the materializer boxes the
promise), which EventStream's own `this.isComplete(event)` calls need.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Weak collections already lower to checked-dynamic storage as locals, but a
class field of those types was fenced as a dynamic-typed field (pi's
AgentSession keys entry ids by message in a WeakMap field). Declared,
optional, and constructor-assigned fields of those types now store the
native collection like the locals do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1309 let a checker-untyped receiver call a member named like a
String.prototype-only method (`bold`, `italics`, ...), refusing only a
string receiver at run time. TypeScript importing chalk's JavaScript
source types the default export as a plain function, so `chalk.bold` is
an `any` member on a typed receiver and still fenced; pi calls it in
nine places. A checker-untyped member now takes the same path. The chalk 5
fixture pins root, aliased, and Chalk-instance `bold` calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Node checks its builtins before node_modules for a bare core-module name,
so an installed npm package named string_decoder (a transitive dependency
of pi) never answers `import ... from "string_decoder"`. scriptc
attributed the import to the package and refused it. The npm resolution
now answers null for bare builtin names (prefix-only builtins such as
node:test keep their bare names for packages), for ES imports and CommonJS
requires alike.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three gaps on pi-tui's terminal start path:
- `if (process.stdin.setRawMode)` read the method as a value (SC2020).
  The read now answers the stream object's member: the method on a TTY,
  undefined otherwise, as on Node.
- process.stdin.setEncoding had no runtime method, and a string 'data'
  listener was fenced. stdin now decodes with the shared StringDecoder
  once per read (split multibyte sequences stay whole, end() flushes the
  tail before 'end'), string listeners receive the text, and dynamic
  listeners receive what Node delivers. A string listener with no
  encoding set throws the setEncoding-state TypeError, as child streams
  do.
- process.stdout.on("resize", cb) (once/off/removeListener, stderr too)
  registers on the stream object, which listens for SIGWINCH and emits
  when a TTY's size changed; the listener never keeps the loop alive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
quickjs-wasi's JSException constructor runs `delete this.stack` so V8's
own stack property stops shadowing the class's stack accessor; scriptc
fenced the delete, so every guest exception reaching the host threw the
fence error. Compiled errors keep the trace internally and class dispatch
already reads the accessor, so the delete now evaluates its receiver and
nothing else, for Error subclasses whose chain declares a stack getter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

# Conflicts:
#	packages/runtime/src/scr_async.h
The process.emitWarning replacement and the stdin encoding, string data
listener and stdio 'resize' support changed runtime functions every
program with the dyn-async or events unit links (the shared warning
paths, stdin delivery, the process stream object) and grew their
thread-local state. That code now compiles only into new runtime-pack
variants, SCR_WARNING_REPLACE of scr_async_dyn.c and SCR_STDIO_EXTENDED
of scr_events.c, which the link selects when the program assigns
process.emitWarning or reaches setEncoding, a string 'data' listener or
'resize'. The default objects are byte-identical to before, so other
executables do not change. The development and sanitizer builds pass the
same defines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
scriptc Ready Ready Preview, v0 Oct 19, 2026 9:48pm UTC

The fixture's class was rejected through a WeakMap-typed field, which
this branch makes legal. A MapIterator-typed field is still refused, so
the class stays rejected and the test still checks that a class reached
only through a receiver probe reports its own diagnostic exactly once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JSON.stringify of the runtime-features record follows the declared field
order in the self-hosted compiler and the literal's insertion order under
Node. warningReplace and stdioExtended were declared before processEvents
but built after it, so the two compilers serialized the record differently
(self-hosting-runtime-features). The interface now lists them in the
literal's order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ctate
ctate merged commit 4770b3f into main Oct 20, 2026
73 of 74 checks passed
@ctate
ctate deleted the opt/pi-small-fixes branch October 20, 2026 00:50

This branch was successfully deployed

1 active deployment
Preview — c7329845 Deployed Oct 19, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants