Fix rethrowing in nested async catch #71578
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
| return result; | ||
| } | ||
|
|
||
| // We cannot get here from a catch without awaits. | ||
| Debug.Assert(!_inCatchWithoutAwaits); |
There was a problem hiding this comment.
Are we certain this is the case? What if a catch without awaits contains another try/catch that uses awaits? The outer catch doesn't use await directly, but the nested one does. Is this assert going to fail when we reach that nested catch with awaits? Are we testing a scenario like that? #Closed
There was a problem hiding this comment.
It looks like NestedRethrow_02 covers this scenario
There was a problem hiding this comment.
Yes, the "await analysis" for the outer catch visits the whole block including the nested catches, I think, so it will report any outer catch to contain awaits whenever any of its nested catches contain an await. Hence we cannot have a catch without awaits and nested inside it a catch with awaits.
|
LGTM (commit 1) |
|
@dotnet/roslyn-compiler for a second review, thanks |
|
@jcouv PTAL as second reviewer |
|
@jcouv @dotnet/roslyn-compiler for a second review, thanks |
| @@ -518,11 +519,18 @@ private BoundNode VisitCatchBlock(BoundCatchBlock node, AwaitCatchFrame parentAw | |||
| var origCurrentAwaitCatchFrame = _currentAwaitCatchFrame; | |||
| _currentAwaitCatchFrame = parentAwaitCatchFrame; | |||
|
|
|||
| var origInCatchWithoutAwaits = _inCatchWithoutAwaits; | |||
| _inCatchWithoutAwaits = true; | |||
There was a problem hiding this comment.
Not directly related to this PR: There are indirect ways of introducing an await in code. For instance, await foreach and await using.
I believe they get lowered to simple await in local rewriter, so should be registered properly as awaits here (CatchContainsAwait). But it may be good to check or test.
ghost
added this to the 
* Fix rethrowing in nested async catch * Test await using and foreach
Fixes #71569.
The async catch rewriter was handling each rethrow via
_currentAwaitCatchFramebut that might not be the current catch frame, that's only the nearest catch frame with awaits.