From 920ec13a3fbc5428eb2a3fdd4b7a1cd736e9fa4c Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sun, 19 Jul 2026 23:54:43 +0100 Subject: [PATCH] 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 --- .../interface-vtable-simplification-design.md | 88 +++++++++++++++++-- tslang/lib/TypeScript/MLIRGenAccessCall.cpp | 15 ++++ tslang/lib/TypeScript/MLIRGenFunctions.cpp | 18 +++- 3 files changed, 115 insertions(+), 6 deletions(-) diff --git a/tslang/docs/interface-vtable-simplification-design.md b/tslang/docs/interface-vtable-simplification-design.md index ea4fb3c23..7890f500f 100644 --- a/tslang/docs/interface-vtable-simplification-design.md +++ b/tslang/docs/interface-vtable-simplification-design.md @@ -524,9 +524,87 @@ this.siblingMethod(...)` (using a sibling call's return value directly in a `return` statement) within the same type-literal-annotated object literal; calling the sibling as a bare statement (discarding its return value) works fine. Confirmed same-module, unrelated to cross-module -casting at all - likely a self-referential type-inference ordering gap -(the caller's return type depends on resolving the callee's return type, -which depends on `this`, which is still being constructed). Not -investigated further; avoided in the committed tests +casting at all. Avoided in the committed tests (`00object_annotated_method_params.ts` uses `setBase`/re-`scale`, not a -chained-return pattern). +chained-return pattern). See the next section for the full mechanism, +found in a follow-up session. + +### Chained-return inference gap: mechanism found (2+ of 3 layers fixed), root cause remains (2026-07-19) + +Root-caused via `--debug-only=mlir` (traced `discovering 'return type' & +'captured variables'` LLVM_DEBUG output). The mechanism, precisely: + +1. An enclosing function with no explicit return type (e.g. `main()`) runs + a speculative **discovery pass** first + (`discoverFunctionReturnTypeAndCapturedVars`, MLIRGenFunctions.cpp) to + infer its return type and captured variables, via a dummy `FuncOp` and + `GenContext{dummyRun=true, allowPartialResolve=true}`. +2. That pass walks the WHOLE body, including unrelated statements like + `let calc = {...}` - which recurses into the object literal's methods + (`processObjectFunctionLikeProto`/`processObjectFunctionLike`, + MLIRGenImpl.h), each of which ALSO runs the same discovery machinery + for ITS OWN return type if not explicit on the method's own literal + syntax (or, it turns out, even when it IS explicit - see below). +3. Methods are discovered in declaration order, but a method's prototype + is only registered into the literal's (mutable) `ObjectStorageType` + AFTER its OWN discovery completes - so when `scaleAndAdd`'s body (being + walked to infer test ITS return type, or just to find captured vars) + references `this.scale(...)`, `scale`'s prototype may not be in the + storage type yet EVEN THOUGH `scale` is declared earlier in the source + and even when `scaleAndAdd` HAS an explicit return type on its own + literal syntax (confirmed: adding `: number` to `scaleAndAdd` itself + did not avoid the failure - the discovery machinery runs regardless, + apparently also needed for captured-variable discovery). +4. Property access on `this` for `scale` then fails to resolve + (`mlirGenPropertyAccessExpressionBaseLogic`, MLIRGenAccessCall.cpp) - + but ONLY when the call's result is USED (assigned, returned, or part of + a larger expression). A bare-statement call (`this.scale(factor);`, + result discarded) does not hit this: statement calls don't need the + property access to produce a typed VALUE the way an expression context + does, so they don't trip the same resolution path during discovery. + +**Fixed** (two gates, both matching the SAME `dummyRun`/`allowPartialResolve` +tolerance idiom already used elsewhere in both files - e.g. +`mlirGenCallExpression`'s `!result.value && genContext.allowPartialResolve` +case in MLIRGenAccessCall.cpp): +- `mlirGenPropertyAccessExpressionBaseLogic`'s final + `emitError("Can't resolve property...")` now returns `mlir::success()` + (no value, not a hard failure) when `genContext.dummyRun || + genContext.allowPartialResolve`. +- `discoverFunctionReturnTypeAndCapturedVars`'s + `emitError("'return' is not found...")` now skips the diagnostic (still + returns `mlir::failure()` to signal "this discovery attempt didn't + converge", but without recording a user-facing error) under the same + condition, checked against the OUTER `genContext` (the nested + discovery's caller) rather than the freshly-constructed + `genContextWithPassResult` (which is unconditionally + `dummyRun=true`/`allowPartialResolve=true` for ANY discovery call, + nested or not, and so can't distinguish "nested inside another dummy + run" from "top-level, real discovery" on its own). + +Both verified individually and together: no change in behavior for any +existing test, full suite stayed at 730/730 with both fixes applied. + +**NOT fixed - a third, more fundamental layer remains.** Even with both +gates, the original repro still fails, now with a bare +`error: failed statement` for `main()` and no more specific diagnostic. +Cause: `mlir::failure()` - even without an emitted diagnostic - still +propagates as a hard abort through ordinary SEQUENTIAL STATEMENT +processing (`mlirGenFunctionBody` and friends don't distinguish "a nested +speculative sub-discovery legitimately didn't converge, continue as if +this statement is fine" from "a real error occurred, stop everything"). +So `scaleAndAdd`'s nested discovery failure - now silent, thanks to the +two gates above - still bubbles up through processing the (entirely +unrelated to `main`'s return type) `let calc = {...}` statement, aborting +`main`'s WHOLE discovery pass, which is where the final `failed statement` +error comes from. A real fix needs the statement-processing layer (or +whichever call site turns "one nested dummy sub-discovery didn't +converge" into "abort the entire enclosing discovery") to tolerate this +too - and it's not yet known how many such call sites exist, since this +is the third layer found and each one so far needed its own targeted +gate. Deliberately not chased further this session - the two committed +gates are real, idiom-consistent, and non-regressing improvements on their +own, but do not by themselves fix the user-visible repro. No regression +test added (the repro this was investigating still fails); the two +partial fixes are covered only by "full suite still green," not by a +test proving the original bug is fixed. diff --git a/tslang/lib/TypeScript/MLIRGenAccessCall.cpp b/tslang/lib/TypeScript/MLIRGenAccessCall.cpp index 101f00104..b3bdac111 100644 --- a/tslang/lib/TypeScript/MLIRGenAccessCall.cpp +++ b/tslang/lib/TypeScript/MLIRGenAccessCall.cpp @@ -381,6 +381,21 @@ namespace mlirgen if (!value) { + // During a speculative discovery/dummy run (e.g. inferring an enclosing + // function's return type, which recurses into an object literal's method + // bodies to guess ITS return type too), a sibling method's prototype may + // not be registered into the literal's (mutable) object-storage type yet - + // `this.siblingMethod(...)` used in an expression (not a bare statement) + // then fails to resolve here even though it will resolve fine once real + // compilation runs with all prototypes registered. Don't hard-fail the + // whole discovery run over that; let the caller treat this as "unknown for + // now" (same idiom as mlirGenCallExpression's `!result.value && + // genContext.allowPartialResolve` case above). + if (genContext.dummyRun || genContext.allowPartialResolve) + { + return mlir::success(); + } + emitError(location, "Can't resolve property '") << name << "' of type " << to_print(objectValue.getType()); return mlir::failure(); } diff --git a/tslang/lib/TypeScript/MLIRGenFunctions.cpp b/tslang/lib/TypeScript/MLIRGenFunctions.cpp index 116b770bb..c18ba8c00 100644 --- a/tslang/lib/TypeScript/MLIRGenFunctions.cpp +++ b/tslang/lib/TypeScript/MLIRGenFunctions.cpp @@ -445,11 +445,27 @@ namespace mlirgen exitNamespace(); auto &passResult = genContextWithPassResult.passResult; - if (passResult->functionReturnTypeShouldBeProvided + if (passResult->functionReturnTypeShouldBeProvided && mth.isNoneType(passResult->functionReturnType)) { // has return value but type is not provided yet genContextWithPassResult.clean(); + + // if THIS discovery is itself nested inside an outer speculative + // discovery/dummy run (e.g. an object literal's method being + // return-type-discovered as a side effect of discovering the + // enclosing function - see the allowPartialResolve tolerance in + // mlirGenPropertyAccessExpressionBaseLogic), a sibling member's + // prototype may not be registered yet, so a return expression that + // depends on it can legitimately come back as "unknown" here. Don't + // hard-fail the whole discovery over that - the outer caller (and + // the real, non-dummy compile pass) will resolve it once every + // sibling's prototype is registered. + if (genContext.dummyRun || genContext.allowPartialResolve) + { + return mlir::failure(); + } + emitError(loc(functionLikeDeclarationBaseAST)) << "'return' is not found in function or return type can't be resolved"; return mlir::failure(); }