From 632eb2d35366f48005d60115dec23104e4aead61 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 6 Sep 2024 15:03:25 -0700 Subject: [PATCH 1/8] Allow yield inside of try / catch and catch This proposal expands the use of `yield` to - `try` portion of a `try / catch` block - `catch` portion of a `try / catch` block with some restrictions around `finally` --- proposals/yield-in-try-catch.md | 388 ++++++++++++++++++++++++++++++++ 1 file changed, 388 insertions(+) create mode 100644 proposals/yield-in-try-catch.md diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md new file mode 100644 index 0000000000..f6fc5bed46 --- /dev/null +++ b/proposals/yield-in-try-catch.md @@ -0,0 +1,388 @@ +# Permit yield in a try with a catch clause + +## Summary + +This proposal will allow `yield` statements to be written inside `try` and `catch` blocks. + +## Motivation + +The inability to use `yield` inside a `try` and `catch` block is a long standing pain point for customers. The restriction prevents the use of `yield` in a number of appealing scenarios. For example: + +```csharp +IEnumerable M1(IEnumerable col) +{ + try + { + foreach (var item in col) + { + var otherItem = Translate(item); + yield return otherItem; + } + } + catch (Exception ex) + { + throw new MoreSpecificException(ex); + } +} + +IEnumerable M2(IEnumerable col) +{ + foreach (var item in col) + { + try + { + var otherItem = Translate(item); + yield return otherItem; + } + catch (Exception ex) + { + Log(ex); + } + } +} +``` + +These restrictions exist in large part because it presented technical challenges for the native compiler and it wasn't a high enough priority item. The state machine support in Roslyn already supports the type of transforms necessary for this feature. + +This proposal will allow for the common cases of `yield` within `try` and `catch` without the awkward workarounds that are necessary today to move the `yield` outside the `try`. + +## Detailed Design + +### yield in try and catch + +The `yield` statement will be allowed inside `try` and `catch` blocks. The behavior will be the same as a `yield` statement today: + +- `yield return` will cause the method to suspend and return the value to the caller via `Current`. +- `yield break` will cause the iterator to return `false` from `MoveNext`. + +For example: + +```csharp +foreach (var e in Iterator()) +{ + Console.WriteLine(e); +} + +IEnumerable Iterator() +{ + try + { + yield return 1; + throw new Exception(""); + } + catch (Exception ex) + { + yield return 2; + } +} +``` + +This code will output: + +```cmd +1 +2 +``` + +The `catch` block will go through the same rewriting as an `await` inisde of a `catch` block. That will be observable when the `throw;` statement is used to rethrow an exception as it will reset the stack trace vs. perserving it (just as it is for an `async` method). + +The `yield` statement will not be allowed inside a `catch` when there is an associated or nested `finally` block. That would allow `yield` to be executed in the `Dispose` method which is not supported ([more details][catch-finally]). + +Detailed notes: + +- The `yield` statement will be allowed in a `try` block. +- The `yield` statement will be allowed in a `catch` block provided that: + - The `try` block does not contain a `finally` block. + - The `try` block does not have a nested `finally` block. + +### Dispose and finally + +A lesser known detail of iterators is that `finally` blocks can be executed as part of the `IDisposable.Dispose` implementation. The `Dispose` method has the same state machine implementation as the generated `MoveNext` except it only has the parts necessary for executing the `finally` blocks. That allows `Dispose` to _resume_ the method from the last suspend and execute the `finally` that were _active_ at the last suspend point. + +For example consider this code sample: + +```csharp +var e = Iterator().GetEnumerator(); +e.MoveNext(); +e.Dispose(); + +static T M(T t) => t; + +static IEnumerable Iterator() +{ + try + { + try + { + yield return M(1); + Console.WriteLine("After yield"); + } + finally + { + Console.WriteLine("Finally Inner"); + } + } + finally + { + Console.WriteLine("Finally Outer"); + } +} +``` + +This program will output: + +```cmd +Finally Inner +Finally Outer +``` + +The code generation for the `Dispose` method is meant to mirror the original `finally` structure as closely as possible. This includes execution of the code in the face of an exception during `Dispose`. This is achieved by refactoring the contents of the `finally` block into a method on the iterator and then having both `MoveNext` and `Dispose` generate the same `try / finally` structure and call into the methods. + +For example this is the `Dispose` method for the above iterator: + +```csharp +[DebuggerHidden] +void IDisposable.Dispose() +{ + int num = <>1__state; + if ((uint)(num - -4) > 1u && num != 1) + { + return; + } + try + { + if (num != -4 && num != 1) + { + return; + } + try + { + } + finally + { + <>m__Finally2(); + } + } + finally + { + <>m__Finally1(); + } +} +``` + +This behavior is important to understand when considering the code generation for `try / catch` blocks. + +### Code generation yield inside try with catch + +The code generation for iterators that have `yield` inside `try` blocks must preserve the same exception semantics for the `catch` in both `MoveNext` and `Dispose`. To achieve this the compiler will take a similar approach to what it does for `finally` blocks. + +For every `catch` block where the `try` has a nested `try / finally` with `yield`: + +1. The contents of the `catch` block will be generated into a parameterless method on the iterator with a `void` return. +2. The contents of the `when` clause will be generated into a parameterless method on the iterator with a `bool` return. +3. The `catch` block will be replaced with a call to the generated method in `MoveNext`. +4. The `when` clause will be replaced with a call to the generated method in `MoveNext`. +5. The `Dispose` method will mirror `catch` and `finally` blocks in the same way it mirrors `finally` blocks today and dispatch to the appropriate method + +The exception object will be lifted into a field just as any other local would be and accesses to it in the generated `catch` and `when` methods will use the field. In the case the language allowed `when` clauses the pattern variables would be lifted into fields as well. + +For example consider this code sample: + +```csharp +static IEnumerable Iterator() +{ + try + { + try + { + yield return M(1); + } + finally + { + Console.WriteLine("Finally Inner"); + } + } + catch (InvalidOperationException ex1) when (ex.Message.Contains("hello")) + { + Console.WriteLine("Catch1"); + } + catch (Exception ex2) + { + Console.WriteLine("Catch2"); + } + finally + { + Console.WriteLine("Finally Outer"); + } +} +static T M(T t) => t; +``` + +Will generate the following `Dispose` method: + +```csharp +[DebuggerHidden] +void IDisposable.Dispose() +{ + int num = <>1__state; + if ((uint)(num - -4) > 1u && num != 1) + { + return; + } + try + { + if (num != -4 && num != 1) + { + return; + } + try + { + } + finally + { + <>m__Finally2(); + } + } + catch (InvalidOperationException ex) when (<>1__ex1 = ex, <>m__When1()) + { + <>m__Catch1(); + } + catch (Exception ex) + { + <>1__ex2 = ex; + <>m__Catch1(); + } + finally + { + <>m__Finally1(); + } +} +``` + +The `<>1__ex1 = ex` in the `when` clause is not legal but the IL generated for the `when` will conceptually have this behavior. + +### Code generation yield inside try / catch in async iterators + +The code generation for `try / catch` blocks in async iterators will be largely the same as traditional iterators. The difference is that the return type of generated `catch` methods will be `ValueTask` instead of `void`. + +### Code generation yield inside catch + +The restrictions on the feature mean that `yield` inside a `catch` cannot be observed from a `finally` block. That means the code generation does not need to consider the impact on `Dispose` and can focus soley on `MoveNext`. + +Given that the code generation for `yield` inside `catch` will have the same structure as `await` inside of `catch`. Essentially the user written contents of the `catch` block will be moved outside the `catch`. The `catch` block will be replaced with saving the `Exception` object into the state machine and updating of the state variable to reflect execution is logically inside the catch block. + +For example consider this code sample: + +```csharp +IEnumerable M() +{ + try + { + M(); + } + catch (Exception ex) + { + Console.WriteLine("Catch1"); + yield return 1; + Console.WriteLine("Catch2"); + } + Console.WriteLine("Done"); +} +``` + +This would be generated as effectively: + +```csharp +bool MoveNext() +{ + switch (<>1__state) + { + case 0: + <>1__state = -1; + try + { + M(); + } + catch (Exception ex) + { + <>3__ex = ex; + <>1__state = 1; + } + + int num2 = <>1__state; + if (num2 != 1) + { + goto case 2; + } + + Console.WriteLine("Catch1"); + <>1__state = 1; + <>2__current = 1; + return true; + case 1: + <>1__state = -1; + Console.WriteLine("Catch2"); + goto case 2; + case 2: + Console.WriteLine("Done"); + return true; + default: + return false; + } +} +``` + +## Considerations + +### yield inside catch with nested finally + +[catch-finally]: #yield-inside-catch-with-nested-finally + +The `yield` statement cannot be reasonbly supported inside a `catch` blocks with a nested `finally` due to the behavior of the `Dispose` method. It is possible that a `catch` block will run as part of executing a `finally` in the `Dispose` method. + +For example consider the following: + +```csharp +var e = InCatchFinally(); +e.MoveNext(); +e.Dispose(); + +IEnumerable InCatchFinally() +{ + try + { + try + { + yield return 1; + } + finally + { + throw new Exception(); + } + } + catch + { + yield return 2; + } +} +``` + +This code would cause the statement `yield return 2` to be executed in the `Dispose` method. The state machine is not executing at this point hence it cannot be returned. Ignoring the statement would certainly be surprising the users. + +For these reasons this proposal will not support `yield` inside a `catch` block that is observable from a `finally`. + +### yield inside finally + +The `yield` statement inside a `finally` creates the same type of code generation issues as [catch with finally][catch-finally]. For that reason it was excluded from this proposal. + +## Open Issues + +### try only + +The proposal does allow `yield` inside of `catch` but it comes with a lot of caveats around `finally`. It's possible that this will lead to enoguh customer confusion that we should hold off on this until there is more demand for it. + +## Related Issues + +Related Items: + +- https://github.com/dotnet/csharplang/discussions/765 From 090ab503b8012b551d5dd21ded7f37bd1cc4cb82 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 9 Sep 2024 10:01:06 -0700 Subject: [PATCH 2/8] Apply suggestions from code review Co-authored-by: Jan Jones --- proposals/yield-in-try-catch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md index f6fc5bed46..b96b1a6529 100644 --- a/proposals/yield-in-try-catch.md +++ b/proposals/yield-in-try-catch.md @@ -84,7 +84,7 @@ This code will output: 2 ``` -The `catch` block will go through the same rewriting as an `await` inisde of a `catch` block. That will be observable when the `throw;` statement is used to rethrow an exception as it will reset the stack trace vs. perserving it (just as it is for an `async` method). +The `catch` block will go through the same rewriting as an `await` inside of a `catch` block. That will be observable when the `throw;` statement is used to rethrow an exception as it will reset the stack trace vs. preserving it (just as it is for an `async` method). The `yield` statement will not be allowed inside a `catch` when there is an associated or nested `finally` block. That would allow `yield` to be executed in the `Dispose` method which is not supported ([more details][catch-finally]). From 05ff13ed842dbaa3590c4bd7689307fed974f0c5 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 22 Oct 2024 16:53:46 -0700 Subject: [PATCH 3/8] feedback --- proposals/yield-in-try-catch.md | 295 ++++++++++++++++++++++---------- 1 file changed, 201 insertions(+), 94 deletions(-) diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md index b96b1a6529..df9fc81b8c 100644 --- a/proposals/yield-in-try-catch.md +++ b/proposals/yield-in-try-catch.md @@ -42,13 +42,13 @@ IEnumerable M2(IEnumerable col) } ``` -These restrictions exist in large part because it presented technical challenges for the native compiler and it wasn't a high enough priority item. The state machine support in Roslyn already supports the type of transforms necessary for this feature. +These restrictions exist in large part because it presented technical challenges for the native compiler and it wasn't a high enough priority item. That is no longer a blocker as the state machine in Rsolyn supports the types of transforms necessary to support this feature. There are still several semantic challenges to work through and this proposal has been written to address those. This proposal will allow for the common cases of `yield` within `try` and `catch` without the awkward workarounds that are necessary today to move the `yield` outside the `try`. ## Detailed Design -### yield in try and catch +### yield inside try and catch The `yield` statement will be allowed inside `try` and `catch` blocks. The behavior will be the same as a `yield` statement today: @@ -84,56 +84,55 @@ This code will output: 2 ``` -The `catch` block will go through the same rewriting as an `await` inside of a `catch` block. That will be observable when the `throw;` statement is used to rethrow an exception as it will reset the stack trace vs. preserving it (just as it is for an `async` method). - -The `yield` statement will not be allowed inside a `catch` when there is an associated or nested `finally` block. That would allow `yield` to be executed in the `Dispose` method which is not supported ([more details][catch-finally]). +The `yield` inside the `catch` block will go through the same rewriting as an `await` inside of a `catch` block. That will be observable when the `throw;` statement is used to rethrow an exception as it will reset the stack trace vs. preserving it (just as it is for an `async` method). Detailed notes: -- The `yield` statement will be allowed in a `try` block. -- The `yield` statement will be allowed in a `catch` block provided that: - - The `try` block does not contain a `finally` block. - - The `try` block does not have a nested `finally` block. +- The `yield` statement will be allowed in a `try / catch` block provided that: + - The `catch` block does not have a `when` clause. +- The `yield` statement will be allowed in a `catch` block -### Dispose and finally +### Dispose and finally in iterators -A lesser known detail of iterators is that `finally` blocks can be executed as part of the `IDisposable.Dispose` implementation. The `Dispose` method has the same state machine implementation as the generated `MoveNext` except it only has the parts necessary for executing the `finally` blocks. That allows `Dispose` to _resume_ the method from the last suspend and execute the `finally` that were _active_ at the last suspend point. +A lesser known detail of iterators is that `finally` blocks can be executed as part of the `IDisposable.Dispose` implementation. The `Dispose` method has the same state machine structure as the generated `MoveNext` except it only has the parts necessary for executing the `finally` blocks. That allows `Dispose` to _resume_ the method from the last suspend and execute the `finally` that were _active_ at the last suspend point. -For example consider this code sample: +The `Dispose` method _only_ executes the `finally` blocks and ignores all code inbetween them. For example consider this code sample: ```csharp -var e = Iterator().GetEnumerator(); +var e = M1().GetEnumerator(); e.MoveNext(); e.Dispose(); -static T M(T t) => t; - -static IEnumerable Iterator() +IEnumerable M1() { try { try { - yield return M(1); - Console.WriteLine("After yield"); + yield return 13; } finally { - Console.WriteLine("Finally Inner"); + Console.WriteLine("inner finally"); } + + Console.WriteLine("before yield return 1"); + yield return 1; + Console.WriteLine("after yield return 1"); } finally { - Console.WriteLine("Finally Outer"); + Console.WriteLine("outer finally"); } } + ``` This program will output: ```cmd -Finally Inner -Finally Outer +inner finally +outer finally ``` The code generation for the `Dispose` method is meant to mirror the original `finally` structure as closely as possible. This includes execution of the code in the face of an exception during `Dispose`. This is achieved by refactoring the contents of the `finally` block into a method on the iterator and then having both `MoveNext` and `Dispose` generate the same `try / finally` structure and call into the methods. @@ -172,100 +171,120 @@ void IDisposable.Dispose() This behavior is important to understand when considering the code generation for `try / catch` blocks. -### Code generation yield inside try with catch - -The code generation for iterators that have `yield` inside `try` blocks must preserve the same exception semantics for the `catch` in both `MoveNext` and `Dispose`. To achieve this the compiler will take a similar approach to what it does for `finally` blocks. - -For every `catch` block where the `try` has a nested `try / finally` with `yield`: - -1. The contents of the `catch` block will be generated into a parameterless method on the iterator with a `void` return. -2. The contents of the `when` clause will be generated into a parameterless method on the iterator with a `bool` return. -3. The `catch` block will be replaced with a call to the generated method in `MoveNext`. -4. The `when` clause will be replaced with a call to the generated method in `MoveNext`. -5. The `Dispose` method will mirror `catch` and `finally` blocks in the same way it mirrors `finally` blocks today and dispatch to the appropriate method - -The exception object will be lifted into a field just as any other local would be and accesses to it in the generated `catch` and `when` methods will use the field. In the case the language allowed `when` clauses the pattern variables would be lifted into fields as well. +### Code generation of yield inside try with catch in iterators -For example consider this code sample: +The code generation for iterators that have `yield` inside a `try / catch` will preserve the `catch` semantics in the `Dispose` method. That is _only_ the contents of the `finally` blocks will execute even in the case exceptions come into play. For example: ```csharp -static IEnumerable Iterator() +var e = M1(true).GetEnumerator(); +e.MoveNext(); +e.Dispose(); + +IEnumerable M1(bool b) { try { try { - yield return M(1); + try + { + yield return 13; + } + finally + { + Console.WriteLine("inner finally"); + if (b) + { + throw new Exception(); + } + } } - finally + catch { - Console.WriteLine("Finally Inner"); + Console.WriteLine("catch"); } - } - catch (InvalidOperationException ex1) when (ex.Message.Contains("hello")) - { - Console.WriteLine("Catch1"); - } - catch (Exception ex2) - { - Console.WriteLine("Catch2"); + + Console.WriteLine("after catch"); } finally { - Console.WriteLine("Finally Outer"); + Console.WriteLine("outer finally"); } } -static T M(T t) => t; ``` -Will generate the following `Dispose` method: +This program will output: + +```cmd +inner finally +outer finally +``` + +The `"after catch"` is not printed beacuse only the `finally` structure is mirrored in the `Dispose` method. The `catch`, like all other statements between `finally` is not included. statements in between the `catch` and `finally` are not executed in `Dispose`. This may seem odd at first glance but is leaning into the specified behavior for iterator `Dispose`. + +### Dispose and finally in async iterators + +The `DiposeAsync` behavior for async iterators mirrors that of traditional iterators in that it executes the `finally` blocks at the the point the state machine is suspended. However it does this by setting the `state` variable to a value that represents disposing and then calls `MoveNextAsync`. The implementation of `MoveNextAsync` will then execute only the `finally` blocks for the suspend point. The `DisposeAsync` method does not have a mirror copy of the `finally` blocks. + +### Code generation of yield inside try / catch in async iterators + +The code generation of async iterators will change such that `catch` blocks do not observably execute during `DisposeAsync`. To achieve this all `catch` blocks visible from a `yield` will rethrow exceptions if the state machine is in a disposing state. For example consider the following code:: ```csharp -[DebuggerHidden] -void IDisposable.Dispose() +var e = M(true).GetEnumerator(); +await e.MoveNext(); +await e.DisposeAsync(); + +async IAsyncEnumerable M(bool b) { - int num = <>1__state; - if ((uint)(num - -4) > 1u && num != 1) - { - return; - } try { - if (num != -4 && num != 1) - { - return; - } try { + yield return 13; + await Task.Yield(); } finally { - <>m__Finally2(); + Console.WriteLine("inner finally"); + if (b) + { + throw new Exception(); + } } } - catch (InvalidOperationException ex) when (<>1__ex1 = ex, <>m__When1()) + catch { - <>m__Catch1(); - } - catch (Exception ex) - { - <>1__ex2 = ex; - <>m__Catch1(); + Console.WriteLine("catch"); } finally { - <>m__Finally1(); + Console.WriteLine("outer finally"); } } ``` -The `<>1__ex1 = ex` in the `when` clause is not legal but the IL generated for the `when` will conceptually have this behavior. +This will output: -### Code generation yield inside try / catch in async iterators +```cmd +inner finally +outer finally +``` -The code generation for `try / catch` blocks in async iterators will be largely the same as traditional iterators. The difference is that the return type of generated `catch` methods will be `ValueTask` instead of `void`. +To achieve this the `catch` will be effectively rewritten as follows: + +```csharp +catch (Exception ex) +{ + if (<>1__state == /* dispatching state */) + { + throw; + } + Console.WriteLine("catch"); +} +``` -### Code generation yield inside catch +### Code generation of yield inside catch The restrictions on the feature mean that `yield` inside a `catch` cannot be observed from a `finally` block. That means the code generation does not need to consider the impact on `Dispose` and can focus soley on `MoveNext`. @@ -334,52 +353,140 @@ bool MoveNext() ## Considerations -### yield inside catch with nested finally - -[catch-finally]: #yield-inside-catch-with-nested-finally - -The `yield` statement cannot be reasonbly supported inside a `catch` blocks with a nested `finally` due to the behavior of the `Dispose` method. It is possible that a `catch` block will run as part of executing a `finally` in the `Dispose` method. +### yield inside finally -For example consider the following: +Consideration was given to supporting `yield` inside a `finally` block but this creates significant semantic challenges. Consider the following code as an example: ```csharp -var e = InCatchFinally(); +var e = M().GetEnumerator(); e.MoveNext(); e.Dispose(); -IEnumerable InCatchFinally() +void M() +{ + try + { + yield return 42; + } + finally + { + yield return 13; + } +} +``` + +The `finally` block would be executed in `Dispose` which means the language has to decide on the behavior of `yield` during `Dispose`. That could be modeled as: + +1. Throw an exception when `yield` is encountered in `Dispose`. +2. Silently ignore the value and continue executing the `finally` block without suspend. + +Neither of these seem like desirable outcomes and as such `yield` will not be allowed in `finally` blocks. + +## Open Issues + +### Preserve catch in Dispose paths + +The `Dispose` method could include mirroring both `catch` and `finally` blocks. This would allow the `catch` block to be executed in `Dispose` when `finally` blocks threw an exceptoin. The approach for this would be to do the followng. + +For every `catch` block where the `try` has a nested `try / finally` with `yield`: + +1. The contents of the `catch` block will be generated into a parameterless method on the iterator with a `void` return. +2. The contents of the `when` clause will be generated into a parameterless method on the iterator with a `bool` return. +3. The `catch` block will be replaced with a call to the generated method in `MoveNext`. +4. The `when` clause will be replaced with a call to the generated method in `MoveNext`. +5. The `Dispose` method will mirror `catch` and `finally` blocks in the same way it mirrors `finally` blocks today and dispatch to the appropriate method + +The exception object will be lifted into a field just as any other local would be and accesses to it in the generated `catch` and `when` methods will use the field. In the case the language allowed `when` clauses the pattern variables would be lifted into fields as well. + +For example consider this code sample: + +```csharp +static IEnumerable Iterator() { try { try { - yield return 1; + yield return M(1); } finally { - throw new Exception(); + Console.WriteLine("Finally Inner"); } } - catch + catch (InvalidOperationException ex1) when (ex.Message.Contains("hello")) { - yield return 2; + Console.WriteLine("Catch1"); + } + catch (Exception ex2) + { + Console.WriteLine("Catch2"); + } + finally + { + Console.WriteLine("Finally Outer"); } } +static T M(T t) => t; ``` -This code would cause the statement `yield return 2` to be executed in the `Dispose` method. The state machine is not executing at this point hence it cannot be returned. Ignoring the statement would certainly be surprising the users. +Will generate the following `Dispose` method: -For these reasons this proposal will not support `yield` inside a `catch` block that is observable from a `finally`. +```csharp +[DebuggerHidden] +void IDisposable.Dispose() +{ + int num = <>1__state; + if ((uint)(num - -4) > 1u && num != 1) + { + return; + } + try + { + if (num != -4 && num != 1) + { + return; + } + try + { + } + finally + { + <>m__Finally2(); + } + } + catch (InvalidOperationException ex) when (<>1__ex1 = ex, <>m__When1()) + { + <>m__Catch1(); + } + catch (Exception ex) + { + <>1__ex2 = ex; + <>m__Catch1(); + } + finally + { + <>m__Finally1(); + } +} +``` -### yield inside finally +The `<>1__ex1 = ex` in the `when` clause is not legal but the IL generated for the `when` will conceptually have this behavior. -The `yield` statement inside a `finally` creates the same type of code generation issues as [catch with finally][catch-finally]. For that reason it was excluded from this proposal. +This would add a bit of complexity the feature and it only produces observable differences when a `finally` block throws an exception on the `Dispose` path. That is likely a rare case. Further it potentially increases the complexity for developers. The `Dispose` method at first glance is likely unintuitive in that it only mirrors `finally` blocks and no other statements but that is also a very simple rule to learn. Preserving `catch` and `finally` and discussing how the code flows between them is potentially more complex. + +This also brings into question what happens when a `yield` occurs in a `catch` during `Dispose`. That cannot execute correctly as the state machine can't suspend during `Dispose`. To account for this the feature likely needs further restrictions like: + +- The `yield` statement will be allowed in a `catch` block provided that: + - The `try` block does not contain a `finally` block. + - The `try` block does not have a nested `finally` block. + +On the whole it seems simpler to not support `catch` in `Dispose` paths. + +## catch and when in iterators -## Open Issues -### try only -The proposal does allow `yield` inside of `catch` but it comes with a lot of caveats around `finally`. It's possible that this will lead to enoguh customer confusion that we should hold off on this until there is more demand for it. ## Related Issues From 788c3360dac40fbed269f3ecfd7c931bf6369765 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 22 Oct 2024 16:58:52 -0700 Subject: [PATCH 4/8] more --- proposals/yield-in-try-catch.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md index df9fc81b8c..de67e75139 100644 --- a/proposals/yield-in-try-catch.md +++ b/proposals/yield-in-try-catch.md @@ -88,8 +88,7 @@ The `yield` inside the `catch` block will go through the same rewriting as an `a Detailed notes: -- The `yield` statement will be allowed in a `try / catch` block provided that: - - The `catch` block does not have a `when` clause. +- The `yield` statement will be allowed in a `try / catch` block - The `yield` statement will be allowed in a `catch` block ### Dispose and finally in iterators @@ -284,6 +283,16 @@ catch (Exception ex) } ``` +The generator will also need to modify any `when` clauses on `catch` blocks to ensure they don't execute during `DisposeAsync`. To do this a prefix will be generated that returns `false` when the state machine is in a disposing state. For example: + +```csharp +// User code +catch (Exception ex) when (SomeMethod(ex)) + +// Generated code +catch (Exception ex) when (<>1__state == /* dispatching state */ ? false : SomeMethod(ex)) +``` + ### Code generation of yield inside catch The restrictions on the feature mean that `yield` inside a `catch` cannot be observed from a `finally` block. That means the code generation does not need to consider the impact on `Dispose` and can focus soley on `MoveNext`. @@ -483,11 +492,6 @@ This also brings into question what happens when a `yield` occurs in a `catch` d On the whole it seems simpler to not support `catch` in `Dispose` paths. -## catch and when in iterators - - - - ## Related Issues Related Items: From 976c0b7a9d7fa1560704671cd3423af476b5a4c3 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Wed, 23 Oct 2024 08:49:35 -0700 Subject: [PATCH 5/8] Apply suggestions from code review Co-authored-by: Jan Jones --- proposals/yield-in-try-catch.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md index de67e75139..fd69890171 100644 --- a/proposals/yield-in-try-catch.md +++ b/proposals/yield-in-try-catch.md @@ -42,7 +42,7 @@ IEnumerable M2(IEnumerable col) } ``` -These restrictions exist in large part because it presented technical challenges for the native compiler and it wasn't a high enough priority item. That is no longer a blocker as the state machine in Rsolyn supports the types of transforms necessary to support this feature. There are still several semantic challenges to work through and this proposal has been written to address those. +These restrictions exist in large part because it presented technical challenges for the native compiler and it wasn't a high enough priority item. That is no longer a blocker as the state machine in Roslyn supports the types of transforms necessary to support this feature. There are still several semantic challenges to work through and this proposal has been written to address those. This proposal will allow for the common cases of `yield` within `try` and `catch` without the awkward workarounds that are necessary today to move the `yield` outside the `try`. @@ -219,7 +219,7 @@ inner finally outer finally ``` -The `"after catch"` is not printed beacuse only the `finally` structure is mirrored in the `Dispose` method. The `catch`, like all other statements between `finally` is not included. statements in between the `catch` and `finally` are not executed in `Dispose`. This may seem odd at first glance but is leaning into the specified behavior for iterator `Dispose`. +The `"after catch"` is not printed beacuse only the `finally` structure is mirrored in the `Dispose` method. The `catch`, like all other statements between `finally` is not included. Statements in between the `catch` and `finally` are not executed in `Dispose`. This may seem odd at first glance but is leaning into the specified behavior for iterator `Dispose`. ### Dispose and finally in async iterators @@ -227,7 +227,7 @@ The `DiposeAsync` behavior for async iterators mirrors that of traditional itera ### Code generation of yield inside try / catch in async iterators -The code generation of async iterators will change such that `catch` blocks do not observably execute during `DisposeAsync`. To achieve this all `catch` blocks visible from a `yield` will rethrow exceptions if the state machine is in a disposing state. For example consider the following code:: +The code generation of async iterators will change such that `catch` blocks do not observably execute during `DisposeAsync`. To achieve this all `catch` blocks visible from a `yield` will rethrow exceptions if the state machine is in a disposing state. For example consider the following code: ```csharp var e = M(true).GetEnumerator(); @@ -275,7 +275,7 @@ To achieve this the `catch` will be effectively rewritten as follows: ```csharp catch (Exception ex) { - if (<>1__state == /* dispatching state */) + if (<>1__state == /* disposing state */) { throw; } @@ -290,7 +290,7 @@ The generator will also need to modify any `when` clauses on `catch` blocks to e catch (Exception ex) when (SomeMethod(ex)) // Generated code -catch (Exception ex) when (<>1__state == /* dispatching state */ ? false : SomeMethod(ex)) +catch (Exception ex) when (<>1__state == /* disposing state */ ? false : SomeMethod(ex)) ``` ### Code generation of yield inside catch @@ -395,7 +395,7 @@ Neither of these seem like desirable outcomes and as such `yield` will not be al ### Preserve catch in Dispose paths -The `Dispose` method could include mirroring both `catch` and `finally` blocks. This would allow the `catch` block to be executed in `Dispose` when `finally` blocks threw an exceptoin. The approach for this would be to do the followng. +The `Dispose` method could include mirroring both `catch` and `finally` blocks. This would allow the `catch` block to be executed in `Dispose` when `finally` blocks threw an exception. The approach for this would be to do the following. For every `catch` block where the `try` has a nested `try / finally` with `yield`: @@ -482,7 +482,7 @@ void IDisposable.Dispose() The `<>1__ex1 = ex` in the `when` clause is not legal but the IL generated for the `when` will conceptually have this behavior. -This would add a bit of complexity the feature and it only produces observable differences when a `finally` block throws an exception on the `Dispose` path. That is likely a rare case. Further it potentially increases the complexity for developers. The `Dispose` method at first glance is likely unintuitive in that it only mirrors `finally` blocks and no other statements but that is also a very simple rule to learn. Preserving `catch` and `finally` and discussing how the code flows between them is potentially more complex. +This would add a bit of complexity to the feature and it only produces observable differences when a `finally` block throws an exception on the `Dispose` path. That is likely a rare case. Further it potentially increases the complexity for developers. The `Dispose` method at first glance is likely unintuitive in that it only mirrors `finally` blocks and no other statements but that is also a very simple rule to learn. Preserving `catch` and `finally` and discussing how the code flows between them is potentially more complex. This also brings into question what happens when a `yield` occurs in a `catch` during `Dispose`. That cannot execute correctly as the state machine can't suspend during `Dispose`. To account for this the feature likely needs further restrictions like: From 7bf56ff181b9aafe24980df9f576f70569fe7e12 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Wed, 23 Oct 2024 08:56:30 -0700 Subject: [PATCH 6/8] more --- proposals/yield-in-try-catch.md | 1 + 1 file changed, 1 insertion(+) diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md index fd69890171..112a256173 100644 --- a/proposals/yield-in-try-catch.md +++ b/proposals/yield-in-try-catch.md @@ -217,6 +217,7 @@ This program will output: ```cmd inner finally outer finally +Unhandled exception. System.Exception: Exception of type 'System.Exception' was thrown ``` The `"after catch"` is not printed beacuse only the `finally` structure is mirrored in the `Dispose` method. The `catch`, like all other statements between `finally` is not included. Statements in between the `catch` and `finally` are not executed in `Dispose`. This may seem odd at first glance but is leaning into the specified behavior for iterator `Dispose`. From fbc3d9f2effa40037cc0b7c1527f0c5a34a35699 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 24 Oct 2024 14:19:13 -0700 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Julien Couvreur --- proposals/yield-in-try-catch.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md index 112a256173..6b0143aa91 100644 --- a/proposals/yield-in-try-catch.md +++ b/proposals/yield-in-try-catch.md @@ -95,7 +95,7 @@ Detailed notes: A lesser known detail of iterators is that `finally` blocks can be executed as part of the `IDisposable.Dispose` implementation. The `Dispose` method has the same state machine structure as the generated `MoveNext` except it only has the parts necessary for executing the `finally` blocks. That allows `Dispose` to _resume_ the method from the last suspend and execute the `finally` that were _active_ at the last suspend point. -The `Dispose` method _only_ executes the `finally` blocks and ignores all code inbetween them. For example consider this code sample: +The `Dispose` method _only_ executes the `finally` blocks and ignores all code between them. For example consider this code sample: ```csharp var e = M1().GetEnumerator(); @@ -296,7 +296,7 @@ catch (Exception ex) when (<>1__state == /* disposing state */ ? false : SomeMet ### Code generation of yield inside catch -The restrictions on the feature mean that `yield` inside a `catch` cannot be observed from a `finally` block. That means the code generation does not need to consider the impact on `Dispose` and can focus soley on `MoveNext`. +The restrictions on the feature mean that `yield` inside a `catch` cannot be observed from a `finally` block. That means the code generation does not need to consider the impact on `Dispose` and can focus solely on `MoveNext`. Given that the code generation for `yield` inside `catch` will have the same structure as `await` inside of `catch`. Essentially the user written contents of the `catch` block will be moved outside the `catch`. The `catch` block will be replaced with saving the `Exception` object into the state machine and updating of the state variable to reflect execution is logically inside the catch block. From 55b0784399e8a82220578d2074fd9537daecedd3 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 24 Oct 2024 14:21:42 -0700 Subject: [PATCH 8/8] pr feedback --- proposals/yield-in-try-catch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/yield-in-try-catch.md b/proposals/yield-in-try-catch.md index 6b0143aa91..e18cb79388 100644 --- a/proposals/yield-in-try-catch.md +++ b/proposals/yield-in-try-catch.md @@ -224,7 +224,7 @@ The `"after catch"` is not printed beacuse only the `finally` structure is mirro ### Dispose and finally in async iterators -The `DiposeAsync` behavior for async iterators mirrors that of traditional iterators in that it executes the `finally` blocks at the the point the state machine is suspended. However it does this by setting the `state` variable to a value that represents disposing and then calls `MoveNextAsync`. The implementation of `MoveNextAsync` will then execute only the `finally` blocks for the suspend point. The `DisposeAsync` method does not have a mirror copy of the `finally` blocks. +The `DiposeAsync` behavior for async iterators mirrors that of traditional iterators in that it executes the `finally` blocks at the the point the state machine is suspended. However it does this by setting the `disposeMode` variable and then calls `MoveNextAsync`. The implementation of `MoveNextAsync` will then execute only the `finally` blocks for the suspend point. The `DisposeAsync` method does not have a mirror copy of the `finally` blocks. ### Code generation of yield inside try / catch in async iterators