diff --git a/Compilation/ILCompiler.Classes.Methods.cs b/Compilation/ILCompiler.Classes.Methods.cs index 39533911..2a5a5bd5 100644 --- a/Compilation/ILCompiler.Classes.Methods.cs +++ b/Compilation/ILCompiler.Classes.Methods.cs @@ -811,6 +811,10 @@ private void EmitPrivateMethodBody( // Emit method body if (method.Body != null) { + // #1237: materialize inner function declarations in place, matching the public-method + // path so an inner `function` declared inside a private method becomes a binding. + WireInPlaceInnerFunctions(ctx); + foreach (var stmt in method.Body) { emitter.EmitStatement(stmt); @@ -1185,6 +1189,15 @@ private void EmitMethod(TypeBuilder typeBuilder, Stmt.Function method, FieldInfo // Abstract methods have no body to emit if (method.Body != null) { + // #1237: an inner `function` declared inside a method is collected (its method/display + // class are emitted) but was never materialized into a binding here, so every reference + // fell through to ThrowUndefinedVariable. Wire the in-place materializer so each inner + // function declaration is created at its textual position by the statement emitter's + // Stmt.Function arm. Methods have no function-level display class, so a top-of-body hoist + // (EmitInnerFunctionHoisting) would snapshot captured method-locals before they are + // assigned; in-place materialization captures them correctly. See WireInPlaceInnerFunctions. + WireInPlaceInnerFunctions(ctx); + foreach (var stmt in method.Body) { emitter.EmitStatement(stmt); diff --git a/Compilation/ILCompiler.Classes.Static.cs b/Compilation/ILCompiler.Classes.Static.cs index e617652b..40cd8248 100644 --- a/Compilation/ILCompiler.Classes.Static.cs +++ b/Compilation/ILCompiler.Classes.Static.cs @@ -505,6 +505,10 @@ private void EmitStaticMethodBody(string className, Stmt.Function method) // Abstract methods have no body to emit if (method.Body != null) { + // #1237: materialize inner function declarations in place, matching the instance-method + // path so an inner `function` declared inside a static method becomes a binding. + WireInPlaceInnerFunctions(ctx); + foreach (var stmt in method.Body) { emitter.EmitStatement(stmt); diff --git a/Compilation/ILCompiler.InnerFunctions.cs b/Compilation/ILCompiler.InnerFunctions.cs index 762117a7..df7b71d1 100644 --- a/Compilation/ILCompiler.InnerFunctions.cs +++ b/Compilation/ILCompiler.InnerFunctions.cs @@ -717,6 +717,27 @@ private void EmitInnerFunctionHoisting(ILGenerator il, CompilationContext ctx, L } } + /// + /// Wires the in-place inner-function materializer () + /// onto WITHOUT running the top-of-body two-pass hoist that + /// performs. Class-method bodies (#1237) use this: unlike a + /// plain function or arrow, a method has no function-level display class, so hoisting a capturing + /// inner function to the top of the body would snapshot its captured method-locals BEFORE the body + /// assigns them — leaving the closure's capture fields null. Leaving + /// empty means every inner function + /// declaration — top-level-of-body or block-nested — is materialized in place at its textual + /// position by the statement emitter's Stmt.Function arm, so its capture snapshot sees the + /// values already assigned. This matches how arrows inside methods capture (snapshot at creation), + /// and admits every program the type checker allows: it rejects forward references to a method's + /// inner functions (unlike function bodies, which it hoists), so no valid program needs the two-pass + /// pre-materialization here. + /// + private void WireInPlaceInnerFunctions(CompilationContext ctx) + { + ctx.EmitBlockScopedInnerFunction ??= EmitBlockScopedInnerFunctionDeclaration; + ctx.HoistedInnerFunctions ??= new HashSet(ReferenceEqualityComparer.Instance); + } + /// /// Materializes a single inner function declaration nested inside a block/loop/if at its /// textual position: creates its TSFunction and binds a block-scoped local, then snapshots its diff --git a/SharpTS.Tests/SharedTests/InnerFunctionInMethodTests.cs b/SharpTS.Tests/SharedTests/InnerFunctionInMethodTests.cs new file mode 100644 index 00000000..ebbdc1aa --- /dev/null +++ b/SharpTS.Tests/SharedTests/InnerFunctionInMethodTests.cs @@ -0,0 +1,241 @@ +using SharpTS.Tests.Infrastructure; +using Xunit; + +namespace SharpTS.Tests.SharedTests; + +/// +/// Regression tests for #1237: a function declaration inside a class method body must be +/// referenceable (and callable) in compiled mode, matching the interpreter. +/// +/// The class-method emission path (ILCompiler.Classes.Methods.cs / +/// ILCompiler.Classes.Static.cs) emitted a method body via a bare +/// foreach EmitStatement and never wired the inner-function materializer, so an inner +/// function in a method was collected (its method and display class were emitted) but never +/// materialized into a binding — every reference fell through to ThrowUndefinedVariable +/// (ReferenceError: Undefined variable). This affected non-capturing, capturing, +/// top-level-in-method, and block-nested declarations, on instance, static, and private methods. +/// +/// The fix wires the in-place materializer onto the method context +/// (WireInPlaceInnerFunctions) so each inner function declaration is created at its textual +/// position by the statement emitter's Stmt.Function arm. Unlike a plain function or arrow, a +/// method has no function-level display class; hoisting a capturing inner function to the top of the +/// body would snapshot its captured method-locals before the body assigns them (null capture), so +/// in-place materialization is used instead — its snapshot sees the already-assigned values, matching +/// how arrows inside methods capture. The type checker rejects forward references to a method's inner +/// functions, so no valid program needs the top-of-body two-pass hoist here. +/// +public class InnerFunctionInMethodTests +{ + // ---- The headline #1237 repro: non-capturing inner function in an instance method ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void InstanceMethod_NonCapturingInnerFunction_IsReferenceable(ExecutionMode mode) + { + var source = """ + class C { + m() { + function top() { return "top"; } + return top(); + } + } + console.log(new C().m()); + """; + Assert.Equal("top\n", TestHarness.Run(source, mode)); + } + + // ---- Capturing a method-local const (was "Undefined variable", then a null capture pre-fix) ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void InstanceMethod_CapturingInnerFunction_SeesMethodLocal(ExecutionMode mode) + { + var source = """ + class C { + m() { + const k = "K"; + function cap() { return "k=" + k; } + return cap(); + } + } + console.log(new C().m()); + """; + Assert.Equal("k=K\n", TestHarness.Run(source, mode)); + } + + // ---- Capturing a method parameter (value-type double must be boxed into the closure field) ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void InstanceMethod_InnerFunction_CapturesParameter(ExecutionMode mode) + { + var source = """ + class C { + dbl(x: number) { + function inner() { return x * 2; } + return inner(); + } + } + console.log(new C().dbl(21)); + """; + Assert.Equal("42\n", TestHarness.Run(source, mode)); + } + + // ---- Capturing a value-type method-local (boxing on the local fallback path) ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void InstanceMethod_InnerFunction_CapturesNumericLocal(ExecutionMode mode) + { + var source = """ + class C { + m() { + let n = 5; + function f() { return n + 1; } + return f(); + } + } + console.log(new C().m()); + """; + Assert.Equal("6\n", TestHarness.Run(source, mode)); + } + + // ---- Self-recursion (own name resolved via direct dispatch, not a captured local) ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void InstanceMethod_InnerFunction_SelfRecursion(ExecutionMode mode) + { + var source = """ + class C { + fact() { + function fac(n: number): number { return n <= 1 ? 1 : n * fac(n - 1); } + return fac(5); + } + } + console.log(new C().fact()); + """; + Assert.Equal("120\n", TestHarness.Run(source, mode)); + } + + // ---- Two inner functions sharing a captured method-local ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void InstanceMethod_TwoInnerFunctions_ShareCapture(ExecutionMode mode) + { + var source = """ + class C { + m() { + const a = "A"; + function f1() { return a; } + function f2() { return a + a; } + return f1() + f2(); + } + } + console.log(new C().m()); + """; + Assert.Equal("AAA\n", TestHarness.Run(source, mode)); + } + + // ---- Capturing a module top-level variable from inside a method (routes via entry-point DC) ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void InstanceMethod_InnerFunction_CapturesTopLevelVar(ExecutionMode mode) + { + var source = """ + const G = "top-level"; + class C { + m() { + function g() { return G; } + return g(); + } + } + console.log(new C().m()); + """; + Assert.Equal("top-level\n", TestHarness.Run(source, mode)); + } + + // ---- Block-nested inner function inside a method (composes with #1230 in-place path) ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Method_LoopBody_InnerFunction_CapturesLoopVariable_PerIteration(ExecutionMode mode) + { + var source = """ + class C { + m() { + const out: string[] = []; + for (let i = 0; i < 3; i++) { + function make() { return i; } + out.push("" + make()); + } + return out.join(","); + } + } + console.log(new C().m()); + """; + Assert.Equal("0,1,2\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Method_IfBlock_InnerFunction_CapturesBlockConst(ExecutionMode mode) + { + var source = """ + class C { + m() { + let acc = 0; + if (true) { + const step = 10; + function add() { return acc + step; } + acc = add(); + } + return acc; + } + } + console.log(new C().m()); + """; + Assert.Equal("10\n", TestHarness.Run(source, mode)); + } + + // ---- Static method ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void StaticMethod_InnerFunction_IsReferenceable(ExecutionMode mode) + { + var source = """ + class C { + static s() { + const tag = "S"; + function h() { return "static-" + tag; } + return h(); + } + } + console.log(C.s()); + """; + Assert.Equal("static-S\n", TestHarness.Run(source, mode)); + } + + // ---- Private method (ES2022 #member) ---- + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void PrivateMethod_InnerFunction_IsReferenceable(ExecutionMode mode) + { + var source = """ + class C { + #impl() { + const v = "priv"; + function h() { return v + "!"; } + return h(); + } + run() { return this.#impl(); } + } + console.log(new C().run()); + """; + Assert.Equal("priv!\n", TestHarness.Run(source, mode)); + } +}