Fix ~30 more crashes across §5.1/§5.2 of the not-implemented audit - #304
Merged
Conversation
Closes the named/specific marker list (§5.1) and every generic TypeSwitch::Default fallback this audit had inventoried (§5.2) across 16 files. Real, confirmed crashes fixed include: array widening to any[], class/interface heritage clauses naming the wrong kind of type, accessor ++/--, generic zero-arg type inference failure, destructuring assignment gaps (array-from-string, object rest-spread), spreading a malformed custom iterator, non-string computed interface method names, Foo[string] indexed access, and a getNameWithArguments UB crash (return nullptr as std::string). Also fixes a genuine missing NamespaceType case in MLIRTypeIterator.h, found by diffing against TypeScriptTypes.td's type list rather than a repro. Full per-site real/dead/unconfirmed breakdown in docs/not-implemented-audit.md (§4.12-§4.13). 829/829 tests passing, verified after every file's fixes throughout both passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both share one root cause: TupleFieldName() (MLIRGenTypes.cpp) returns a
null mlir::Attribute in two unguarded cases - a computed field name that
fails to extract a compile-time constant, and a BindingPattern name
falling through to an invalid name.as<Expression>() cast. Both manifest
as an unguarded dyn_cast/cast<StringAttr> crashing on the null result
downstream ("dyn_cast on a non-existent value").
Fixed with null-checks at 4 call sites in MLIRGenClasses.cpp (computed
class-field names, e.g. `class X { [key] = 42; }`) and a BindingPattern
guard directly in TupleFieldName itself (parameter properties combined
with destructuring, e.g. `constructor(public {x,y}: T)`), fixing the
root function once for every caller.
829/829 tests passing.
Co-Authored-By: Claude Sonnet 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
1 participant
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.
Summary
Continues the ongoing
llvm_unreachable("not implemented")crash audit (seedocs/not-implemented-audit.md). This PR closes:MLIRGenImpl.h's last 2 sites,MLIRGenInterfaces.cpp's remaining 2, all 10 ofMLIRGenTypes.cpp,LLVMCodeHelper.h:452)TypeSwitch::Default/switch fallback this audit had inventoried, across 12 files (CastLogicHelper.h,MLIRGenClasses.cpp,MLIRGenExpressions.cpp,MLIRGenGenerics.cpp,MLIRGenImpl.h's remaining cluster,CodeLogicHelper.h,OptionalLogicHelper.h,UndefLogicHelper.h,MLIRCodeLogic.h,MLIRPrinter.h,MLIRTypeIterator.h,Win32ExceptionPass.cpp)Real, confirmed crashes fixed
number[]to anany[]parameterclass extends <interface>/class implements <type-alias>(naming the wrong kind of type in a heritage clause)obj.accessor++/--on a get/set accessor property (the oldest documented gap in the audit — its own// TODO: finish it for field accesspredates this whole effort)foo<T>(): Tcalled asfoo()with nothing to inferTfrom[a, b] = "hi") and object rest-destructuring (({a, ...rest} = obj))next()isn't a well-formed{value, done}shapeFoo[string]indexed access, type-literal get/set accessorsgetNameWithArgumentsUB crash (return nullptr;constructing astd::string, found via WinDbg while chasing an unrelated repro)Notable finds
TypeSwitchloop, so the first attempt'semitErrorwas silently swallowed until asuccessflag was added.MLIRTypeIterator.h(this compiler's most widely-called recursive type-walker) was missing aNamespaceTypecase entirely — found by diffing againstTypeScriptTypes.td's type list rather than reproducing a crash.Casting.hassertion reachable via a computed class-property initializer, and a delete-expression crash that reproduced once and then never again.Full per-site real/dead/unconfirmed breakdown is in
docs/not-implemented-audit.md(§4.12-§4.13).Test plan
ctest -C Debug -j8→ 829/829 passing, re-verified after every file's fixes throughout both passes (not just once at the end)🤖 Generated with Claude Code