fix(binder,sdk): reject yield-in-try-with-catch; restore try/finally in iterators (#836) (#846)

Iterator (yield) bodies in G# already support try/finally lowering
end-to-end (IteratorMoveNextBodyBuilder + IteratorTryDispatchPlanner,
landed under #419). What was missing was the binder-side rejection of
the C#-forbidden combination 'try { yield … } catch (…) { … }' and the
follow-through in the Gsharp.Extensions.Sequences stdlib, where #806
had to work around the missing diagnostic by hoisting cleanup outside
the iterator.

Binder:
* GS0367 (new): a yield lexically inside a try block that also has any
  catch clause is rejected (C# §15.14 / ECMA-335). The diagnostic fires
  once per offending yield, with the yield keyword's location. Pure
  try/finally around yield continues to bind cleanly; the iterator
  rewriter's existing 'drop catch clauses' fallback now becomes
  defense-in-depth.
* The walker descends only the try block (not catches/finallies, not
  nested FunctionLiteralExpression bodies), so a yield inside a nested
  lambda inside the try does not trip the diagnostic.

SDK:
* WindowedIterator[T] and IndexedIterator[T] in
  Gsharp.Extensions.Sequences now use the explicit
  GetEnumerator + try/finally + Dispose shape, matching the C#
  baseline that #806 had to flatten. Their tests (104 in
  Extensions.Tests) continue to pass; consumers that break out early
  now observe deterministic enumerator disposal.
* The InterleaveIterator comment is refreshed — using-let stays as
  the declarative shape but is no longer the only option.

Tests:
* Issue836IteratorTryFinallyBinderTests (Core.Tests): 7 binder cases
  covering try/finally accepted, try/catch rejected, try/catch/finally
  rejected, nested try/finally accepted, inner-catch-with-yield
  rejected, non-iterator try/catch unaffected, and yield-in-nested-
  lambda-inside-outer-try-with-only-finally accepted.
* Issue836IteratorTryFinallyEmitTests (Compiler.Tests): 4 end-to-end
  emit + ilverify-checked tests — full enumeration runs the finally
  exactly once, early break runs the finally on Dispose, nested
  early-break runs inner then outer finally.
* Issue836IteratorTryFinallyInterpreterTests (Interpreter.Tests): 3
  tree-walking interpreter parity tests for try/finally + yield;
  asserts every value is observed and each finally runs exactly once.
* IteratorTryFinallyEmitTests.Iterator_YieldInsideTryCatchFinally_*
  is repurposed to assert GS0367 fires (previously asserted the catch
  was silently dropped).

Full GSharp.sln test suite (5345 tests) green; dotnet-ilverify clean
on the new emit fixtures; website builds without broken links.

Closes #836
Related #806, ADR-0084 §L5, parent #706

Co-authored-by: Copilot CLI <noreply@github.com>

Author: DavidObando

src/Core/CodeAnalysis/Binding/StatementBinder.cs

@@ -2195,9 +2195,64 @@ private BoundStatement BindTryStatement(TryStatementSyntax syntax)
             return BindErrorStatement();
         }
 
+        // Issue #836: a `yield` lexically inside a `try` block that also
+        // has any `catch` clause is rejected (C# §15.14 / ECMA-335). The
+        // iterator state machine cannot safely resume into a protected
+        // region from a synthesized dispatch when that region also acts
+        // as a CLR exception handler frame. Pure `try`/`finally` around
+        // `yield` is supported and lowered by IteratorMoveNextBodyBuilder.
+        if (catches.Count > 0
+            && function != null
+            && isIteratorReturnType(function.Type)
+            && YieldFinder.ContainsYieldInOwnTryBlock(tryBlock))
+        {
+            foreach (var yieldLocation in YieldFinder.GetYieldLocationsInOwnTryBlock(tryBlock))
+            {
+                Diagnostics.ReportYieldInsideTryWithCatch(yieldLocation);
+            }
+        }
+
         return new BoundTryStatement(syntax, tryBlock, catches.ToImmutable(), finallyBlock);
     }
 
