Fix #836: iterator lowering supports try/finally around yield - #846
Merged
Conversation
…in iterators (#836) 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Iterator (
yield) bodies now bind and lower correctly withtry { … } finally { … }aroundyield, and the C#-forbiddentry { yield … } catch (…) { … }combination is rejected with a clean diagnostic (GS0367). Closes the#806workaround in theGsharp.Extensions.Sequencesstdlib.Root cause
The end-to-end iterator lowering for
try/finallywas already implemented inIteratorMoveNextBodyBuilder+IteratorTryDispatchPlanner(landed under #419). What was missing was the binder-level rejection oftry/catcharoundyield— the previous behaviour silently dropped the catch clause during lowering, which violates ECMA-335 §I.12 / C# §15.14 (yield cannot resume into a protected region that doubles as a CLR exception-handler frame).The
Gsharp.Extensions.Sequencesstdlib then had to flattenWindowedIterator/IndexedIterator(the C# baseline shape usedenumerator + try/finally + Dispose) when ported during #806, because there was no positive signal that try/finally + yield actually worked.Fix
Binder (
StatementBinder.BindTryStatement):yieldlexically inside atryblock coincides with one or morecatchclauses on the sametry. Fires once per offending yield.FunctionLiteralExpressionbodies — yields inside nested lambdas do not trip the rejection.SDK (
Gsharp.Extensions.Sequences.SequenceExtensions):WindowedIterator[T]andIndexedIterator[T]restored to the explicitGetEnumerator + try/finally + Disposeshape that matches the C# baseline.InterleaveIterator'susing letcomment refreshed.Tests
Issue836IteratorTryFinallyBinderTests): try/finally accepted; try/catch rejected; try/catch/finally rejected; nested try/finally accepted; inner-catch-with-yield rejected; non-iterator try/catch unaffected; nested-lambda yield inside outer try-with-only-finally accepted.Issue836IteratorTryFinallyEmitTests): full enumeration runs the finally exactly once; early break runs the finally on Dispose; nested early-break runs inner-then-outer finally; finally runs exactly once on normal completion. All passdotnet-ilverify(net10.0).Issue836IteratorTryFinallyInterpreterTests): tree-walking interpreter parity for try/finally + yield, including nested.IteratorTryFinallyEmitTests.Iterator_YieldInsideTryCatchFinally_*repurposed to assert GS0367 fires (previously asserted the catch was silently dropped).Full
dotnet test GSharp.slnsuite green (5345 tests). Website (npm run build) builds cleanly with no broken links. ilverify clean on every emit fixture.Follow-ups
None deferred — the implemented scope covers the issue requirements:
Gsharp.Extensions.Sequencesstdlib uses the restored shape.Refs
Closes #836
Related #806, ADR-0084 §L5, parent #706