Fix llvm_unreachable aborts resolving unknown field/method names in getFieldTypeByFieldName - #238
Merged
Merged
Conversation
…ay types getFieldTypeByFieldName's ArrayType case only special-cased "length" and the synthetic index-access field names, then hit llvm_unreachable for any other field name -- including legitimate extension-method names like "push"/"pop"/"entries" on the array. This aborts whenever --export=all forces eager instantiation of a generic class whose body calls array extension methods (e.g. the default lib's Array<T>/TypedArray<T>, which Uint8Array/Int32Array alias), as seen when compiling a .d.ts with `declare function` signatures that reference typed arrays (#231). Return an empty type instead, matching the "field not found" convention already used by the tuple/const-tuple cases in the same function, whose callers (the `in` operator check and extendsType) already treat a null type as "not present" correctly.
Same defect as the ArrayType case fixed in the previous commit, present in four more branches of the same function: - InterfaceType/ClassType: aborted when a string field name matched neither a field nor a method, instead of returning "not found" so the caller can keep looking (e.g. via extension functions). - ConstArrayType/StringType: only special-cased "length" (and, for ConstArrayType, the synthetic index-access field), then aborted for any other field/method name -- e.g. String.fromCharCode. All five branches now return an empty type for an unresolved field name, matching the convention already used by the ConstTupleType/TupleType branches earlier in the same function. This matters in particular for the UnionType branch (right below), which calls this function per member and treats a falsy result as "this member doesn't have the field" -- before this fix, a union containing a class/interface/array/string member would abort the process instead.
ASDAlexander77
changed the title
The ClassType branch called getInterfaceInfoByFullName() with a class's
full name instead of getClassInfoByFullName(), so it was looking up
class field/method info in the wrong registry. Since classes and
interfaces are registered in separate tables, this lookup essentially
never succeeded for a real class, silently returning "field not found"
for every field/method access on a class type -- affecting the `in`
operator on class instances and extendsType's structural matching of
classes against tuple-shaped constraints (e.g. `T extends { length:
number }`).
ClassInfo's findField/findMethod have a different signature than
InterfaceInfo's (findField takes a bool& out-param and returns a value
rather than a pointer), so the fix updates the call site accordingly
rather than just swapping the lookup function.
Verified against the full tester/tests suite (365 files): no crashes,
no new failures compared to the pre-fix baseline.
ASDAlexander77
added a commit
that referenced
this pull request
Jul 17, 2026
…rage (#241) * Fix in-operator crash and literal-string field lookup; add regression tests - mlirGenInLogic assumed any `in` expression whose right side has `.length` was a numeric-index check, so a string-literal left side (e.g. "length" in arr) got cast to an index type and crashed LLVM translation. Now falls through to the general field-lookup path for string literals. - getFieldTypeByFieldName didn't strip LiteralType wrappers, so `in` checks against const strings/string literals (e.g. "length" in "hi") incorrectly resolved to false instead of using the underlying type's field lookup. - Add 00in_method_names.ts and 00class_structural_extends.ts covering the #238 area (in-operator and structural extends against classes), wired into both test-compile and test-jit CMake targets. Verified against the full 696-test suite, no regressions. - Document a separate, deferred bug in docs/bugs/: generators lose their state across manual .next() calls (only for...of works). Root-caused to const bindings lacking backing storage; a first fix attempt broke unrelated interface/symbol tests and was reverted, so the fix itself is left for a dedicated follow-up. * Enhance boolean arithmetic and comparison operations; add regression tests for coercion behavior * Fix === strict-equality coercion and any==any loose-equality coercion === / !== previously shared codegen with ==/!=, so mismatched primitive kinds (1 === true) were coerced to a common type and wrongly compared equal. adjustTypesForBinaryOp now short-circuits to a constant when both operand types are unambiguous, differing primitives. AnyCompareOp's ==/!= lowering did a raw memcmp of the boxed payload bytes, never applying JS loose-equality coercion across differing any payload kinds (number<->string, boolean<->number, boolean<->string). It now compares the boxed type tags first and coerces via the existing cast helpers when they differ, accounting for the fact that boxed integer literals carry concrete-width tags (s32/s64) rather than "number". Adds 00mixed_type_ops.ts covering cross-type binary op coercion. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Make conditionalExpressionLowering nesting-safe; fix stale 01tuple.ts assertion conditionalExpressionLowering (CodeLogicHelper.h) branched into its result block from the then/else block handles captured before invoking the builder callbacks, assuming the insertion point never moved. That broke as soon as a callback itself called the helper again (needed for AnyCompareOp's new multi-way coercion dispatch), producing an "operation with block successors must terminate its parent block" verifier error. It now re-reads the actual insertion block after each builder runs, so it composes safely when nested. AnyCompareOpLowering's local workaround (nestableConditional) is removed in favor of the shared, now-fixed helper. 01tuple.ts's `assert(obj8.field1 === 10)` relied on the old, buggy === semantics that coerced like == (fixed in the previous commit) -- field1 is string-typed and holds the coerced "10", so strict equality against the number 10 is correctly false. Updated to match the coercion pattern already used elsewhere in the same file. Full suite: 700/700 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- 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
--export=alldeclaration-export walker, reproducing the reporter'sBrowserLibproject against the real default lib (TypeScriptCompilerDefaultLib) surfaced further crashes inMLIRTypeHelper::getFieldTypeByFieldName, and while fixing those, a separate wrong-lookup bug in the same function.Crashes (commits 1-2)
Uint8Array/Int32Arrayresolve through the default lib astype Uint8Array = TypedArray<u8>, a generic class built onArray<T>.--export=allforces eager full-body instantiation of referenced types, which walks every method ofArray<T>/TypedArray<T>-- including ones likepush/pop/entriesthat call through tothis.data.<method>(), an extension-method call on the raw array type.getFieldTypeByFieldNamehad the same defect in five branches: it special-cased a couple of known field names, then calledllvm_unreachable("not implemented")for anything else -- including legitimate method names -- instead of returning "not found" so the caller can keep looking (e.g. via extension functions). Fixed all five (ArrayType,ConstArrayType,StringType,InterfaceType,ClassType).UnionTypebranch in the same function, which callsgetFieldTypeByFieldNameper member and expects a falsy result for "member doesn't have this field" -- before this fix, a union containing an affected member type would abort the whole process instead of just excluding that member.Wrong lookup table (commit 3)
ClassTypebranch, found it calledgetInterfaceInfoByFullName()with a class's full name instead ofgetClassInfoByFullName()-- looking up class field/method info in the wrong registry. Since classes and interfaces are registered in separate tables, this essentially never succeeded for a real class, silently returning "field not found" for every field/method access on a class type. This affects theinoperator on class instances andextendsType's structural matching of classes against tuple-shaped constraints (e.g.T extends { length: number }).ClassInfo::findField/findMethodhave a different signature thanInterfaceInfo's (findFieldtakes abool&out-param and returns a value rather than a pointer), so the call site needed updating to match, not just the lookup function swap.Test plan
tslang(release) with all three commits.native.d.ts(from the issue's attachedBrowserLib.zip) with--export=allagainst the real default lib -- previously aborted withUNREACHABLE executed at .../MLIRTypeHelper.h:2375, now compiles cleanly (exit 0)..ts/.d.tsfiles from the reporter'sBrowserLib/runtime/object/Windowdirectory individually with--export=all-- no crashes on any file; remaining failures are ordinary unresolved-symbol errors from files needing siblingimports not present when compiled standalone (expected).test/tester/testssuite (365.tsfiles) through the fixed compiler (parse + MLIR codegen): zero crashes, zero new failures vs. the pre-fix baseline binary (the 3 pre-existing failures fail identically on both, for unrelated reasons -- missing sibling files / pre-existing issues).🤖 Generated with Claude Code