+    /// <summary>
+    /// Walker that locates <c>yield</c> statements lexically inside a
+    /// bound block, but does not descend into nested function bodies
+    /// (lambdas / local functions). Issue #836.
+    /// </summary>
+    private sealed class YieldFinder : BoundTreeWalker
+    {
+        private readonly List<TextLocation> locations = new List<TextLocation>();
+
+        public static bool ContainsYieldInOwnTryBlock(BoundStatement tryBlock)
+        {
+            var walker = new YieldFinder();
+            walker.VisitStatement(tryBlock);
+            return walker.locations.Count > 0;
+        }
+
+        public static IReadOnlyList<TextLocation> GetYieldLocationsInOwnTryBlock(BoundStatement tryBlock)
+        {
+            var walker = new YieldFinder();
+            walker.VisitStatement(tryBlock);
+            return walker.locations;
+        }
+
+        protected override void VisitYieldStatement(BoundYieldStatement node)
+        {
+            // Prefer the `yield` keyword's location; fall back to the
+            // full statement syntax location if available.
+            if (node.Syntax is YieldStatementSyntax yieldSyntax)
+            {
+                this.locations.Add(yieldSyntax.YieldKeyword.Location);
+            }
+            else if (node.Syntax != null)
+            {
+                this.locations.Add(node.Syntax.Location);
+            }
+        }
+    }
+
     private BoundStatement BindThrowStatement(ThrowStatementSyntax syntax)
     {
         var expression = bindExpression(syntax.Expression);

src/Core/CodeAnalysis/DiagnosticBag.cs

@@ -3722,6 +3722,27 @@ public void ReportBaseInterfaceCallTargetsPrivateHelper(
             DiagnosticSeverity.Error);
     }
 
