Skip to content

Check narrowed member reads that a call can invalidate - #846

Open
cramforce wants to merge 9 commits into
mainfrom
fix/narrowed-property-calls
Open

cramforce wants to merge 9 commits into
mainfrom
fix/narrowed-property-calls

Conversation

@cramforce

Copy link
Copy Markdown
Contributor

Problem

TypeScript keeps the narrowing of a property reference (c.cb, this.item, o.shape) across calls, and of a let that a closure assigns. After c.cb = fn or if (c.name !== undefined), a callback, method or getter that clears or replaces the property leaves later reads typed by the old value. scriptc trusted that type:

  • maybeNarrow bridged the union read with a tag-unchecked unionNarrow;
  • ?., truthiness, typeof and equality folded by the narrowed type.

So a second c.cb?.() after a self-clearing callback threw undefined is not representable in the target union where Node returns undefined. Reads of string | undefined, Item | null, string | number, interface and discriminated-union fields after such a call segfaulted or reinterpreted memory (typeof c.v printed number 2.1e-314 after the field became a string). The same held for a captured let cleared by a closure.

Fix

NarrowingStability (new, frontend/lowering/narrowing-stability.ts) decides whether a narrowing is provably current at the read:

  • every property in the reference is construction-only and the root is stable (this, or a binding no other function assigns). Construction-only means no store to the name anywhere except this.name = ... directly in a constructor body, no accessor of that name, and no computed or reflective store into class instances. This is the same conservative principle as backend/cycle-mutability.ts, applied to source because lowering runs before that IR analysis exists; or
  • nothing between the earliest occurrence of the reference in the use's function and the use can run other code or store the property through another reference. Calls, new, tagged templates, await/yield, user accessors, user iterators and user conversion hooks count; standard library calls without function arguments (Math, console, string, array and Map/Set methods) don't. A loop around the use that starts inside that span contributes its whole body. TypeScript never carries a member narrowing into a nested function, and every narrowing is an occurrence of the reference, so this span covers every narrowing path.

Proven reads keep the unchecked extraction. Otherwise the read observes the stored value:

  • In a ?. receiver, an absence/type/truthiness test (??, ||, &&, !, ===, typeof, instanceof, conditions, template spans), a member read the declared type supports (a discriminant on every arm, or a method of the declared class), and a slot that accepts the declared type (arguments, returns, annotated initializers, assignments), the reference reads as declared, so the consumer tests at runtime exactly as Node does.
  • Everywhere else the extraction is checked and throws Node's TypeError for the consuming operation: Cannot read properties of undefined (reading 'x'), Cannot read properties of null (reading 'x'), o.f is not a function. Calls through callback fields use the same message.
  • A restored ?. receiver whose only nullish arm is null short-circuits to undefined, and typeof evaluates an effectful union operand (a getter) once.

