diff --git a/Compilation/AsyncArrowMoveNextEmitter.ArrowFunctions.cs b/Compilation/AsyncArrowMoveNextEmitter.ArrowFunctions.cs index 22ddc733..1fdba6e8 100644 --- a/Compilation/AsyncArrowMoveNextEmitter.ArrowFunctions.cs +++ b/Compilation/AsyncArrowMoveNextEmitter.ArrowFunctions.cs @@ -111,18 +111,55 @@ private void EmitNestedAsyncArrow(Expr.ArrowFunction af) // stub's arg0, clobbering the first real parameter. (#615) if (nestedBuilder.IsStandalone) { - // A capturing standalone nested arrow would need its capture fields populated from the - // enclosing frame, which this path does not wire up — fail loudly rather than read - // uninitialized captures (it did not compile at all before #615; tracked in #641). - if (nestedBuilder.StandaloneCaptureFields.Count > 0) + // A capturing standalone nested arrow's stub takes its single captured value as a leading + // argument. $TSFunction's "static method with target" mechanism prepends the target before + // the call args, so we pass the capture AS the target, read from THIS (enclosing) arrow's + // frame. Mirrors the module-level standalone path in ILEmitter.EmitAsyncArrowFunction. + // Before #641 every capturing standalone nested arrow threw "not yet supported"; now the + // single read-only capture (the #641 repros) is supported, and the two harder shapes below + // fail loudly rather than miscompiling. + var captureOrder = nestedBuilder.StandaloneCaptureFields.Keys + .OrderBy(k => k, System.StringComparer.Ordinal).ToList(); + + if (captureOrder.Count > 0) { - throw new CompileException( - "A nested async arrow that captures variables inside a top-level async arrow is " + - "not yet supported in compiled mode; hoist it to a named async arrow or async " + - "function, or run in interpreted mode."); + // A standalone arrow captures BY VALUE (the values are copied into its own state + // machine). A write to a capture therefore cannot propagate back to the enclosing + // binding, so reject it instead of silently dropping the write (#684). + var written = CapturedWriteAnalysis.CollectImmediateWrites(af); + written.IntersectWith(captureOrder); + if (written.Count > 0) + { + throw new CompileException( + "A nested async arrow inside a top-level async arrow that WRITES a captured " + + $"variable ({string.Join(", ", written.OrderBy(n => n, System.StringComparer.Ordinal))}) " + + "is not yet supported in compiled mode (#684): the standalone arrow captures by " + + "value, so the write would be lost. Hoist it to a named async function, or run " + + "in interpreted mode."); + } + + // Multiple captures would need to ride in $TSFunction's single prepended target slot + // (an object[]), but the standalone stub expects each capture as a separate arg — a + // mismatch also broken for module-level standalone async arrows (#684). + if (captureOrder.Count > 1) + { + throw new CompileException( + "A nested async arrow inside a top-level async arrow that captures more than " + + "one variable is not yet supported in compiled mode (#684); reduce it to a " + + "single capture, hoist it to a named async function, or run in interpreted mode."); + } + } + + if (captureOrder.Count == 1) + { + LoadVariableForCapture(captureOrder[0]); + EnsureBoxed(); + } + else + { + _il.Emit(OpCodes.Ldnull); } - _il.Emit(OpCodes.Ldnull); _il.Emit(OpCodes.Ldtoken, nestedBuilder.StubMethod); _il.Emit(OpCodes.Call, Types.MethodBaseGetMethodFromHandle); _il.Emit(OpCodes.Castclass, typeof(MethodInfo)); diff --git a/Compilation/AsyncArrowMoveNextEmitter.Variables.cs b/Compilation/AsyncArrowMoveNextEmitter.Variables.cs index 2b24185d..d85afae5 100644 --- a/Compilation/AsyncArrowMoveNextEmitter.Variables.cs +++ b/Compilation/AsyncArrowMoveNextEmitter.Variables.cs @@ -10,6 +10,18 @@ protected override void EmitVariable(Expr.Variable v) { string name = v.Name.Lexeme; + // A capture promoted into the enclosing function's display class (#625) is read through + // `outer.functionDC.field`, NOT the boxed state-machine field. Checked before the resolver: + // the variable may still have a (now-unused) hoisted SM field that the resolver would load + // a stale value from. + if (TryGetOuterFunctionDCField(name, out var dcReadField)) + { + EmitLoadOuterFunctionDC(); + _il.Emit(OpCodes.Ldfld, dcReadField); + SetStackUnknown(); + return; + } + // Try resolver first (params, locals, hoisted, captured) var stackType = _resolver!.TryLoadVariable(name); if (stackType != null) @@ -57,6 +69,20 @@ protected override void EmitVariable(Expr.Variable v) return; } + // Standalone capture (#641): a value the enclosing arrow's frame passed as a leading stub + // argument, which the stub copied into a field on THIS arrow's state machine. Checked LAST, + // below the module-level globals above: a top-level variable a standalone arrow closes over + // is ALSO registered as a standalone capture but must be read LIVE from its static field + // (the standalone copy is a stale snapshot) — handling it earlier broke compound/logical + // assignment to such a variable. Reaches here only for genuine enclosing-arrow locals. + if (_builder.StandaloneCaptureFields.TryGetValue(name, out var standaloneField)) + { + _il.Emit(OpCodes.Ldarg_0); + _il.Emit(OpCodes.Ldfld, standaloneField); + SetStackUnknown(); + return; + } + // Not found - push null _il.Emit(OpCodes.Ldnull); SetStackType(StackType.Null); @@ -70,6 +96,20 @@ protected override void EmitAssign(Expr.Assign a) EnsureBoxed(); _il.Emit(OpCodes.Dup); + // A capture promoted into the enclosing function's display class (#625) is written through + // `outer.functionDC.field = value` (the DC is a reference type, so the store is verifiable), + // not by mutating the boxed value-type state machine in place. Checked first. + if (TryGetOuterFunctionDCField(name, out var dcAssignField)) + { + var temp = _il.DeclareLocal(_types.Object); + _il.Emit(OpCodes.Stloc, temp); // consume the duplicated value + EmitLoadOuterFunctionDC(); + _il.Emit(OpCodes.Ldloc, temp); + _il.Emit(OpCodes.Stfld, dcAssignField); + SetStackUnknown(); // remaining copy is the assignment's value + return; + } + // Check if it's a captured top-level variable in entry-point display class if (_ctx?.CapturedTopLevelVars?.Contains(name) == true && _ctx.EntryPointDisplayClassFields?.TryGetValue(name, out var entryPointField) == true && @@ -185,6 +225,18 @@ private void LoadVariable(string name) private void StoreVariable(string name) { + // A capture promoted into the enclosing function's display class (#625) is stored through + // `outer.functionDC.field`. Checked first so it wins over any (now-unused) hoisted SM field. + if (TryGetOuterFunctionDCField(name, out var dcStoreField)) + { + var temp = _il.DeclareLocal(_types.Object); + _il.Emit(OpCodes.Stloc, temp); + EmitLoadOuterFunctionDC(); + _il.Emit(OpCodes.Ldloc, temp); + _il.Emit(OpCodes.Stfld, dcStoreField); + return; + } + // Check if it's a parameter of this arrow if (_builder.ParameterFields.TryGetValue(name, out var paramField)) { @@ -343,4 +395,34 @@ private void LoadVariableForCapture(string name) _il.Emit(OpCodes.Ldnull); SetStackType(StackType.Null); } + + /// + /// True when is a captured variable the enclosing async function placed + /// in its (reference-type) display class (#625). Such a variable must be read/written through + /// outer.functionDC.field rather than the boxed state-machine field. Requires the outer + /// reference plumbing — present only for non-standalone arrows nested directly in an async + /// function — so standalone/top-level async arrows fall through to the existing paths. + /// + private bool TryGetOuterFunctionDCField(string name, out FieldBuilder dcField) + { + dcField = null!; + return _ctx?.OuterFunctionDCField != null + && _builder.OuterStateMachineField != null + && _builder.OuterStateMachineType != null + && _ctx.FunctionDisplayClassFields?.TryGetValue(name, out dcField!) == true; + } + + /// + /// Pushes the enclosing async function's display-class instance: outer.functionDC. + /// Reading the DC reference field through the unbox'd (readonly) outer pointer is + /// verifiable; the resulting reference is an ordinary class instance, so the caller's + /// subsequent ldfld/stfld on it verifies (unlike storing into the boxed struct). + /// + private void EmitLoadOuterFunctionDC() + { + _il.Emit(OpCodes.Ldarg_0); + _il.Emit(OpCodes.Ldfld, _builder.OuterStateMachineField!); + _il.Emit(OpCodes.Unbox, _builder.OuterStateMachineType!); + _il.Emit(OpCodes.Ldfld, _ctx!.OuterFunctionDCField!); + } } diff --git a/Compilation/AsyncArrowStateMachineBuilder.cs b/Compilation/AsyncArrowStateMachineBuilder.cs index 16be266f..38ebd226 100644 --- a/Compilation/AsyncArrowStateMachineBuilder.cs +++ b/Compilation/AsyncArrowStateMachineBuilder.cs @@ -329,7 +329,8 @@ public void DefineStubMethod(TypeBuilder programType) // For standalone arrows with captures, add capture parameters first // Order: [captures...], [arrow params...] - var captureOrder = StandaloneCaptureFields.Keys.OrderBy(k => k).ToList(); + // Ordinal ordering must match the call sites (ILEmitter / AsyncArrowMoveNextEmitter). + var captureOrder = StandaloneCaptureFields.Keys.OrderBy(k => k, System.StringComparer.Ordinal).ToList(); if (IsStandalone) { foreach (var _ in captureOrder) diff --git a/Compilation/AsyncArrowVariableResolver.cs b/Compilation/AsyncArrowVariableResolver.cs index a545ec3a..8a814456 100644 --- a/Compilation/AsyncArrowVariableResolver.cs +++ b/Compilation/AsyncArrowVariableResolver.cs @@ -71,6 +71,10 @@ public AsyncArrowVariableResolver( return StackType.Unknown; } + // NOTE: standalone captures (#641) are resolved by the EMITTER (AsyncArrowMoveNextEmitter) + // at LOWER priority than module-level globals, because a top-level variable a standalone + // arrow closes over is ALSO registered as a standalone capture but must be read LIVE from + // its static field, not from the by-value snapshot. Handling it here would shadow that. return null; // Not found - caller handles fallback } diff --git a/Compilation/CapturedWriteAnalysis.cs b/Compilation/CapturedWriteAnalysis.cs new file mode 100644 index 00000000..8e4e8782 --- /dev/null +++ b/Compilation/CapturedWriteAnalysis.cs @@ -0,0 +1,69 @@ +using SharpTS.Parsing; +using SharpTS.Parsing.Visitors; + +namespace SharpTS.Compilation; + +/// +/// Determines which variables an arrow/function body assigns to within its own scope — +/// descending through blocks, loops, conditionals and call arguments, but NOT into nested arrows +/// or function declarations (those introduce their own bindings, so a write there is not a write +/// to this scope's captures). Intersecting the result with a closure's capture set identifies the +/// captures it mutates, which the compiler must back with shared (reference-type) storage rather +/// than a by-value snapshot. +/// +internal static class CapturedWriteAnalysis +{ + /// + /// Returns the names assigned within 's own scope. + /// + public static HashSet CollectImmediateWrites(Expr.ArrowFunction arrow) + { + var collector = new WrittenNameCollector(); + if (arrow.ExpressionBody != null) + collector.Visit(arrow.ExpressionBody); + if (arrow.BlockBody != null) + foreach (var stmt in arrow.BlockBody) + collector.Visit(stmt); + return collector.Names; + } + + private sealed class WrittenNameCollector : AstVisitorBase + { + public readonly HashSet Names = []; + + protected override void VisitAssign(Expr.Assign expr) + { + Names.Add(expr.Name.Lexeme); + base.VisitAssign(expr); + } + + protected override void VisitCompoundAssign(Expr.CompoundAssign expr) + { + Names.Add(expr.Name.Lexeme); + base.VisitCompoundAssign(expr); + } + + protected override void VisitLogicalAssign(Expr.LogicalAssign expr) + { + Names.Add(expr.Name.Lexeme); + base.VisitLogicalAssign(expr); + } + + protected override void VisitPrefixIncrement(Expr.PrefixIncrement expr) + { + if (expr.Operand is Expr.Variable v) Names.Add(v.Name.Lexeme); + base.VisitPrefixIncrement(expr); + } + + protected override void VisitPostfixIncrement(Expr.PostfixIncrement expr) + { + if (expr.Operand is Expr.Variable v) Names.Add(v.Name.Lexeme); + base.VisitPostfixIncrement(expr); + } + + // Stop at nested closure boundaries: their assignments target their own bindings (or + // deeper captures), so they must not be attributed to the enclosing scope's captures. + protected override void VisitArrowFunction(Expr.ArrowFunction expr) { } + protected override void VisitFunction(Stmt.Function stmt) { } + } +} diff --git a/Compilation/CompilationContext.Closures.cs b/Compilation/CompilationContext.Closures.cs index c029c051..403841fe 100644 --- a/Compilation/CompilationContext.Closures.cs +++ b/Compilation/CompilationContext.Closures.cs @@ -144,6 +144,15 @@ public partial class CompilationContext /// public FieldBuilder? CurrentArrowFunctionDCField { get; set; } + /// + /// For an async arrow's MoveNext: the <>__functionDC field on the enclosing async + /// function's state machine. Combined with , it lets the + /// arrow read/write captured locals that were promoted into the (reference-type) function display + /// class — `outer.functionDC.field` — instead of mutating a field of the boxed value-type state + /// machine in place, which is unverifiable (`unbox` yields a readonly managed pointer; #625). + /// + public FieldBuilder? OuterFunctionDCField { get; set; } + // ============================================ // Arrow Scope Display Class (for arrow-local vars captured by nested arrows) // ============================================ diff --git a/Compilation/ExpressionEmitterBase.cs b/Compilation/ExpressionEmitterBase.cs index 6303c46f..1af14004 100644 --- a/Compilation/ExpressionEmitterBase.cs +++ b/Compilation/ExpressionEmitterBase.cs @@ -1502,6 +1502,22 @@ protected virtual void EmitArrowFunction(Expr.ArrowFunction af) return; } + // Capturing arrow: its body is emitted as an instance method on a display class, so the + // $TSFunction must be bound to a freshly-constructed display-class instance whose fields + // hold the captured values. Emitting a null target here (the old behaviour) produced + // "Non-static method requires a target" when the arrow was later invoked (#435/#669, + // sync generator bodies). Captured values are read through the GetHoistedVariableField / + // GetThisField hooks plus Ctx.Locals, which every emitter that reaches this base default + // already implements. GeneratorMoveNextEmitter is currently the only such emitter — all + // others (ILEmitter, Async/AsyncArrow/AsyncGenerator MoveNext) override EmitArrowFunction. + if (Ctx.DisplayClasses?.ContainsKey(af) == true && + Ctx.DisplayClassConstructors?.TryGetValue(af, out var displayCtor) == true) + { + EmitCapturingArrowViaHooks(af, method, displayCtor); + return; + } + + // Non-capturing arrow: static method, null target. IL.Emit(OpCodes.Ldnull); IL.Emit(OpCodes.Ldtoken, method); IL.Emit(OpCodes.Call, Types.MethodBaseGetMethodFromHandle); @@ -1515,6 +1531,59 @@ protected virtual void EmitArrowFunction(Expr.ArrowFunction af) SetStackUnknown(); } + /// + /// Emits a capturing arrow as a $TSFunction bound to a new display-class instance, populating + /// the display class's captured fields from the emitter's storage (hoisted state-machine + /// fields, this, or IL locals). Mirrors the per-emitter overrides in + /// AsyncMoveNextEmitter / AsyncGeneratorMoveNextEmitter, but resolves storage through the + /// shared GetHoistedVariableField / GetThisField hooks so a single implementation serves any + /// emitter that uses the base . + /// + private void EmitCapturingArrowViaHooks(Expr.ArrowFunction af, MethodBuilder method, ConstructorBuilder displayCtor) + { + IL.Emit(OpCodes.Newobj, displayCtor); + + if (Ctx.DisplayClassFields?.TryGetValue(af, out var fieldMap) == true) + { + foreach (var (capturedVar, field) in fieldMap) + { + IL.Emit(OpCodes.Dup); + + // `this` is checked before the hoisted-variable map: the generator's + // hoisting analyzer can mint a state-machine field keyed "this" that the + // stub never populates, so resolving it via the map would snapshot null + // (NRE when the arrow dereferences `this`). The real receiver lives in the + // builder's dedicated ThisField (set by the instance-method stub). + if (capturedVar == "this" && GetThisField() is FieldBuilder thisField) + { + IL.Emit(OpCodes.Ldarg_0); + IL.Emit(OpCodes.Ldfld, thisField); + } + else if (GetHoistedVariableField(capturedVar) is FieldBuilder hoistedField) + { + IL.Emit(OpCodes.Ldarg_0); + IL.Emit(OpCodes.Ldfld, hoistedField); + } + else if (Ctx.Locals.TryGetLocal(capturedVar, out var local)) + { + IL.Emit(OpCodes.Ldloc, local); + } + else + { + IL.Emit(OpCodes.Ldnull); + } + + IL.Emit(OpCodes.Stfld, field); + } + } + + IL.Emit(OpCodes.Ldtoken, method); + IL.Emit(OpCodes.Call, Types.MethodBaseGetMethodFromHandle); + IL.Emit(OpCodes.Castclass, typeof(MethodInfo)); + IL.Emit(OpCodes.Newobj, Ctx.Runtime!.TSFunctionCtor); + SetStackUnknown(); + } + // EmitCall is virtual with a default implementation in ExpressionEmitterBase.CallHelpers.cs // EmitCompoundAssign, EmitLogicalAssign, EmitPrefixIncrement, EmitPostfixIncrement // are implemented in ExpressionEmitterBase.Operators.cs as virtual methods. diff --git a/Compilation/GeneratorMoveNextEmitter.ArrowFunctions.cs b/Compilation/GeneratorMoveNextEmitter.ArrowFunctions.cs new file mode 100644 index 00000000..e3e34e10 --- /dev/null +++ b/Compilation/GeneratorMoveNextEmitter.ArrowFunctions.cs @@ -0,0 +1,44 @@ +using SharpTS.Diagnostics.Exceptions; +using SharpTS.Parsing; + +namespace SharpTS.Compilation; + +public partial class GeneratorMoveNextEmitter +{ + /// + /// Arrow emission inside a generator MoveNext. Delegates the actual emission to the + /// base (which instantiates the display + /// class for capturing arrows via the GetThisField / GetHoistedVariableField hooks), but first + /// rejects the one shape the generator path cannot yet honour: an arrow that writes to + /// a variable it captures from the enclosing generator scope. + /// + protected override void EmitArrowFunction(Expr.ArrowFunction af) + { + // Capturing arrows snapshot their captures BY VALUE into the arrow's display class. That is + // correct for read-only captures (the common `yield arr.map(x => x + base)` case), but if the + // arrow assigns to a captured variable, the write lands on the private snapshot and never + // reaches the generator's own storage — the mutation is silently lost. Honouring it needs a + // shared function-level display class threaded through the generator state machine (as plain + // and async functions already have, but generators do not — see #674). Fail fast with a clear + // message rather than miscompiling `arr.forEach(n => sum += n)` to a wrong result. + if (!af.IsAsync && + _ctx?.DisplayClassFields != null && + _ctx.DisplayClassFields.TryGetValue(af, out var captureFields) && + captureFields.Count > 0) + { + var written = CapturedWriteAnalysis.CollectImmediateWrites(af); + written.IntersectWith(captureFields.Keys); + if (written.Count > 0) + { + var names = string.Join(", ", written.OrderBy(n => n, System.StringComparer.Ordinal)); + throw new CompileException( + $"Compiled mode does not yet support an arrow/callback inside a generator (function*) " + + $"body that writes to a variable captured from the generator scope ({names}). The write " + + $"would be lost. Rewrite the mutation outside the callback (e.g. a for…of loop), or run " + + $"in interpreted mode. (#674)"); + } + } + + base.EmitArrowFunction(af); + } +} diff --git a/Compilation/GeneratorStateAnalyzer.cs b/Compilation/GeneratorStateAnalyzer.cs index b8d75652..4e046176 100644 --- a/Compilation/GeneratorStateAnalyzer.cs +++ b/Compilation/GeneratorStateAnalyzer.cs @@ -314,8 +314,35 @@ protected override void VisitSuper(Expr.Super expr) protected override void VisitArrowFunction(Expr.ArrowFunction expr) { - // Arrow functions inside generators don't affect yield analysis - // Don't traverse into them + // Arrow bodies contribute no yield points or hoisted locals to the enclosing + // generator (an arrow can't `yield` to it), so the full generator analysis is + // intentionally NOT run over them. But an arrow lexically captures `this`/`super` + // from the generator, so when the arrow (or a nested arrow) references either, the + // generator instance method must still materialize its `<>4__this` receiver. + // Without this, `this` used ONLY inside an arrow left UsesThis false → no ThisField + // → the captured `this` snapshot was null → NRE when the arrow dereferenced it + // (#435/#669). Detection over-approximates (a `this` inside a nested *regular* + // function, which rebinds `this`, also trips it) — harmless: the worst case is an + // unused field, only ever read by arrows that genuinely capture `this`. + if (!_usesThis) + { + var detector = new ThisUsageDetector(); + detector.Visit(expr); + if (detector.UsesThis) + _usesThis = true; + } + } + + /// + /// Lightweight visitor that reports whether a subtree references this or + /// super. Used to decide whether a generator that only touches this + /// from inside an arrow must still materialize its receiver field. + /// + private sealed class ThisUsageDetector : Parsing.Visitors.AstVisitorBase + { + public bool UsesThis { get; private set; } + protected override void VisitThis(Expr.This expr) => UsesThis = true; + protected override void VisitSuper(Expr.Super expr) => UsesThis = true; } #endregion diff --git a/Compilation/ILCompiler.ArrowFunctions.cs b/Compilation/ILCompiler.ArrowFunctions.cs index 159840bd..5cbfdaa5 100644 --- a/Compilation/ILCompiler.ArrowFunctions.cs +++ b/Compilation/ILCompiler.ArrowFunctions.cs @@ -751,6 +751,16 @@ private void CollectArrowsFromExpr(Expr expr) case Expr.Await aw: CollectArrowsFromExpr(aw.Expression); break; + case Expr.Yield y: + // Arrows inside a yielded expression (`yield arr.map(x => …)`, + // `yield* gen(() => …)`) must be collected too. Without this case the + // collection walk stops at the yield, the arrow's method is never + // registered in ArrowMethods, and EmitArrowFunction falls back to + // `ldnull` — surfacing at runtime as "callback is not callable" inside + // a sync generator body (#435/#669). + if (y.Value != null) + CollectArrowsFromExpr(y.Value); + break; case Expr.DynamicImport di: CollectArrowsFromExpr(di.PathExpression); break; diff --git a/Compilation/ILCompiler.Async.cs b/Compilation/ILCompiler.Async.cs index ab9c9bf7..d6ed19cc 100644 --- a/Compilation/ILCompiler.Async.cs +++ b/Compilation/ILCompiler.Async.cs @@ -48,7 +48,8 @@ private void DefineAsyncFunction(Stmt.Function funcStmt) // Variables also captured by async arrows are excluded since they use the hoisted field mechanism. _closures.FunctionAstNodes[qualifiedFunctionName] = funcStmt; - // Collect variables captured by async arrows (these can't use the function DC) + // Collect variables captured by async arrows. By default these can't use the function DC — + // they're read through the boxed outer state machine in the arrow's MoveNext. var asyncCapturedVars = new HashSet(); foreach (var arrowInfo in analysis.AsyncArrows) { @@ -56,7 +57,31 @@ private void DefineAsyncFunction(Stmt.Function funcStmt) asyncCapturedVars.Add(capture); } - // Filter out async-captured vars before creating the DC + // EXCEPTION: a captured variable that a direct-child async arrow WRITES is promoted into the + // function display class (a reference type) instead of staying on the boxed value-type state + // machine. A boxed struct cannot be mutated in place by verifiable IL — `unbox` yields a + // readonly managed pointer, so `stfld` through it fails verification and can drop the write in + // complex state machines (#625). Routing the write through `outer.functionDC.field` (a class + // reference) is verifiable. Read-only captures stay on the SM field (the existing, working + // load path). Variables also captured by a NESTED async arrow are left as-is — the nested + // arrow's DC-access path isn't wired (#673) — so only direct-child writes are promoted. + var nestedAsyncArrowCaptures = new HashSet(); + foreach (var arrowInfo in analysis.AsyncArrows) + if (arrowInfo.ParentArrow != null) + nestedAsyncArrowCaptures.UnionWith(arrowInfo.Captures); + + var promotedToFunctionDC = new HashSet(); + foreach (var arrowInfo in analysis.AsyncArrows) + { + if (arrowInfo.ParentArrow != null) continue; // direct children only + var written = CapturedWriteAnalysis.CollectImmediateWrites(arrowInfo.Arrow); + written.IntersectWith(arrowInfo.Captures); + written.ExceptWith(nestedAsyncArrowCaptures); + promotedToFunctionDC.UnionWith(written); + } + asyncCapturedVars.ExceptWith(promotedToFunctionDC); + + // Filter out (still-)async-captured vars before creating the DC if (asyncCapturedVars.Count > 0) _closures.AsyncCapturedVarsExclusion[qualifiedFunctionName] = asyncCapturedVars; @@ -545,6 +570,10 @@ private void EmitAsyncStateMachineBodies() ctx.FunctionDisplayClassFields = funcDCFields; ctx.CapturedFunctionLocals = new HashSet(funcDCFields.Keys); } + // Carry the function's own DC field so async arrows nested in this function can reach + // promoted captures through `outer.functionDC.field` (#625). Propagated to the arrow + // MoveNext context in EmitAsyncArrowMoveNext. + ctx.OuterFunctionDCField = smBuilder.FunctionDCField; // Emit MoveNext body var moveNextEmitter = new AsyncMoveNextEmitter(smBuilder, analysis, _types); @@ -633,7 +662,13 @@ private void EmitAsyncArrowMoveNext(AsyncArrowStateMachineBuilder arrowBuilder, // Entry-point display class for captured top-level variables EntryPointDisplayClassFields = parentCtx.EntryPointDisplayClassFields, CapturedTopLevelVars = parentCtx.CapturedTopLevelVars, - EntryPointDisplayClassStaticField = parentCtx.EntryPointDisplayClassStaticField + EntryPointDisplayClassStaticField = parentCtx.EntryPointDisplayClassStaticField, + // Captured locals promoted into the enclosing function's display class (#625): the + // arrow reads/writes them through `outer.functionDC.field` rather than mutating the + // boxed value-type state machine in place (unverifiable). Only fields the function + // actually placed in its DC are listed here, so a name present means "route via DC". + FunctionDisplayClassFields = parentCtx.FunctionDisplayClassFields, + OuterFunctionDCField = parentCtx.OuterFunctionDCField, }; // Create arrow-specific emitter diff --git a/Compilation/ILEmitter.Calls.Closures.cs b/Compilation/ILEmitter.Calls.Closures.cs index 12784b6a..aa7ba42d 100644 --- a/Compilation/ILEmitter.Calls.Closures.cs +++ b/Compilation/ILEmitter.Calls.Closures.cs @@ -96,7 +96,8 @@ private void EmitAsyncArrowFunction(Expr.ArrowFunction af, AsyncArrowStateMachin // mechanism prepends the target to the args, so captures get passed as leading args. if (arrowBuilder.IsStandalone && arrowBuilder.StandaloneCaptureFields.Count > 0) { - var captureOrder = arrowBuilder.StandaloneCaptureFields.Keys.OrderBy(k => k).ToList(); + // Ordinal ordering must match the stub's capture-unpacking order (AsyncArrowStateMachineBuilder). + var captureOrder = arrowBuilder.StandaloneCaptureFields.Keys.OrderBy(k => k, System.StringComparer.Ordinal).ToList(); if (captureOrder.Count == 1) { diff --git a/Compilation/ILVerifier.cs b/Compilation/ILVerifier.cs index 35d4361b..3b253ed5 100644 --- a/Compilation/ILVerifier.cs +++ b/Compilation/ILVerifier.cs @@ -62,7 +62,24 @@ public List Verify(Stream assemblyStream) var argsStr = result.Args != null && result.Args.Length > 0 ? $" [{string.Join(", ", result.Args)}]" : ""; - errors.Add($"[IL Error] {typeName}.{methodName}: {result.Code}{argsStr} - {result.Message}"); + var diag = ""; + try + { + var ea = result.GetType().GetProperty("ErrorArguments")?.GetValue(result) as System.Collections.IEnumerable; + if (ea != null) + { + var parts = new List(); + foreach (var a in ea) + { + var n = a.GetType().GetProperty("Name")?.GetValue(a); + var val = a.GetType().GetProperty("Value")?.GetValue(a); + parts.Add($"{n}={val}"); + } + if (parts.Count > 0) diag = $" {{{string.Join(", ", parts)}}}"; + } + } + catch { } + errors.Add($"[IL Error] {typeName}.{methodName}: {result.Code}{argsStr}{diag} - {result.Message}"); } } catch (Exception ex) diff --git a/SharpTS.Tests/Compilation/EmitterSyncTests.cs b/SharpTS.Tests/Compilation/EmitterSyncTests.cs index 20367b40..69aa188f 100644 --- a/SharpTS.Tests/Compilation/EmitterSyncTests.cs +++ b/SharpTS.Tests/Compilation/EmitterSyncTests.cs @@ -105,6 +105,10 @@ public class EmitterSyncTests "EmitYield", // Core: yield value + suspend "EmitSuper", // This field indirection "EmitDynamicImport", // Dynamic import fallback + // #674: reject (with a clear error) an arrow that writes a variable captured from the + // generator scope — the generator SM has no function display class to share storage, so + // a by-value snapshot would silently drop the write. Read-only captures delegate to base. + "EmitArrowFunction", // --- #500: non-local exits must run an enclosing flag-based finally first --- "EmitBreak", // Route a break leaving a try through its finally(s) "EmitContinue", // Route a continue leaving a try through its finally(s) diff --git a/SharpTS.Tests/SharedTests/AsyncArrowCapturedWriteTests.cs b/SharpTS.Tests/SharedTests/AsyncArrowCapturedWriteTests.cs new file mode 100644 index 00000000..9864c5dc --- /dev/null +++ b/SharpTS.Tests/SharedTests/AsyncArrowCapturedWriteTests.cs @@ -0,0 +1,124 @@ +using SharpTS.Tests.Infrastructure; +using Xunit; + +namespace SharpTS.Tests.SharedTests; + +/// +/// An async arrow that WRITES a variable captured from its enclosing async function. The captured +/// local is promoted into the function's (reference-type) display class and the arrow mutates it +/// through `outer.functionDC.field`, because a boxed value-type state machine cannot be mutated in +/// place by verifiable IL — `unbox` yields a readonly managed pointer, so the old `unbox`+`stfld` +/// failed IL verification and could drop the write in complex state machines (#625). +/// +public class AsyncArrowCapturedWriteTests +{ + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void AsyncArrow_WritesCapturedVariable(ExecutionMode mode) + { + var source = """ + async function main() { + let n = 0; + const w = async () => { n = 5; }; + await w(); + console.log(n); + } + main(); + """; + + Assert.Equal("5\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void AsyncArrow_CompoundWriteToCapturedVariable(ExecutionMode mode) + { + var source = """ + async function main() { + let order = ""; + const w = async () => { order += "a"; }; + await w(); + order += "b"; + await w(); + console.log(order); + } + main(); + """; + + Assert.Equal("aba\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void AsyncArrow_WritesCapturedVariableAfterAwait(ExecutionMode mode) + { + var source = """ + async function main() { + let n = 0; + const w = async () => { n = await Promise.resolve(7); }; + await w(); + console.log(n); + } + main(); + """; + + Assert.Equal("7\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void AsyncArrow_MultipleCapturedWrites(ExecutionMode mode) + { + var source = """ + async function main() { + let a = 1, b = 2; + const w = async () => { a = a + b; b = a * 2; }; + await w(); + console.log(a + " " + b); + } + main(); + """; + + Assert.Equal("3 6\n", TestHarness.Run(source, mode)); + } + + [Fact] + public void AsyncArrow_WritesCapturedVariable_PassesILVerification() + { + // The whole point of #625: the emitted store must be VERIFIABLE, not merely JIT-accepted. + var source = """ + async function main() { + let n = 0; + const w = async () => { n = 5; }; + await w(); + console.log(n); + } + main(); + """; + + var (errors, output) = TestHarness.CompileVerifyAndRun(source); + + Assert.Empty(errors); + Assert.Equal("5\n", output); + } + + [Fact] + public void AsyncArrow_ReadOnlyCapture_StillPassesILVerification() + { + // Read-only captures keep using the state-machine field load path; guard against the #625 + // change regressing them. + var source = """ + async function main() { + let s = "hi"; + const r = async () => { console.log(s); }; + await r(); + } + main(); + """; + + var (errors, output) = TestHarness.CompileVerifyAndRun(source); + + Assert.Empty(errors); + Assert.Equal("hi\n", output); + } +} diff --git a/SharpTS.Tests/SharedTests/AsyncArrowNestedCaptureTests.cs b/SharpTS.Tests/SharedTests/AsyncArrowNestedCaptureTests.cs new file mode 100644 index 00000000..a402027c --- /dev/null +++ b/SharpTS.Tests/SharedTests/AsyncArrowNestedCaptureTests.cs @@ -0,0 +1,102 @@ +using SharpTS.Diagnostics.Exceptions; +using SharpTS.Tests.Infrastructure; +using Xunit; + +namespace SharpTS.Tests.SharedTests; + +/// +/// An async arrow nested directly inside a TOP-LEVEL (standalone) async arrow that captures a +/// variable from the enclosing arrow's scope. Compiled mode registers the inner arrow as its own +/// standalone state machine; #641 wires the single read-only capture through the stub's leading +/// capture argument (read from the enclosing frame, copied into the inner state machine). Harder +/// shapes — writing a capture, or capturing more than one variable — are rejected with a clear +/// compile error rather than miscompiled (#684). +/// +public class AsyncArrowNestedCaptureTests +{ + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void NestedAsyncArrow_ReadsSingleCapture(ExecutionMode mode) + { + var source = """ + const f = async () => { + const base = 100; + const x = await (async () => base + 5)(); + console.log(x); + }; + f(); + """; + + Assert.Equal("105\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void NestedAsyncArrow_ReadsCaptureAndOwnParameter(ExecutionMode mode) + { + var source = """ + const f = async () => { + const base = 100; + const inner = async (k: number) => base + k; + const x = await inner(5); + console.log(x); + }; + f(); + """; + + Assert.Equal("105\n", TestHarness.Run(source, mode)); + } + + [Fact] + public void NestedAsyncArrow_ReadsSingleCapture_PassesILVerification() + { + var source = """ + const f = async () => { + const base = 100; + const x = await (async () => base + 5)(); + console.log(x); + }; + f(); + """; + + var (errors, output) = TestHarness.CompileVerifyAndRun(source); + + Assert.Empty(errors); + Assert.Equal("105\n", output); + } + + [Fact] + public void NestedAsyncArrow_WritesCapture_RejectedInCompiledMode() + { + // Standalone arrows capture by value, so a write cannot propagate back. Compiled mode must + // reject this rather than silently dropping the write (#684); the interpreter is correct. + var source = """ + const f = async () => { + let n = 1; + const g = async () => { n = await (async () => n + 10)(); }; + await g(); + console.log(n); + }; + f(); + """; + + Assert.Equal("11\n", TestHarness.RunInterpreted(source)); + Assert.Throws(() => TestHarness.RunCompiled(source)); + } + + [Fact] + public void NestedAsyncArrow_MultipleCaptures_RejectedInCompiledMode() + { + var source = """ + const f = async () => { + const a = 10, b = 20, c = 30; + const inner = async () => a + b + c; + console.log(await inner()); + }; + f(); + """; + + Assert.Equal("60\n", TestHarness.RunInterpreted(source)); + Assert.Throws(() => TestHarness.RunCompiled(source)); + } +} diff --git a/SharpTS.Tests/SharedTests/GeneratorArrowBodyTests.cs b/SharpTS.Tests/SharedTests/GeneratorArrowBodyTests.cs new file mode 100644 index 00000000..ffc9e641 --- /dev/null +++ b/SharpTS.Tests/SharedTests/GeneratorArrowBodyTests.cs @@ -0,0 +1,153 @@ +using SharpTS.Diagnostics.Exceptions; +using SharpTS.Tests.Infrastructure; +using Xunit; + +namespace SharpTS.Tests.SharedTests; + +/// +/// Arrow / function-expression callbacks written INSIDE a generator (function*) body. +/// The compiled path previously failed to collect arrows nested in a yielded expression (so an +/// array-method callback compiled to a null "not callable" value) and emitted capturing arrows +/// with a null display-class target ("Non-static method requires a target"); a this used +/// only inside such an arrow left the generator's receiver field undefined (NRE). See #435 / #669. +/// The interpreter has always been correct. +/// +public class GeneratorArrowBodyTests +{ + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Generator_NonCapturingArrowCallbackInsideYield(ExecutionMode mode) + { + // #435: the arrow lives inside the yielded expression, so it was never collected → + // compiled `map` callback was null ("Array.prototype.map callback is not callable"). + var source = """ + function* g(): Generator { + const a = [1, 2, 3]; + yield "m=" + a.map(n => n * 2).join(","); + } + let s = ""; for (const v of g()) s += v; + console.log(s); + """; + + Assert.Equal("m=2,4,6\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Generator_CapturingClosureReadsLoopVariable(ExecutionMode mode) + { + // #669: per-iteration `for (let k …)` bindings captured by a closure created inside the + // generator body. Each closure must observe its own iteration's value (0,1,2). + var source = """ + function* gen() { + const fns: any[] = []; + for (let k = 0; k < 3; k++) { fns.push(() => k); } + let out = ""; + for (let i = 0; i < fns.length; i++) { out += fns[i](); } + yield out; + } + console.log(gen().next().value); + """; + + Assert.Equal("012\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Generator_ArrowReadsCapturedLocal(ExecutionMode mode) + { + var source = """ + function* g() { + const base = 10; + yield [1, 2, 3].map(x => x + base).join(","); + } + console.log(g().next().value); + """; + + Assert.Equal("11,12,13\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Generator_ArrowReadsCapturedParameter(ExecutionMode mode) + { + var source = """ + function* g(off: number) { + yield [1, 2, 3].map(x => x + off).join(","); + } + console.log(g(100).next().value); + """; + + Assert.Equal("101,102,103\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Generator_InstanceMethodArrowCapturesThis(ExecutionMode mode) + { + // #435/#669: `this` is referenced only inside the arrow, so the generator analyzer must + // still materialize the receiver (<>4__this) for the arrow's capture to be non-null. + var source = """ + class C { + v = 7; + *gen() { yield [1, 2, 3].map(x => x + this.v).join(","); } + } + console.log(new C().gen().next().value); + """; + + Assert.Equal("8,9,10\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Generator_ForEachMutatesCapturedObjectNotBinding(ExecutionMode mode) + { + // A callback that mutates the captured array OBJECT (push) — not the binding — is fine: + // the reference is shared, only the binding-write case (#674) is unsupported. + var source = """ + function* g() { + const acc: number[] = []; + [1, 2, 3].forEach(x => acc.push(x * 2)); + yield acc.join(","); + } + console.log(g().next().value); + """; + + Assert.Equal("2,4,6\n", TestHarness.Run(source, mode)); + } + + [Fact] + public void Generator_ForEachWritesCapturedBinding_InterpretedIsCorrect() + { + // The interpreter handles the write-to-captured-binding case correctly. + var source = """ + function* g() { + const a = [1, 2, 3]; let s = ""; + a.forEach(n => s += n); + yield s; + } + console.log(g().next().value); + """; + + Assert.Equal("123\n", TestHarness.RunInterpreted(source)); + } + + [Fact] + public void Generator_ForEachWritesCapturedBinding_CompiledRejectsClearly() + { + // Compiled mode cannot yet share storage for a mutated capture (would need a function-level + // display class threaded through the generator state machine, #674). Until then it must + // FAIL FAST with a clear message rather than silently dropping the write. + var source = """ + function* g() { + const a = [1, 2, 3]; let s = ""; + a.forEach(n => s += n); + yield s; + } + console.log(g().next().value); + """; + + var ex = Assert.Throws(() => TestHarness.RunCompiled(source)); + Assert.Contains("captured from the generator scope", ex.Message); + } +}