+    /// <summary>
+    /// Reports GS0367 — issue #836: a <c>yield</c> statement appears
+    /// lexically inside a <c>try</c> block that also has one or more
+    /// <c>catch</c> clauses. The C# spec (§15.14) and ECMA-335 forbid
+    /// this combination because the iterator state machine cannot
+    /// safely resume into a protected region from a synthesized
+    /// dispatch. Wrap the <c>yield</c> in a separate <c>try</c>/
+    /// <c>finally</c> instead, or move the exception-handling block to
+    /// the consumer (<c>for v in iter()</c>) side.
+    /// </summary>
+    /// <param name="location">The source location of the offending
+    /// <c>yield</c> keyword.</param>
+    public void ReportYieldInsideTryWithCatch(TextLocation location)
+    {
+        Report(
+            location,
+            "GS0367",
+            "'yield' cannot appear inside a 'try' block that has a 'catch' clause; only 'try'/'finally' is supported around 'yield' (issue #836).",
+            DiagnosticSeverity.Error);
+    }
+
     private static string FormatMissingNames(IEnumerable<string> missingNames)
     {
         var displayed = new List<string>();

src/Sdk/Gsharp.Extensions/Sequences/SequenceExtensions.gs

@@ -195,13 +195,21 @@ func (source IEnumerable[T]) ToMap[T, TKey, TValue](keyFn (T) -> TKey, valueFn (
 // ====================================================================
 
 func WindowedIterator[T](source IEnumerable[T], size int32) IEnumerable[IList[T]] {
-    var buffer = Queue[T](size)
-    for item in source {
-        buffer.Enqueue(item)
-        if buffer.Count == size {
-            yield List[T](buffer)
-            buffer.Dequeue()
+    // Issue #836: with try/finally + yield supported, eagerly grab the
+    // source enumerator and guarantee its disposal even when the
+    // consumer breaks early. Matches the C# baseline shape.
+    let enumerator = source.GetEnumerator()
+    try {
+        var buffer = Queue[T](size)
+        while enumerator.MoveNext() {
+            buffer.Enqueue(enumerator.Current)
+            if buffer.Count == size {
+                yield List[T](buffer)
+                buffer.Dequeue()
+            }
         }
+    } finally {
+        enumerator.Dispose()
     }
 }
 
@@ -222,10 +230,17 @@ func ChunkedIterator[T](source IEnumerable[T], size int32) IEnumerable[IList[T]]
 }
 
 func IndexedIterator[T](source IEnumerable[T]) IEnumerable[(int32, T)] {
-    var i = 0
-    for item in source {
-        yield (i, item)
-        i++
+    // Issue #836: explicit enumerator + try/finally guarantees
+    // disposal on early termination, matching the C# baseline.
+    let enumerator = source.GetEnumerator()
+    try {
+        var i = 0
+        while enumerator.MoveNext() {
+            yield (i, enumerator.Current)
+            i++
+        }
+    } finally {
+        enumerator.Dispose()
     }
 }
 
@@ -243,10 +258,11 @@ func PairwiseIterator[T](source IEnumerable[T]) IEnumerable[(T, T)] {
 }
 
 func InterleaveIterator[T](source IEnumerable[T], other IEnumerable[T]) IEnumerable[T] {
-    // Iterator blocks can't host a `try-finally` wrap around `yield`,
-    // so we use `using let` to bind each enumerator — the iterator
-    // lowering disposes them in the resulting state machine's
-    // FaultBlock.
+    // The `using let` form binds each enumerator's lifetime to the
+    // iterator block — the iterator lowering disposes them when the
+    // state machine's Dispose path runs. Issue #836 also unlocked an
+    // explicit `try { … } finally { … }` shape for this scenario, but
+    // `using let` keeps the intent declarative.
     using let left = source.GetEnumerator()
     using let right = other.GetEnumerator()
 

test/Compiler.Tests/Emit/Issue836IteratorTryFinallyEmitTests.cs

@@ -0,0 +1,220 @@
+// <copyright file="Issue836IteratorTryFinallyEmitTests.cs" company="GSharp">
+// Copyright (C) GSharp Authors. All rights reserved.
+// </copyright>
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Reflection;
+using Xunit;
+
+namespace GSharp.Compiler.Tests.Emit;
+
+/// <summary>
+/// Issue #836 — end-to-end emit + IL-verify coverage for iterator
+/// state machines whose user body contains <c>try</c>/<c>finally</c>
+/// around <c>yield</c>. Asserts both the runtime behaviour (full
+/// enumeration runs the finally once, early-break runs the finally
+/// during Dispose) and that the synthesized state machine passes
+/// <c>dotnet-ilverify</c> on net10.0.
+/// </summary>
+public class Issue836IteratorTryFinallyEmitTests
+{
+    [Fact]
+    public void Iterator_TryFinally_FullEnumeration_RunsFinallyOnce()
+    {
+        var source = """
+            package Probe
+            import System
+            import System.Collections.Generic
+
+            func gen() IEnumerable[int32] {
+                try {
+                    yield 10
+                    yield 20
+                } finally {
+                    Console.WriteLine("dispose")
+                }
+            }
+
+            public var sum = 0
+            public var disposeMarker = ""
+            for v in gen() {
+                sum = sum + v
+            }
+            disposeMarker = "ok"
+            """;
+
+        var assembly = CompileAndRun(source);
+        Assert.Equal(30, GetIntField(assembly, "sum"));
+        Assert.Equal("ok", GetStringField(assembly, "disposeMarker"));
+    }
+
+    [Fact]
+    public void Iterator_TryFinally_EarlyBreak_RunsFinallyOnDispose()
+    {
+        var source = """
+            package Probe
+            import System
+            import System.Collections.Generic
+
+            public var finallyRan = 0
+            public var seen = 0
+
+            func gen() IEnumerable[int32] {
+                try {
+                    yield 1
+                    yield 2
+                    yield 3
+                } finally {
+                    finallyRan = finallyRan + 1
+                }
+            }
+
+            for v in gen() {
+                seen = seen + 1
+                if v == 1 {
+                    break
+                }
+            }
+            """;
+
+        var assembly = CompileAndRun(source);
+        Assert.Equal(1, GetIntField(assembly, "seen"));
+        Assert.Equal(1, GetIntField(assembly, "finallyRan"));
+    }
+
+    [Fact]
+    public void Iterator_NestedTryFinally_EarlyBreak_RunsInnerThenOuter()
+    {
+        var source = """
+            package Probe
+            import System
+            import System.Collections.Generic
+
+            public var trace = ""
+
+            func gen() IEnumerable[int32] {
+                try {
+                    try {
+                        yield 1
+                        yield 2
+                    } finally {
+                        trace = trace + "I"
+                    }
+                } finally {
+                    trace = trace + "O"
+                }
+            }
+
+            for v in gen() {
+                if v == 1 {
+                    break
+                }
+            }
+            """;
+
+        var assembly = CompileAndRun(source);
+        Assert.Equal("IO", GetStringField(assembly, "trace"));
+    }
+
+    [Fact]
+    public void Iterator_TryFinally_FullEnumeration_FinallyExactlyOnce()
+    {
+        var source = """
+            package Probe
+            import System
+            import System.Collections.Generic
+
+            public var finallyRan = 0
+
+            func gen() IEnumerable[int32] {
+                try {
+                    yield 1
+                    yield 2
+                    yield 3
+                } finally {
+                    finallyRan = finallyRan + 1
+                }
+            }
+
+            for v in gen() {
+                // consume
+            }
+            """;
+
+        var assembly = CompileAndRun(source);
+        Assert.Equal(1, GetIntField(assembly, "finallyRan"));
+    }
+
+    #region Helpers
+
+    private static Assembly CompileAndRun(string source)
+    {
+        var outPath = CompileToFile(source, target: "exe");
+
+        var bytes = File.ReadAllBytes(outPath);
+        var assembly = Assembly.Load(bytes);
+
+        var program = assembly.GetTypes().Single(t => t.Name == "<Program>");
+        var entry = program.GetMethod("<Main>$", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static);
+        entry!.Invoke(null, entry.GetParameters().Length == 0 ? null : new object[] { Array.Empty<string>() });
+
+        return assembly;
+    }
+
+    private static string CompileToFile(string source, string target)
+    {
+        var tempDir = Directory.CreateTempSubdirectory("gs_836_").FullName;
+        var srcPath = Path.Combine(tempDir, "test.gs");
+        var outPath = Path.Combine(tempDir, "test.dll");
+        File.WriteAllText(srcPath, source);
+
+        using var compileOut = new StringWriter();
+        using var compileErr = new StringWriter();
+        var prevOut = Console.Out;
+        var prevErr = Console.Error;
+        Console.SetOut(compileOut);
+        Console.SetError(compileErr);
+        int compileExit;
+        try
+        {
+            compileExit = Program.Main(new[]
+            {
+                "/out:" + outPath,
+                "/target:" + target,
+                "/targetframework:net10.0",
+                srcPath,
+            });
+        }
+        finally
+        {
+            Console.SetOut(prevOut);
+            Console.SetError(prevErr);
+        }
+
+        Assert.True(
+            compileExit == 0,
+            $"gsc failed:\nstdout:\n{compileOut}\nstderr:\n{compileErr}");
+
+        IlVerifier.Verify(outPath);
+        return outPath;
+    }
+
+    private static int GetIntField(Assembly assembly, string name)
+    {
+        var program = assembly.GetTypes().Single(t => t.Name == "<Program>");
+        var field = program.GetField(name, BindingFlags.Public | BindingFlags.Static);
+        return (int)field!.GetValue(null)!;
+    }
+
+    private static string GetStringField(Assembly assembly, string name)
+    {
+        var program = assembly.GetTypes().Single(t => t.Name == "<Program>");
+        var field = program.GetField(name, BindingFlags.Public | BindingFlags.Static);
+        return (string)field!.GetValue(null)!;
+    }
+
+    #endregion
+}