Repository navigation
Comparing a union's field with null or undefined no longer crashes - #376
Merged
Merged
Conversation
The discriminated-union narrowing in checkSafeCastPropertyAccessLogic treated every const value as a ConstantOp. isConstValue is also true for null and undefined, so `s.r !== null` on a union-typed `s` asserted in debug builds and dereferenced a null op in release. null and undefined cannot be discriminant literals, so the narrowing now skips them. A field missing from one member of the union no longer indexes the tuple with -1 or dereferences a null FieldInfo. Found in the BrowserLib sources attached to #231. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 27, 2026
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.
Comparing a field of a union-typed object with
nullorundefinedcrashed the compiler. The debug build asserts, and the release build fails with an access violation. Found in the BrowserLib sources attached to #231.Cause
checkSafeCastPropertyAccessLogicnarrows discriminated unions such ass.kind === "circle". It checks that the compared value is a constant withisConstValue, then assumes that value is aConstantOp. ButisConstValueis also true forNullOpandUndefOp, soconstantOpwas null andgetValueAttr()dereferenced it.Fix
ConstantOp.nullandundefinedare never discriminant literals, so skipping them loses nothing.-1or dereferences a nullFieldInfo.Tests
00safe_cast_null_field.ts(compile, jit, and the rc/none corpus). It compares an interface union's field withnulland withundefined, and checks thats.kind === "circle"still narrows. The old build fails it with 0xC0000005.Not in this PR
These are the other #231 findings, left out on purpose:
elsebranch leaks past theif(a separate crash): next PR.type S = {kind:"a"} | {kind:"b"}) fails verification with'ts.PropertyRef' op operand #0 must be …, even with nonullinvolved. It fails the same way before this change.N | null(h.next !== null) now gets that same verification error instead of crashing. TypeScript in strict mode rejects it too ("possibly null").🤖 Generated with Claude Code