Repository navigation
A class's v?: T is T | undefined, directly and through an interface - #433
Merged
Merged
Conversation
`class C { v?: string }` stored a plain string, so an unassigned `c.v` read as null
(0 for a number) and `c.v === undefined` was false; the same for a constructor
parameter field `constructor(public m?: string)`. Both are now optional<T>, as a
type literal's `v?: T` has been since #427.
An interface's `v?: T` reads a field at an offset kept in the vtable, with the
interface's T. A class can't have its vtable patched per object as a type literal
does (an instance is shared, not copied), so its interface vtable marks such a slot
with INTERFACE_OPTIONAL_STORAGE_TAG on top of the offset. InterfaceSymbolRefOpLowering
then reads the field's flag: an unset one is undefined. A write through the interface
(`i.v = x`) sets the flag, and `i.v = undefined` clears it: mlirGenSaveLogicOneItem
marks the access it stores to with INTERFACE_OPTIONAL_WRITE_ATTR.
InterfaceSymbolRefOp reads memory now (MemoryEffects<[MemRead]>, was Pure): a read of
the flag before a write must not stand for one after it (CSE merged them, and LICM
could hoist one out of a loop).
DefaultLib's declarations must match its build: its RegExp classes declared fields
`?` that the implementation stores plain (DefaultLib PR), and Error's `message?` now
matches its `public message?` constructor fields.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
class C { v?: string }stored a plainstring. An unassignedc.vread as null (0 for a number), andc.v === undefinedwas false. The same was true for a constructor parameter field,constructor(public m?: string), sonew Error().message === undefinedwas false.Type literals were fixed in #427, but classes were left out. An interface's
v?: Treads the field at an offset stored in the vtable, using the interface's type. The type-literal fix patches a copy of the vtable for each object, which works because the object is copied. A class instance is shared, so that approach doesn't apply.Fix
v?: T, and apublic m?: Tconstructor parameter field, is stored asoptional<T>, i.e.{T, flag}with the value first.v?: Tacceptsoptional<T>storage. The class's interface vtable marks that slot withINTERFACE_OPTIONAL_STORAGE_TAGadded to the offset.InterfaceSymbolRefOpLowering):undefined;= undefinedclears it.mlirGenSaveLogicOneItemmarks the access a store goes to withINTERFACE_OPTIONAL_WRITE_ATTR.InterfaceSymbolRefOpnow reads memory (MemoryEffects<[MemRead]>, previouslyPure). A flag read before a write must not stand in for one after it: CSE merged them, and LICM could hoist one out of a loop.Needs DefaultLib #20
DefaultLib's declarations have to match its compiled library. Its RegExp classes declare
?fields that the implementation stores plain; DefaultLib #20 drops those?.Error'smessage?now matches itspublic message?constructor fields. A DefaultLib built before this PR has the old layouts, so it needs a rebuild:I:\tslang, which your environment pointsDEFAULT_LIB_PATHat, will failtest-compile-gc-defaultlib-collectoruntil it's rebuilt.Tests
New
00class_optional_fields.ts, run as compile and jit. It covers:undefinedthrough an interface;--opt;It passes under gc, rc and none, with and without
--opt.Release suite: 3351/3351, against a DefaultLib built from DefaultLib [Question] Speed difference #20 with this compiler.
That DefaultLib: 159/159 compiled and 159/159 under JIT (release).
rc: a 2M-iteration loop that writes and clears
?fields directly and through an interface gives the right result. Its memory matches the same loop without?fields (226 MB vs 235 MB). That loop grows under rc either way, and the growth isn't from this change.Not run: Debug builds, x86,
-mm=own.🤖 Generated with Claude Code