The checked failure path groups arms by message (Node's message for null/undefined, one shared stranded-value message for every other arm), so wide unions such as IrExpr don't grow the IR. The first version emitted one throw per arm and pushed the self-hosted compiler's IR past V8's maximum string length.

Remaining divergences (inherent to static representation)

  • A stale value stored into a slot typed by the stale narrowing (const s = o.name without annotation, [o.name], f(o.name) with f(s: string)) throws the stranded-value TypeError where Node carries undefined on.
  • A value replaced by another non-nullish arm and then used through a member the narrowed type has but the declared type does not (c.v.toFixed() after c.v became a string) throws scriptc's TypeError rather than Node's c.v.toFixed is not a function.

Evidence

  • Corpus 4650-stale-property-narrowing.ts: optional calls through self-clearing callbacks (top level and in a function), direct calls after the clear, ?./??/||/===/typeof/!/conditions/template/value reads after a clearing method, null arms, string | number replaced by its other arm, interface fields cleared by a free function, a getter as the intervening call and a narrowed getter, callbacks run by forEach, a loop back edge, an aliased store without a call, a discriminated field replaced by another arm, an instanceof subclass replaced by its base, closure-assigned bindings as values and as member roots, and narrowings that stay valid (construction-only fields across a user call, library calls only). It fails on main at its first line and matches Node byte-for-byte with the fix.
  • narrowing-stability.test.ts: construction-only fields across a user call, mutable fields with no call and with only library calls keep the unchecked extraction; user calls, method calls and loop back edges get the checked read with Node's message.
  • tsc-ts self-check (origin/parallel-check e469e33), Linux, base and candidate built and run interleaved in one VM, diagnostics identical: single-threaded check time median 4.663 s → 4.642 s (−0.45% pooled over 27 runs each, two runs with opposite order: −1.5%, +0.6%); --checkers 8 wall 1.280 s → 1.265 s. Binary +17.5 KB (+0.07%). The self-hosted compiler's IR grows 0.03%.

TypeScript keeps the narrowing of a property reference (and of a binding
that a closure assigns) across calls. After `c.cb = fn` or
`if (c.name !== undefined)`, a callback, method or getter that clears or
replaces the property leaves later reads typed by the old value. Lowering
trusted that type: maybeNarrow extracted the union payload unchecked, and
`?.`, truthiness and `typeof` folded by the narrowed type. A second
`c.cb?.()` after a self-clearing callback threw where Node answers
undefined, and `string | undefined`, `string | number` or record fields
read after such a call crashed or reinterpreted memory.

NarrowingStability decides whether a narrowing is provably current: every
property in the reference is construction-only (no store to its name
outside `this.name = ...` in a constructor body, no accessor, no computed
or reflective store into instances) with a stable root, or nothing between
the earliest occurrence of the reference in the use's function and the use
can run other code or store the property through another reference
(standard library calls without callbacks are inert; a loop around the use
contributes its whole body). Proven reads keep the unchecked extraction.

Otherwise the read observes the stored value. In `?.` receivers, absence,
type and truthiness tests, member reads the declared type supports, and
slots that accept the declared type, the reference reads as declared, so
the consumer tests at runtime exactly as Node does. Everywhere else the
extraction is checked and throws Node's TypeError for the consuming
operation (`Cannot read properties of undefined (reading 'x')`,
`o.f is not a function`). Calls through callback fields use the same
message. A null-only receiver of a restored `?.` short-circuits to
undefined, and `typeof` evaluates an effectful union operand once.

The checked failure path groups arms by message so wide unions stay small:
the self-hosted compiler's IR grows by 0.03%. tsc-ts self-check, Linux,
interleaved in one VM: check time -1.5% (noise), `--checkers 8` -1.2%.

Corpus 4650 pins the shapes; a unit test pins which reads stay unchecked.
@vercel

vercel Bot commented Oct 11, 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 12, 2026 2:38am UTC

An assignment narrows a binding to the assigned arm, and TypeScript keeps
that narrowing across a closure or method that stores another arm. After
`if (acc !== null) return acc;` the checker still types the read as the
old arm, the return slot rejects the declared `T | null`, and the checked
extraction threw on the value the closure stored, where Node returns it.
The compiler itself has this shape (lowerDynObjectLiteral), so the native
compiler aborted with "a value of another type is not representable in
the target union" and closure-nullable-union-return.ts diverged.

A slot that accepts the declared type minus null/undefined now reads the
stale reference as NonNullable<declared>, like a binding narrowed only by
the absence test. Elements and properties of array and object literals
are such slots when the literal itself sits in a typed slot (annotated
declaration, argument, assignment, return of a function with a declared
return type). Untyped literals keep the checked extraction.
A stale narrowing read in a call argument whose parameter accepts the declared type read as declared, so the callee's lowering tests the stored value. For standard-library callees that lowering is often specialized to the argument types programs pass: after `combo.p = {...}; combo()` the argument of `JSON.stringify(combo.p)` became `T | undefined` (SC1090), and after `ci(slow)` the handle of `timers.clearInterval(fast)` became `number | null` (SC2020), so 2072-expando-function-members.ts and 1804-timers-module.ts stopped compiling.

Call and construction arguments (directly or inside literal arguments) now read as declared only where the callee lowers any value of the parameter type: user callees, console methods, and standard-library parameters typed by a type parameter (`push(...items: T[])`, `Map.set`). Other standard-library arguments keep the checker's narrowing and the checked extraction.
# Conflicts:
#	packages/compiler/test/ts7/baselines/order-parity.json
# Conflicts:
#	packages/compiler/test/ts7/baselines/order-parity.json
# Conflicts:
#	packages/compiler/test/ts7/baselines/order-parity.json
# Conflicts:
#	packages/compiler/test/ts7/baselines/order-parity.json

This branch was successfully deployed

1 active deployment
Preview — aa4fa790 Deployed Oct 12, 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