Skip to content

Commit 920ec13

Browse files
Partial hardening for discovery-pass tolerance during nested object-literal method resolution
Investigates the "chained sibling-method return" gap found while extending test coverage (a value-returning method can't `return this.sibling(...)` or `let x = this.sibling(...)` within the same object literal, even though a bare-statement call to the same sibling works fine). Root-caused via --debug-only=mlir tracing: an enclosing function with no explicit return type runs a speculative discovery pass (dummyRun=true, allowPartialResolve=true) that recurses into any nested object literal's methods. A method's prototype is only registered into the literal's (mutable) storage type AFTER its own discovery completes, so an earlier-declared sibling can still be missing from `this`'s type when a later method's body is walked for captured-variable/return-type discovery, regardless of source order. Fixes two of the (at least three) layers this surfaces, both using the same dummyRun/allowPartialResolve tolerance idiom already established elsewhere in both files (mlirGenCallExpression's existing `!result.value && genContext.allowPartialResolve` case): - mlirGenPropertyAccessExpressionBaseLogic (MLIRGenAccessCall.cpp): the "Can't resolve property" error now returns mlir::success() (no value) instead of hard-failing during a dummy/partial-resolve run. - discoverFunctionReturnTypeAndCapturedVars (MLIRGenFunctions.cpp): the "'return' is not found" error is skipped (still returns mlir::failure() to signal non-convergence, but without emitting a diagnostic) when the OUTER context is itself a dummy/partial-resolve run. NOT a complete fix - the original repro still fails, now with a bare "failed statement" and no specific diagnostic. A third, more fundamental layer remains: mlir::failure() without an emitted error still propagates as a hard abort through ordinary sequential statement processing, so the nested discovery's now-silent non-convergence still aborts the whole enclosing function's discovery via a statement (`let calc = {...}`) entirely unrelated to that function's own return type. Full mechanism and the remaining gap are documented in docs/interface-vtable-simplification-design.md for whoever picks this up next - it's not yet known how many more call sites need the same tolerance, and each layer found so far needed a separately-targeted fix. Both changes verified individually and together to cause zero change in behavior for the existing suite: 730/730 passed, unchanged. No regression test added (the repro this was investigating is still broken). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5e995d7 commit 920ec13

3 files changed

Lines changed: 115 additions & 6 deletions

File tree

tslang/docs/interface-vtable-simplification-design.md

Lines changed: 83 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -524,9 +524,87 @@ this.siblingMethod(...)` (using a sibling call's return value directly in
524524
a `return` statement) within the same type-literal-annotated object
525525
literal; calling the sibling as a bare statement (discarding its return
526526
value) works fine. Confirmed same-module, unrelated to cross-module
527-
casting at all - likely a self-referential type-inference ordering gap
528-
(the caller's return type depends on resolving the callee's return type,
529-
which depends on `this`, which is still being constructed). Not
530-
investigated further; avoided in the committed tests
527+
casting at all. Avoided in the committed tests
531528
(`00object_annotated_method_params.ts` uses `setBase`/re-`scale`, not a
532-
chained-return pattern).
529+
chained-return pattern). See the next section for the full mechanism,
530+
found in a follow-up session.
531+
532+
### Chained-return inference gap: mechanism found (2+ of 3 layers fixed), root cause remains (2026-07-19)
533+
534+
Root-caused via `--debug-only=mlir` (traced `discovering 'return type' &
535+
'captured variables'` LLVM_DEBUG output). The mechanism, precisely:
536+
537+
1. An enclosing function with no explicit return type (e.g. `main()`) runs
538+
a speculative **discovery pass** first
539+
(`discoverFunctionReturnTypeAndCapturedVars`, MLIRGenFunctions.cpp) to
540+
infer its return type and captured variables, via a dummy `FuncOp` and
541+
`GenContext{dummyRun=true, allowPartialResolve=true}`.
542+
2. That pass walks the WHOLE body, including unrelated statements like
543+
`let calc = {...}` - which recurses into the object literal's methods
544+
(`processObjectFunctionLikeProto`/`processObjectFunctionLike`,
545+
MLIRGenImpl.h), each of which ALSO runs the same discovery machinery
546+
for ITS OWN return type if not explicit on the method's own literal
547+
syntax (or, it turns out, even when it IS explicit - see below).
548+
3. Methods are discovered in declaration order, but a method's prototype
549+
is only registered into the literal's (mutable) `ObjectStorageType`
550+
AFTER its OWN discovery completes - so when `scaleAndAdd`'s body (being
551+
walked to infer test ITS return type, or just to find captured vars)
552+
references `this.scale(...)`, `scale`'s prototype may not be in the
553+
storage type yet EVEN THOUGH `scale` is declared earlier in the source
554+
and even when `scaleAndAdd` HAS an explicit return type on its own
555+
literal syntax (confirmed: adding `: number` to `scaleAndAdd` itself
556+
did not avoid the failure - the discovery machinery runs regardless,
557+
apparently also needed for captured-variable discovery).
558+
4. Property access on `this` for `scale` then fails to resolve
559+
(`mlirGenPropertyAccessExpressionBaseLogic`, MLIRGenAccessCall.cpp) -
560+
but ONLY when the call's result is USED (assigned, returned, or part of
561+
a larger expression). A bare-statement call (`this.scale(factor);`,
562+
result discarded) does not hit this: statement calls don't need the
563+
property access to produce a typed VALUE the way an expression context
564+
does, so they don't trip the same resolution path during discovery.
565+
566+
**Fixed** (two gates, both matching the SAME `dummyRun`/`allowPartialResolve`
567+
tolerance idiom already used elsewhere in both files - e.g.
568+
`mlirGenCallExpression`'s `!result.value && genContext.allowPartialResolve`
569+
case in MLIRGenAccessCall.cpp):
570+
- `mlirGenPropertyAccessExpressionBaseLogic`'s final
571+
`emitError("Can't resolve property...")` now returns `mlir::success()`
572+
(no value, not a hard failure) when `genContext.dummyRun ||
573+
genContext.allowPartialResolve`.
574+
- `discoverFunctionReturnTypeAndCapturedVars`'s
575+
`emitError("'return' is not found...")` now skips the diagnostic (still
576+
returns `mlir::failure()` to signal "this discovery attempt didn't
577+
converge", but without recording a user-facing error) under the same
578+
condition, checked against the OUTER `genContext` (the nested
579+
discovery's caller) rather than the freshly-constructed
580+
`genContextWithPassResult` (which is unconditionally
581+
`dummyRun=true`/`allowPartialResolve=true` for ANY discovery call,
582+
nested or not, and so can't distinguish "nested inside another dummy
583+
run" from "top-level, real discovery" on its own).
584+
585+
Both verified individually and together: no change in behavior for any
586+
existing test, full suite stayed at 730/730 with both fixes applied.
587+
588+
**NOT fixed - a third, more fundamental layer remains.** Even with both
589+
gates, the original repro still fails, now with a bare
590+
`error: failed statement` for `main()` and no more specific diagnostic.
591+
Cause: `mlir::failure()` - even without an emitted diagnostic - still
592+
propagates as a hard abort through ordinary SEQUENTIAL STATEMENT
593+
processing (`mlirGenFunctionBody` and friends don't distinguish "a nested
594+
speculative sub-discovery legitimately didn't converge, continue as if
595+
this statement is fine" from "a real error occurred, stop everything").
596+
So `scaleAndAdd`'s nested discovery failure - now silent, thanks to the
597+
two gates above - still bubbles up through processing the (entirely
598+
unrelated to `main`'s return type) `let calc = {...}` statement, aborting
599+
`main`'s WHOLE discovery pass, which is where the final `failed statement`
600+
error comes from. A real fix needs the statement-processing layer (or
601+
whichever call site turns "one nested dummy sub-discovery didn't
602+
converge" into "abort the entire enclosing discovery") to tolerate this
603+
too - and it's not yet known how many such call sites exist, since this
604+
is the third layer found and each one so far needed its own targeted
605+
gate. Deliberately not chased further this session - the two committed
606+
gates are real, idiom-consistent, and non-regressing improvements on their
607+
own, but do not by themselves fix the user-visible repro. No regression
608+
test added (the repro this was investigating still fails); the two
609+
partial fixes are covered only by "full suite still green," not by a
610+
test proving the original bug is fixed.

tslang/lib/TypeScript/MLIRGenAccessCall.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,21 @@ namespace mlirgen
381381

382382
if (!value)
383383
{
384+
// During a speculative discovery/dummy run (e.g. inferring an enclosing
385+
// function's return type, which recurses into an object literal's method
386+
// bodies to guess ITS return type too), a sibling method's prototype may
387+
// not be registered into the literal's (mutable) object-storage type yet -
388+
// `this.siblingMethod(...)` used in an expression (not a bare statement)
389+
// then fails to resolve here even though it will resolve fine once real
390+
// compilation runs with all prototypes registered. Don't hard-fail the
391+
// whole discovery run over that; let the caller treat this as "unknown for
392+
// now" (same idiom as mlirGenCallExpression's `!result.value &&
393+
// genContext.allowPartialResolve` case above).
394+
if (genContext.dummyRun || genContext.allowPartialResolve)
395+
{
396+
return mlir::success();
397+
}
398+
384399
emitError(location, "Can't resolve property '") << name << "' of type " << to_print(objectValue.getType());
385400
return mlir::failure();
386401
}

tslang/lib/TypeScript/MLIRGenFunctions.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,11 +445,27 @@ namespace mlirgen
445445
exitNamespace();
446446

447447
auto &passResult = genContextWithPassResult.passResult;
448-
if (passResult->functionReturnTypeShouldBeProvided
448+
if (passResult->functionReturnTypeShouldBeProvided
449449
&& mth.isNoneType(passResult->functionReturnType))
450450
{
451451
// has return value but type is not provided yet
452452
genContextWithPassResult.clean();
453+
454+
// if THIS discovery is itself nested inside an outer speculative
455+
// discovery/dummy run (e.g. an object literal's method being
456+
// return-type-discovered as a side effect of discovering the
457+
// enclosing function - see the allowPartialResolve tolerance in
458+
// mlirGenPropertyAccessExpressionBaseLogic), a sibling member's
459+
// prototype may not be registered yet, so a return expression that
460+
// depends on it can legitimately come back as "unknown" here. Don't
461+
// hard-fail the whole discovery over that - the outer caller (and
462+
// the real, non-dummy compile pass) will resolve it once every
463+
// sibling's prototype is registered.
464+
if (genContext.dummyRun || genContext.allowPartialResolve)
465+
{
466+
return mlir::failure();
467+
}
468+
453469
emitError(loc(functionLikeDeclarationBaseAST)) << "'return' is not found in function or return type can't be resolved";
454470
return mlir::failure();
455471
}

0 commit comments

Comments
 (0)