From 78d5afd348c3f7d49a6c60411c4702205734b98d Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Fri, 24 Jul 2026 00:13:06 +0100 Subject: [PATCH] Fix 2 more crashes: typeof-dispatch cast helpers in MLIRGenCast.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit castPrimitiveTypeFromAny (the __unbox helper, generic type-param unboxing from any) was dead code - its one call site already only passes types the TypeSwitch handles - but fixed anyway (crash -> emitError) for consistency, since location was already in scope. castFromUnion is a real, easily-reachable crash: any union type with a tuple/object-literal-shaped member (e.g. `number | {a: number}`) crashes the moment it needs a runtime cast, since the typeof-dispatch TypeSwitch never got a TupleType/ConstTupleType case. The function's own forward-decl already carries a TODO acknowledging typeof-based dispatch can't handle this properly (can't even distinguish two different tuple shapes from each other) - a real redesign, out of scope here. Converted the crash to a clean error instead. Closes out MLIRGenCast.cpp's two TypeOf sites, the last item from the original not-implemented-audit's §5.1 named/specific list. 829/829 ctest, no regressions. --- tslang/docs/not-implemented-audit.md | 94 +++++++++++++++++++++++---- tslang/lib/TypeScript/MLIRGenCast.cpp | 30 +++++++-- 2 files changed, 106 insertions(+), 18 deletions(-) diff --git a/tslang/docs/not-implemented-audit.md b/tslang/docs/not-implemented-audit.md index 10f39dce0..69e65b3b9 100644 --- a/tslang/docs/not-implemented-audit.md +++ b/tslang/docs/not-implemented-audit.md @@ -1,13 +1,14 @@ # `llvm_unreachable("not implemented")` audit -Status: **8 confirmed crashes fixed across three passes, ~112 markers still +Status: **9 confirmed crashes fixed across four passes, ~110 markers still uninvestigated** — written as a roadmap for continuing this audit, not a claim that the sweep is complete. Triggered by a user request to review every "not implemented" marker in the codebase and see which ones can be implemented. Second pass (§4.3-4.5) worked through this document's own §6 priority list while waiting on the first pass's PR to merge. Third pass (§4.6-4.8) closed out §5.4's `MLIRTypeHelper.h` `funcRef` family after that -PR merged. +PR merged. Fourth pass (§4.9) closed out `MLIRGenCast.cpp`'s two `TypeOf` +sites, the last item left from the original §5.1 named/specific list. ## 1. Scope and method @@ -335,6 +336,69 @@ can't be found" instead of crashing) and via the full suite (`ctest -C Debug existing tests exercising these utility types with a real function argument (`test/tester/tests/00types_utility.ts`, `01types_utility.ts`) still pass. +### 4.9 `MLIRGenCast.cpp`'s two `TypeOf` sites — one dead, one a real, easily-reachable crash + +Both are `.Default` branches of a `TypeSwitch` that builds up a synthetic +`typeof t == '...'` dispatch function as TS source text, then parses and +calls it - the compiler's mechanism for runtime type discrimination when +casting away from a type whose concrete shape isn't known until runtime +(`any`, or a union that can't be merged into one storage type). + +**`castPrimitiveTypeFromAny` (was :1320-1322, the `__unbox` helper for +generic type-parameter unboxing from `any`, as guessed in the original +§5.1 entry): dead code.** Its one call site (`MLIRGenCast.cpp:1140`, inside +`castFromSourceSpecialCases`-family cast dispatch) only invokes it when +`type` (the cast destination) is one of `{NumberType, BooleanType, +StringType, BigIntType, IntegerType, FloatType, ClassType}` - a strict +subset of what the `TypeSwitch` inside already handles (`{Boolean, +TypePredicate, Number, String, Char, Integer, Float, Index, BigInt, +Function×4, Class, Interface, Null, Undefined}`). Same "guarded, therefore +dead" shape as §3's `IntersectionType` and §4.6-4.8's `funcRef` family. +Fixed anyway (crash → set a flag, `emitError` + `return failure()` after +the switch) since `location` was already in scope here and leaving a live +`llvm_unreachable` behind is a landmine for the next caller. + +**`castFromUnion` (was :1498-1499): a real, easily-reachable crash.** Called +from `castFromSourceSpecialCases` whenever casting *from* a union-typed +value whose members can't be merged into one storage representation +(`mth.isUnionTypeNeedsTag`) to anything other than `any`. It loops over +each union member type building the same kind of `typeof`-dispatch +function, and the `TypeSwitch` per member is missing `TupleType`/ +`ConstTupleType` entirely - i.e. **any union with an object-literal-shaped +member hits this the moment it needs a runtime cast**, which is a very +ordinary shape (not an obscure corner case like §4.4's enum reverse-mapping +or §4.5's `super()` edge case): + +```ts +function main() { + let x: number | { a: number }; + x = 5; + let y = x; // crash: UNREACHABLE at MLIRGenCast.cpp:1499 +} +``` + +The function's own forward declaration in `MLIRGenImpl.h` already carries a +`// TODO: remove using typeof for Union types as it can't handle types such +as 2 tuples in union etc` - confirming this is a known, **genuinely missing +feature** (like §4.2), not just an unreached architectural corner: even two +*different* tuple-shaped union members couldn't be told apart by `typeof` +alone (both report `"object"`), so a real fix needs a structural redesign +(a runtime shape tag, not `typeof` string dispatch), out of scope here. A +partial start at this is visible in the code - a `tupleTypes` +`SmallVector` and `TYPE_TUPLE_ALIAS` templating exist and are wired up at +the end of the function, but nothing ever pushes into `tupleTypes` because +no `.Case` was ever added to populate it; that +half-finished thread was left as-is rather than completed, since finishing +it properly means solving the "2 tuples in union" ambiguity the TODO +already flags, not just adding one more `.Case`. Converted the crash to +`emitError(location) << "Cast from " << to_print(value.getType()) << " to " +<< to_print(type) << " is not supported"; return mlir::failure();` after the +member loop, gated by the same kind of flag used for `castPrimitiveTypeFromAny` +(a per-subtype lambda can't `return` the enclosing function directly). + +Verified individually (clean diagnostic, no crash) and via the full suite +(`ctest -C Debug -j8`: 829/829, no regressions). + ## 5. Inventory of remaining markers (untested this pass) Grouped by file. "Shape" is a guess from reading the surrounding code, not a @@ -343,12 +407,12 @@ verified verdict — see §2 for how to actually check one. ### 5.1 Named/specific (cheapest to investigate next — read the message + local branch, write a 5-line repro) **Fixed this pass**: `MLIRGenAccessCall.cpp`'s three sites (was lines -1159/1219/1535) — see §4.3-4.5. +1159/1219/1535) — see §4.3-4.5. **Fixed a previous pass** (§4.9): +`MLIRGenCast.cpp`'s two `TypeOf` sites (was lines 1321-1322/1498-1499) — one +dead (guarded), one a real crash (union with a tuple-shaped member). | Site | Message | Shape (unverified guess) | | --- | --- | --- | -| `MLIRGenCast.cpp:1321-1322` | TypeOf NOT IMPLEMENTED for Type | inside a generated `__unbox` helper (generic type-parameter unboxing from `any`); `.Default` for a type kind not in its explicit list (Tuple/Array/Enum/Union/Optional are plausible candidates) | -| `MLIRGenCast.cpp:1498-1499` | TypeOf NOT IMPLEMENTED for Type | second, near-identical site — check if it's reachable via a different call path than 1321 | | `MLIRGenImpl.h:5330` | not implemented | unread | | `MLIRGenImpl.h:6732` | not implemented | unread | | `MLIRGenImpl.h:7314` | not implemented | unread | @@ -424,17 +488,23 @@ bug (the built-in utility types); tracing real callers is what worked. (faster than the originally-planned unit-test approach), found and fixed 3 more live crashes (§4.6-4.8); the other 3 functions in the family were fixed too even though proven dead, for consistency within the family. -4. §5.2 (generic fallbacks) — triage a handful against existing passing +4. ~~`MLIRGenCast.cpp`'s two `TypeOf` sites~~ — done this pass (§4.9): one + dead (guarded), one a real crash (union with a tuple-shaped member, + `x` where `x: number | {a: number}`) — fixed. That was the last + item from the original §5.1 named/specific list; only the large + `MLIRGenImpl.h`/`MLIRGenInterfaces.cpp`/`MLIRGenTypes.cpp` cluster remains + from §5.1, plus the stray + `MLIRTypeHelper.h:410/420/2108/2256-2257/2290/2307/2685/2709` sites + (confirmed *not* part of the `funcRef` family, see §5.4). +5. The `MLIRGenImpl.h`/`MLIRGenInterfaces.cpp`/`MLIRGenTypes.cpp` cluster + (§5.1's last remaining block) — read-and-repro each per §2's recipe, same + as every other §5.1 item so far. +6. §5.2 (generic fallbacks) — triage a handful against existing passing tests using §3's method before assuming any individual one is live. -5. §5.3 (RTTI) — lowest priority from this (Windows) machine; the Linux +7. §5.3 (RTTI) — lowest priority from this (Windows) machine; the Linux variants need a WSL/Linux build to exercise at all, and even the Windows ones are deep in a code path (RTTI/exception typeinfo generation) that's hard to reach without a specific class-hierarchy-plus-exception scenario. -6. `MLIRGenCast.cpp`'s two `TypeOf` sites, the large `MLIRGenImpl.h`/ - `MLIRGenInterfaces.cpp`/`MLIRGenTypes.cpp` cluster, and the stray - `MLIRTypeHelper.h:410/420/2108/2256-2257/2290/2307/2685/2709` sites - (confirmed *not* part of the `funcRef` family, see §5.4) all remain - unread from the original §5.1 list. ## 7. Non-goals / out of scope diff --git a/tslang/lib/TypeScript/MLIRGenCast.cpp b/tslang/lib/TypeScript/MLIRGenCast.cpp index 43cb19960..3daae95cd 100644 --- a/tslang/lib/TypeScript/MLIRGenCast.cpp +++ b/tslang/lib/TypeScript/MLIRGenCast.cpp @@ -1293,6 +1293,7 @@ namespace mlirgen SmallVector classInstances; ss << S("function __unbox(a: any) : T {\n"); auto subType = type; + auto hasUnsupportedType = false; mlir::TypeSwitch(subType) .Case([&](auto _) { typeOfs["boolean"] = true; }) .Case([&](auto _) { typeOfs["boolean"] = true; }) @@ -1317,10 +1318,16 @@ namespace mlirgen // review code to use null in "TypeGuard" .Case([&](auto _) { /* TODO: uncomment when finish with TypeGuard and null */ /*typeOfs["null"] = true;*/ }) .Case([&](auto _) { /* TODO: I don't think we need any code here */ /*typeOfs["undefined"] = true;*/ }) - .Default([&](auto type) { + .Default([&](auto type) { LLVM_DEBUG(llvm::dbgs() << "\n\t TypeOf NOT IMPLEMENTED for Type: " << type << "\n";); - llvm_unreachable("not implemented yet"); - }); + hasUnsupportedType = true; + }); + + if (hasUnsupportedType) + { + emitError(location) << "Cast from 'any' to " << to_print(type) << " is not supported"; + return mlir::failure(); + } auto next = false; for (auto& pair : typeOfs) @@ -1457,6 +1464,7 @@ namespace mlirgen StringMap typeOfs; SmallVector classInstances; SmallVector tupleTypes; + auto hasUnsupportedType = false; ss << S("function __cast(t: T) : U {\n"); for (auto subType : normalizedUnion.getTypes()) { @@ -1494,10 +1502,20 @@ namespace mlirgen .Case([&](auto _) { typeOfs["object"] = true; }) .Case([&](auto _) { typeOfs["null"] = true; }) .Case([&](auto _) { typeOfs["undefined"] = false; }) - .Default([&](auto type) { + .Default([&](auto type) { LLVM_DEBUG(llvm::dbgs() << "\n\t TypeOf NOT IMPLEMENTED for Type: " << type << "\n";); - llvm_unreachable("not implemented yet"); - }); + hasUnsupportedType = true; + }); + } + + if (hasUnsupportedType) + { + // e.g. a tuple/object-literal-shaped member of the union - see the + // "must be improved"/"can't handle types such as 2 tuples in union" + // TODO on castFromUnion's declaration; typeof-based dispatch can't + // distinguish these today, that's a separate, larger redesign. + emitError(location) << "Cast from " << to_print(value.getType()) << " to " << to_print(type) << " is not supported"; + return mlir::failure(); } if (isNullDest)