diff --git a/Compilation/AsyncGeneratorMoveNextEmitter.Statements.TryCatch.cs b/Compilation/AsyncGeneratorMoveNextEmitter.Statements.TryCatch.cs index 46cd19e6..545c95d6 100644 --- a/Compilation/AsyncGeneratorMoveNextEmitter.Statements.TryCatch.cs +++ b/Compilation/AsyncGeneratorMoveNextEmitter.Statements.TryCatch.cs @@ -28,7 +28,7 @@ private sealed class LoopScope : IExitScope { public required Label BreakLabel; public required Label ContinueLabel; - public string? LabelName; + public IReadOnlyList LabelNames = CompilationContext.NoLabels; // Lazily assigned the first time a break/continue to this loop must run an intervening // finally; identifies that exit in the `<>pendingExit` field. Unset while the loop's @@ -102,11 +102,11 @@ private FieldBuilder GetPendingReturnValueField() => // ---- Loop-scope methods (override the base stack to use `_exitScopes`) ----------------------- protected override void EnterLoop(Label breakLabel, Label continueLabel, string? labelName = null) - // Adopt a pending label (`outer: for (...)`) just like the sync generator emitter does, so a - // labeled `break`/`continue` can resolve this loop as its target. Without the fallback the - // LoopScope registers with LabelName == null and FindLabeledLoopScope never matches, making a - // labeled break/continue a silent no-op — the loop keeps iterating (#586). - => _exitScopes.Add(new LoopScope { BreakLabel = breakLabel, ContinueLabel = continueLabel, LabelName = labelName ?? Ctx.TakePendingLoopLabel() }); + // Adopt any pending labels (`outer: for (...)`, or a chain `a: b: for`) just like the sync + // generator emitter does, so a labeled `break`/`continue` can resolve this loop as its target. + // Without the fallback the LoopScope registers with no labels and FindLabeledLoopScope never + // matches, making a labeled break/continue a silent no-op — the loop keeps iterating (#586). + => _exitScopes.Add(new LoopScope { BreakLabel = breakLabel, ContinueLabel = continueLabel, LabelNames = labelName != null ? new[] { labelName } : Ctx.TakePendingLoopLabels() }); protected override void ExitLoop() { @@ -115,19 +115,19 @@ protected override void ExitLoop() _exitScopes.RemoveAt(_exitScopes.Count - 1); } - protected override (Label BreakLabel, Label ContinueLabel, string? LabelName)? CurrentLoop + protected override (Label BreakLabel, Label ContinueLabel, IReadOnlyList LabelNames)? CurrentLoop { get { var loop = CurrentLoopScope(); - return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelName); + return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelNames); } } - protected override (Label BreakLabel, Label ContinueLabel, string? LabelName)? FindLabeledLoop(string labelName) + protected override (Label BreakLabel, Label ContinueLabel, IReadOnlyList LabelNames)? FindLabeledLoop(string labelName) { var loop = FindLabeledLoopScope(labelName); - return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelName); + return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelNames); } private LoopScope? CurrentLoopScope() @@ -141,7 +141,7 @@ protected override (Label BreakLabel, Label ContinueLabel, string? LabelName)? F private LoopScope? FindLabeledLoopScope(string labelName) { for (int i = _exitScopes.Count - 1; i >= 0; i--) - if (_exitScopes[i] is LoopScope ls && ls.LabelName == labelName) + if (_exitScopes[i] is LoopScope ls && ls.LabelNames.Contains(labelName)) return ls; return null; } diff --git a/Compilation/CompilationContext.cs b/Compilation/CompilationContext.cs index 6114e674..25b77592 100644 --- a/Compilation/CompilationContext.cs +++ b/Compilation/CompilationContext.cs @@ -202,27 +202,36 @@ public partial class CompilationContext // Loop and Exception Block Control // ============================================ - // Loop control labels (with optional label name for labeled statements) - public Stack<(Label BreakLabel, Label ContinueLabel, string? LabelName)> LoopLabels { get; } = new(); + // Loop control labels. LabelNames carries every label a labeled break/continue can target — + // usually zero or one, but a chain like `a: b: for` hands the loop both, so `continue a` and + // `continue b` resolve to the same loop. Empty (NoLabels) for an unlabeled loop. + public Stack<(Label BreakLabel, Label ContinueLabel, IReadOnlyList LabelNames)> LoopLabels { get; } = new(); - /// - /// Label name awaiting attachment to the next loop's own break/continue targets. - /// Set by EmitLabeledStatement when the labeled statement is a loop, and consumed - /// by that loop's EnterLoop (here or in an emitter override). This lets - /// continue <label> branch to the loop's real continue point — a for-loop's - /// increment, a while's condition — instead of a point ahead of the loop's initializer, - /// which would re-run it forever (#558). - /// - public string? PendingLoopLabel { get; set; } + /// Shared empty label set for unlabeled loops (avoids per-loop allocation). + public static readonly IReadOnlyList NoLabels = []; + + // Labels parked by EmitLabeledStatement for the loop a chain of them directly wraps. The loop + // drains them all at entry via TakePendingLoopLabels and treats a continue/break to any of them + // as targeting itself, running the loop's own step (a for's increment, a while's re-test) rather + // than restarting it — restarting a `for` would re-run its initializer forever (#558/#580). + private readonly List _pendingLoopLabels = []; + + /// Parks a label for the next loop to adopt. A chain parks several before the loop. + public void AddPendingLoopLabel(string label) => _pendingLoopLabels.Add(label); + + /// Discards any parked labels the next loop didn't drain (defensive cleanup). + public void ClearPendingLoopLabels() => _pendingLoopLabels.Clear(); /// - /// Returns the pending labeled-loop name and clears it, so it attaches to exactly one loop. + /// Returns the labels parked for the loop now being entered, and clears them, so they attach to + /// exactly one loop. /// - public string? TakePendingLoopLabel() + public IReadOnlyList TakePendingLoopLabels() { - var label = PendingLoopLabel; - PendingLoopLabel = null; - return label; + if (_pendingLoopLabels.Count == 0) return NoLabels; + var labels = _pendingLoopLabels.ToArray(); + _pendingLoopLabels.Clear(); + return labels; } // Hoisted array type caches: stack of per-loop dictionaries mapping @@ -340,27 +349,38 @@ public void ClearParameters() public void EnterLoop(Label breakLabel, Label continueLabel, string? labelName = null) { - // An unlabeled EnterLoop call adopts any label parked by an enclosing labeled - // statement, so the loop's own continue/break targets carry the label (#558). - LoopLabels.Push((breakLabel, continueLabel, labelName ?? TakePendingLoopLabel())); + // An explicit label names this loop alone; otherwise the loop adopts whatever an enclosing + // labeled statement parked — a chain hands it several — so its own continue/break targets + // carry every label (#558/#580). + var labels = labelName != null ? new[] { labelName } : TakePendingLoopLabels(); + LoopLabels.Push((breakLabel, continueLabel, labels)); } + /// + /// Enters a loop carrying a pre-collected set of label names. Used where the labels are drained + /// once up front and handed to each of several alternative runtime paths (e.g. for-of's iterator + /// / index-based variants), so every path's break/continue targets resolve no matter which one + /// runs at runtime (#558). + /// + public void EnterLoop(Label breakLabel, Label continueLabel, IReadOnlyList labelNames) + => LoopLabels.Push((breakLabel, continueLabel, labelNames)); + public void ExitLoop() { LoopLabels.Pop(); } - public (Label BreakLabel, Label ContinueLabel, string? LabelName)? CurrentLoop => + public (Label BreakLabel, Label ContinueLabel, IReadOnlyList LabelNames)? CurrentLoop => LoopLabels.Count > 0 ? LoopLabels.Peek() : null; /// - /// Find a loop label by name (for labeled break/continue). + /// Find a loop scope that carries the given label name (for labeled break/continue). /// - public (Label BreakLabel, Label ContinueLabel, string? LabelName)? FindLabeledLoop(string labelName) + public (Label BreakLabel, Label ContinueLabel, IReadOnlyList LabelNames)? FindLabeledLoop(string labelName) { foreach (var entry in LoopLabels) { - if (entry.LabelName == labelName) + if (entry.LabelNames.Contains(labelName)) return entry; } return null; diff --git a/Compilation/ExpressionEmitterBase.CallHelpers.cs b/Compilation/ExpressionEmitterBase.CallHelpers.cs index a575b91e..2b8f9c4b 100644 --- a/Compilation/ExpressionEmitterBase.CallHelpers.cs +++ b/Compilation/ExpressionEmitterBase.CallHelpers.cs @@ -1097,34 +1097,107 @@ protected bool TryEmitOptionalChainMethodCall(Expr.Call c) IL.Emit(OpCodes.Isinst, Ctx.Runtime!.UndefinedType); IL.Emit(OpCodes.Brtrue, nullishLabel); - // Pre-spilled, already-boxed argument locals shared by the string fast path and the generic - // path below. Stays null unless an argument can suspend. - LocalBuilder[]? argLocals = null; - - // GetProperty can't resolve most string prototype methods (see - // EmitDynamicMethodCallPreservingThis) — keep its string fast path. - if (IsRuntimeDispatchableStringMethod(g.Name.Lexeme)) - { - // The string fast path and the generic path below both contain the arguments and are - // mutually exclusive at runtime. When an argument can suspend, spill the arguments once - // here — before the string-vs-generic split — so each await/yield is emitted exactly - // once; emitting them in both branches would desync the await-state counter (#614). The - // spill is reached only on the non-nullish receiver path, so the arguments stay - // unevaluated when the chain short-circuits. + bool stringMethod = IsRuntimeDispatchableStringMethod(g.Name.Lexeme); + + if (stringMethod && awaitSafe) + { + // A string-named method with a suspending argument needs the args resolved *after* the + // short-circuit decision yet emitted exactly once. Handled in its own block (#614/#627). + EmitOptionalChainStringMethodAwaitSafe(c, g, recvLocal, nullishLabel, endLabel); + } + else + { + // Pre-spilled, already-boxed argument locals shared by the string fast path and the + // generic path below. Stays null unless an argument can suspend. + LocalBuilder[]? argLocals = null; + + // GetProperty can't resolve most string prototype methods (see + // EmitDynamicMethodCallPreservingThis) — keep its string fast path. Without a suspending + // argument the args are emitted inline; the two paths are mutually exclusive at runtime, + // and with no await/yield, duplicating the (side-effect-free-to-the-counter) arg IL in + // both is harmless. The generic branch's fn-nullish check below still short-circuits + // before its inline args run, so a missing method never evaluates the args. + if (stringMethod) + { + var notStringLabel = IL.DefineLabel(); + IL.Emit(OpCodes.Ldloc, recvLocal); + IL.Emit(OpCodes.Isinst, Types.String); + IL.Emit(OpCodes.Brfalse, notStringLabel); + EmitRuntimeStringMethod(recvLocal, g.Name.Lexeme, c.Arguments, argLocals); + IL.Emit(OpCodes.Br, endLabel); + IL.MarkLabel(notStringLabel); + } + + // fn = GetProperty(recv, name); nullish fn short-circuits to undefined, + // matching the interpreter's HasOptionalInChain rule. + IL.Emit(OpCodes.Ldloc, recvLocal); + IL.Emit(OpCodes.Ldstr, g.Name.Lexeme); + IL.Emit(OpCodes.Call, Ctx.Runtime!.GetProperty); + var fnLocal = _helpers.SpillStoreObject(); + IL.Emit(OpCodes.Ldloc, fnLocal); + IL.Emit(OpCodes.Brfalse, nullishLabel); + IL.Emit(OpCodes.Ldloc, fnLocal); + IL.Emit(OpCodes.Isinst, Ctx.Runtime!.UndefinedType); + IL.Emit(OpCodes.Brtrue, nullishLabel); + + // InvokeMethodValue(recv, fn, args) — args evaluated only on the non-nullish path, per + // spec. For a non-string method spill the args here, after the method lookup, so an await + // in a later arg doesn't suspend with the receiver, fn, and the partially-built array all + // stacked (#439); this preserves the lookup-before-args evaluation order. if (awaitSafe) argLocals = c.Arguments.Select(SpillBoxed).ToArray(); - - var notStringLabel = IL.DefineLabel(); IL.Emit(OpCodes.Ldloc, recvLocal); - IL.Emit(OpCodes.Isinst, Types.String); - IL.Emit(OpCodes.Brfalse, notStringLabel); - EmitRuntimeStringMethod(recvLocal, g.Name.Lexeme, c.Arguments, argLocals); + IL.Emit(OpCodes.Ldloc, fnLocal); + EmitBoxedArgsArray(c.Arguments, argLocals); + IL.Emit(OpCodes.Call, Ctx.Runtime!.InvokeMethodValue); IL.Emit(OpCodes.Br, endLabel); - IL.MarkLabel(notStringLabel); } - // fn = GetProperty(recv, name); nullish fn short-circuits to undefined, - // matching the interpreter's HasOptionalInChain rule. + IL.MarkLabel(nullishLabel); + IL.Emit(OpCodes.Ldsfld, Ctx.Runtime!.UndefinedInstance); + + IL.MarkLabel(endLabel); + SetStackUnknown(); + return true; + } + + /// + /// Emits the optional-chain dispatch for a string-named method (substring, charAt, + /// …) when an argument can suspend (await/yield). Two constraints collide here: + /// + /// The string fast path (, no method-existence check — + /// the method always exists on a real string) and the generic GetProperty+ + /// InvokeMethodValue path are mutually exclusive at runtime, but the suspending argument + /// must be emitted exactly once; emitting it in both branches desyncs the await-state + /// counter and crashes the compiler (#614). + /// When the receiver is a non-string object that lacks the method, the chain short-circuits + /// to undefined and the argument must stay unevaluated — matching the interpreter, + /// which short-circuits before evaluating arguments (#627). + /// + /// So the dispatch is decided and the generic fn null-checked before any argument runs + /// (short-circuiting straight to when the method is missing), then + /// the argument(s) are evaluated once in a shared block both live dispatches reach. The receiver + /// kind is re-tested with isinst string after the evaluation rather than stashed in a flag, + /// because persists across the suspension but a plain bool local would + /// reset on MoveNext re-entry. + /// + /// Note: like the rest of this keeps SharpTS's + /// pre-existing HasOptionalInChain quirk — a missing method on a non-nullish receiver yields + /// undefined rather than throwing TypeError (ECMA-262 would evaluate the args and + /// throw). Both modes share that quirk; this only realigns when the args run. + /// + private void EmitOptionalChainStringMethodAwaitSafe(Expr.Call c, Expr.Get g, LocalBuilder recvLocal, Label nullishLabel, Label endLabel) + { + var evalArgsLabel = IL.DefineLabel(); + var genericDispatchLabel = IL.DefineLabel(); + + // String receiver → the method exists, so skip the lookup and go straight to arg eval. + IL.Emit(OpCodes.Ldloc, recvLocal); + IL.Emit(OpCodes.Isinst, Types.String); + IL.Emit(OpCodes.Brtrue, evalArgsLabel); + + // Non-string receiver: resolve the method generically; a nullish fn short-circuits to + // undefined WITHOUT evaluating the argument (the parity fix — #627). IL.Emit(OpCodes.Ldloc, recvLocal); IL.Emit(OpCodes.Ldstr, g.Name.Lexeme); IL.Emit(OpCodes.Call, Ctx.Runtime!.GetProperty); @@ -1135,25 +1208,25 @@ protected bool TryEmitOptionalChainMethodCall(Expr.Call c) IL.Emit(OpCodes.Isinst, Ctx.Runtime!.UndefinedType); IL.Emit(OpCodes.Brtrue, nullishLabel); - // InvokeMethodValue(recv, fn, args) — args evaluated only on the non-nullish path, per spec. - // For a non-string method (or when the string path didn't pre-spill) spill the args here, - // after the method lookup, so an await in a later arg doesn't suspend with the receiver, fn, - // and the partially-built array all stacked (#439); this preserves the lookup-before-args - // evaluation order. String methods reuse the locals pre-spilled above. - if (awaitSafe && argLocals == null) - argLocals = c.Arguments.Select(SpillBoxed).ToArray(); + // Shared block both live dispatches reach: evaluate the suspending argument(s) exactly once. + IL.MarkLabel(evalArgsLabel); + var argLocals = c.Arguments.Select(SpillBoxed).ToArray(); + + // Re-test the receiver kind (recvLocal survives the suspension) to pick the dispatch. + IL.Emit(OpCodes.Ldloc, recvLocal); + IL.Emit(OpCodes.Isinst, Types.String); + IL.Emit(OpCodes.Brfalse, genericDispatchLabel); + EmitRuntimeStringMethod(recvLocal, g.Name.Lexeme, c.Arguments, argLocals); + IL.Emit(OpCodes.Br, endLabel); + + // Non-string receiver with a real method: InvokeMethodValue(recv, fn, args). fnLocal was + // stored on the non-string path above (the only way to reach this dispatch at runtime). + IL.MarkLabel(genericDispatchLabel); IL.Emit(OpCodes.Ldloc, recvLocal); IL.Emit(OpCodes.Ldloc, fnLocal); EmitBoxedArgsArray(c.Arguments, argLocals); IL.Emit(OpCodes.Call, Ctx.Runtime!.InvokeMethodValue); IL.Emit(OpCodes.Br, endLabel); - - IL.MarkLabel(nullishLabel); - IL.Emit(OpCodes.Ldsfld, Ctx.Runtime!.UndefinedInstance); - - IL.MarkLabel(endLabel); - SetStackUnknown(); - return true; } /// diff --git a/Compilation/GeneratorMoveNextEmitter.Statements.TryCatch.cs b/Compilation/GeneratorMoveNextEmitter.Statements.TryCatch.cs index c128822a..712ced5d 100644 --- a/Compilation/GeneratorMoveNextEmitter.Statements.TryCatch.cs +++ b/Compilation/GeneratorMoveNextEmitter.Statements.TryCatch.cs @@ -28,7 +28,7 @@ private sealed class LoopScope : IExitScope { public required Label BreakLabel; public required Label ContinueLabel; - public string? LabelName; + public IReadOnlyList LabelNames = CompilationContext.NoLabels; // Lazily assigned the first time a break/continue to this loop must run an intervening // finally; identifies that exit in the `<>pendingExit` field. Unset while the loop's @@ -133,7 +133,7 @@ private FieldBuilder DefineExceptionPresentField() => // ---- Loop-scope methods (override the base stack to use `_exitScopes`) ----------------------- protected override void EnterLoop(Label breakLabel, Label continueLabel, string? labelName = null) - => _exitScopes.Add(new LoopScope { BreakLabel = breakLabel, ContinueLabel = continueLabel, LabelName = labelName ?? Ctx.TakePendingLoopLabel() }); + => _exitScopes.Add(new LoopScope { BreakLabel = breakLabel, ContinueLabel = continueLabel, LabelNames = labelName != null ? new[] { labelName } : Ctx.TakePendingLoopLabels() }); protected override void ExitLoop() { @@ -142,19 +142,19 @@ protected override void ExitLoop() _exitScopes.RemoveAt(_exitScopes.Count - 1); } - protected override (Label BreakLabel, Label ContinueLabel, string? LabelName)? CurrentLoop + protected override (Label BreakLabel, Label ContinueLabel, IReadOnlyList LabelNames)? CurrentLoop { get { var loop = CurrentLoopScope(); - return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelName); + return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelNames); } } - protected override (Label BreakLabel, Label ContinueLabel, string? LabelName)? FindLabeledLoop(string labelName) + protected override (Label BreakLabel, Label ContinueLabel, IReadOnlyList LabelNames)? FindLabeledLoop(string labelName) { var loop = FindLabeledLoopScope(labelName); - return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelName); + return loop == null ? null : (loop.BreakLabel, loop.ContinueLabel, loop.LabelNames); } private LoopScope? CurrentLoopScope() @@ -168,7 +168,7 @@ protected override (Label BreakLabel, Label ContinueLabel, string? LabelName)? F private LoopScope? FindLabeledLoopScope(string labelName) { for (int i = _exitScopes.Count - 1; i >= 0; i--) - if (_exitScopes[i] is LoopScope ls && ls.LabelName == labelName) + if (_exitScopes[i] is LoopScope ls && ls.LabelNames.Contains(labelName)) return ls; return null; } diff --git a/Compilation/ILEmitter.Statements.cs b/Compilation/ILEmitter.Statements.cs index abff9502..e6554632 100644 --- a/Compilation/ILEmitter.Statements.cs +++ b/Compilation/ILEmitter.Statements.cs @@ -488,10 +488,10 @@ protected override void EmitDoWhile(Stmt.DoWhile dw) protected override void EmitForOf(Stmt.ForOf f) { // A for-of emits several alternative runtime paths (iterator protocol, index-based, …), - // each registering its own loop scope. Capture any labeled-loop name once up front and - // hand it to every path, so `continue`/`break - private void EmitForOfArrayDirect(Stmt.ForOf f, LocalBuilder iterableLocal, ArrayElementsDescriptor desc, string? labelName = null) + private void EmitForOfArrayDirect(Stmt.ForOf f, LocalBuilder iterableLocal, ArrayElementsDescriptor desc, IReadOnlyList? labelNames = null) { var builder = _ctx.ILBuilder; var listType = desc.GetListType(_ctx.Types); @@ -771,7 +771,7 @@ private void EmitForOfArrayDirect(Stmt.ForOf f, LocalBuilder iterableLocal, Arra // Loop entry: listLocal holds the list. builder.MarkLabel(loopHeadLabel); - _ctx.EnterLoop(endLabel, continueLabel, labelName); + _ctx.EnterLoop(endLabel, continueLabel, labelNames ?? CompilationContext.NoLabels); // var i = 0 var indexLocal = IL.DeclareLocal(_ctx.Types.Int32); @@ -1299,31 +1299,33 @@ protected override void EmitContinue(Stmt.Continue c) protected override void EmitLabeledStatement(Stmt.LabeledStatement labeledStmt) { - string labelName = labeledStmt.Label.Lexeme; + // Look through a chain of labels (a: b: …) to whatever they ultimately wrap. + var inner = UnwrapLabelChain(labeledStmt, out var chainLabels); - if (IsLabelableLoop(labeledStmt.Statement)) + if (IsLabelableLoop(inner)) { - // Direct loop: park the label so the inner loop attaches it to its OWN break/continue - // targets (a for-loop's increment, a while's condition, …). Marking a continue label - // here — ahead of the for initializer — would re-run the initializer forever (#558). - _ctx.PendingLoopLabel = labelName; + // Direct (or chained) loop: park EVERY label in the chain so the inner loop attaches them + // all to its OWN break/continue targets (a for-loop's increment, a while's condition, …). + // Marking a continue label here — ahead of a for's initializer — would re-run it forever, + // and the outer label of a chain used to fall into exactly that path (#558/#580). + foreach (var label in chainLabels) + _ctx.AddPendingLoopLabel(label); try { - EmitStatement(labeledStmt.Statement); + EmitStatement(inner); } finally { - // The loop's EnterLoop consumes the label; clear it if somehow it didn't. - _ctx.PendingLoopLabel = null; + // The loop's EnterLoop drains the parked labels; clear any it somehow didn't. + _ctx.ClearPendingLoopLabels(); } return; } - // Non-loop labeled statement (a block, etc.) or a chained label (a: b: loop) whose inner - // labeled statement owns the loop. Mark the continue target before the statement: harmless - // for a block, and for a chained while/for-of/for-in/do-while it re-enters at the loop - // head. (A chained label on a `for` re-runs its initializer — a pre-existing limitation, - // not regressed here; single-label `for` continue is fixed above.) + // Non-loop labeled statement (a block, etc.). Mark the continue target before the statement + // (harmless for a block) and keep one wrapper scope per label by recursing through the chain; + // only `break