runtime-async: pretty-test gaps in exception/try-finally rewriting

[siegfriedpammer]

Exploratory testing of the runtime-async branch surfaced several gaps in the RuntimeAsync* pretty-test pipeline. The full Async / RuntimeAsync test set is green today, but the matchers introduced in RuntimeAsyncExceptionRewriteTransform are narrowly fitted to the shapes the existing fixtures exercise. Patterns just outside that envelope produce raw lowered IL in the decompiled output.

All findings reproduce on the runtime-async branch (post-review HEAD). They are runtime-async specific (corresponding state-machine Async tests pass), except where noted.

Cluster 1 — Flag-based early-return matcher (TryRewriteFlagBasedEarlyReturn)

The matcher is too narrowly fitted. Four distinct failing shapes:

1. Direct return X; inside try-finally with await in finally.
   The matcher requires parentBlock.Instructions[tryFinallyIdx + 1] is Branch br (i.e. a br checkBlock to a dedicated check-block). In the simplest case the compiler emits the flag check inline: directly after the TryFinally, parentBlock has if (flag == 1) leave function (capture); throw null (the IfInstruction is the next sibling, not preceded by a br).

   public async Task<int> ReturnFromTryFinally()
   {
       try { return 42; }
       finally { await Task.CompletedTask; }
   }

   Decompiled output retains int num; int result; ...; if (num == 1) return result; throw null;.

2. Void return; — same gate, value-less form. Same diagnosis as 1.

3. Nested try-finally with return in inner try.
   The flag-setter block in the outer try is no longer 2 instructions (stloc flag(K); leave try) — it has a preceding capture-forwarding store (stloc result(innerCapture); stloc outerFlag(K); leave outer-try). The strict b.Instructions.Count == 2 gate in the flag-setter scan rejects it.

   public async Task<int> ReturnFromInsideNestedTryFinally()
   {
       try
       {
           try { return 42; }
           finally { await Task.CompletedTask; }
       }
       finally { await Task.CompletedTask; }
   }

4. break / continue inside loop inside try-finally with await.
   Runtime-async applies the capture-and-flag-then-leave pattern not just to return but to any non-local jump that needs to run the finally first. directLeave.IsLeavingFunction rejects loop-level leaves, so the flag machinery survives.

   for (int i = 0; i < n; i++)
   {
       try
       {
           if (i == 1) continue;
           if (i == 5) break;
       }
       finally { await Task.CompletedTask; }
   }

Proposed fix: generalise the matcher.

• Accept either a Branch to a dedicated check-block or an inline IfInstruction immediately after the TryFinally (closes 1, 2).
• Relax the flag-setter shape to "ends with stloc flagVar(K); leave try", allowing arbitrary capture-forwarding stores before it (closes 3).
• Replace directLeave.IsLeavingFunction with "leaves any ancestor container" and synthesise the appropriate Leave to that container — the AST builder renders break / continue / return based on the target (closes 4).

Cluster 2 — Multi-handler matcher (TryRewriteMultiHandlerTryCatch)

The matcher succeeds for LoadsToCatch's specific shape (three Exception-typed handlers, all with throw; in their bodies, uniform variable access) but fails for any heterogeneous variant:

// Fails: two different typed catches
try { await t; }
catch (InvalidOperationException ex)
{
    await Task.Yield();
    Console.WriteLine(ex.Message);
}
catch (ArgumentException ex)
{
    await Task.Yield();
    Console.WriteLine(ex.Message);
}

// Also fails: typed catch + untyped catch
try { await t; }
catch (InvalidOperationException ex) { await Task.Yield(); Console.WriteLine(ex.Message); }
catch { await Task.Yield(); Console.WriteLine("other"); }

// Also fails: typed catch + Exception with throw;
try { await t; }
catch (InvalidOperationException ex) { await Task.Yield(); Console.WriteLine(ex.Message); }
catch (Exception) { await Task.Yield(); throw; }

All three produce raw lowered output with the flag-dispatched switch (num) post-catch handling intact, plus the ExceptionDispatchInfo.Capture(...).Throw() idiom where a throw; was the source.

Proposed investigation: add ILAst-level logging to identify which handler trips up the matcher, then relax the per-handler shape constraint to accept heterogeneous prefix-store sequences. Most likely the issue is the prefix-store pattern differing per handler (e.g. cast-class store layout differs when one handler accesses the variable and the other doesn't).

Cluster 3 — "Must be last instruction" Run-loop gate

// At the top of the Run-loop, before any per-tryCatch matcher:
if (tryCatch.ChildIndex != parentBlock.Instructions.Count - 1)
    continue;

This rejects any try-catch that isn't the last instruction in its parent block. For a top-level try-catch followed by an implicit function-leave it works, but for nested cases the parent block typically continues with a follow-up instruction (br continuation or similar), so the gate rejects.

Concrete reproducer:

public async Task TryCatchFinallyAllAwait()
{
    try { await Task.CompletedTask; Console.WriteLine("try"); }
    catch (Exception) { await Task.CompletedTask; Console.WriteLine("catch"); }
    finally { await Task.CompletedTask; Console.WriteLine("finally"); }
}

The outer try-finally recovers, but the inner try-catch (now inside the outer try block) is no longer the last instruction in its parent block. The gate rejects, leaving the flag-based catch dispatch intact.

Proposed fix: drop the gate. Each matcher already verifies the post-catch successor shape via its own parentBlock.Instructions[tryCatch.ChildIndex + 1]-style checks; this Run-loop-level gate is over-restrictive. Alternatively run the rewriter iteratively to a fixed point so each level is matched in turn.

Cluster 4 — Pre-existing bug exposed (also fails on baseline)

throw as the only exit from try body:

public async Task ThrowInsideTryFinally()
{
    try { throw new InvalidOperationException(); }
    finally { await Task.Yield(); }
}

The try body has no outward Branch / Leave (just a throw). The existence check seenExit (and the pre-review existence check) rejects, leaving the raw lowered output. Confirmed to fail on the branch tip prior to this review as well — not a regression, but the same pattern of "too-strict matcher" applies.

Proposed fix: drop the seenExit requirement entirely. The other gates (dispatch idiom recognition, catch-handler shape, stloc obj(ldnull) pre-init) are sufficient to identify the lowered pattern; a try body whose only exit is a throw is a legitimate input.

Out of scope (general decompiler limitations, not runtime-async specific)

These failed in the exploration but fail equivalently on the state-machine Async pipeline, so they're pre-existing decompiler limitations rather than runtime-async bugs:

• ternary ?: with awaits in both arms — decompiled as if/return.
• is T name pattern inside when filter — produces a BlockContainer the expression-builder can't render.
• checked { ... } block decompiled as inline checked(...).
• 3+ awaits in single arithmetic expression on non-Optimize — split into intermediate locals.

Tests that pass and could be added as coverage (no bugs)

These pass today but weren't in the fixtures — worth adding as regression guards:

• consecutive top-level try-finally
• async ValueTask<T> (smoke test)
• async Task<(int, int)> returning tuple from awaited task
• direct return await foo();
• true throw; rethrow after await
• typed catch with await + variable use
• catch (T) when (...) with no variable name
• switch with awaits in cases inside try-finally
• nested try-catch inside catch
• try-finally inside catch
• multiple consecutive awaits in finally
• return in try-catch (no finally)

[siegfriedpammer]

Status

The five new commits on the branch close clusters 2, 3, 4 and most of cluster 1; clusters 1.2 and 1.4 and everything under "out of scope" remain follow-ups.

Clusters

• Cluster 3 — drop the "must be last instruction" Run-loop gate
  (e31f68d1d Reverse runtime-async exception lowering for try-catch nested inside try-finally.)
  Also relaxed IsLeavingFunction to "leaves any ancestor container" (new IsContainerExit helper that accepts direct Leave, cross-container Branch, and indirect leave-block branches), and matched the flag-init by slot/kind/type to survive SplitVariables. Fixture: TryCatchFinallyAllAwait.
• Cluster 4 — drop the seenExit requirement
  (d954a9697 Reverse runtime-async try-finally where the try body always throws.)
  Also skip the post-TryFinally successor append when the new TryFinally has unreachable endpoint (avoids a block-invariant violation). Fixture: ThrowInsideTryFinally.
• Cluster 1.1, 1.3 — generalise the flag-based early-return matcher
  (420bf0cd8 Generalise runtime-async flag-based early-return for nested try-finally.)
  Rebuilt the matcher around an early-action template (the chain of stores the early path performs before its terminating Leave); each flag-setter becomes its own prefix + a clone of the template, so the inner / outer flag chain in nested try-finally collapses naturally over two descendant-order passes. Fixtures: ReturnFromTryFinally, ReturnFromInsideNestedTryFinally (gated on a new RUNTIMEASYNC preprocessor symbol).
• Cluster 1.2 — void return; at the end of a try-finally body
  The IL rewrite is correct (Leave function (Nop) ends up inside the try block), but the AST emitter drops the trailing return; because falling off the end of a void async method is equivalent. Needs an AST-side change rather than a matcher change.
• Cluster 1.4 — break / continue across a try-finally with await
  Roslyn lowers this as a multi-K dispatch (often a switch keyed on the flag), which the current single-K TryRewriteFlagBasedEarlyReturn can't recognise. Either iterate the matcher per K or grow it to handle switch / if-chain dispatches the same way TryRewriteMultiHandlerTryCatch now does.
• Cluster 2 — heterogeneous multi-handler
  (ced469460 Reverse runtime-async multi-handler try-catch dispatched via an if-chain.)
  Added a parallel MatchIfChainDispatch that walks Roslyn's chained if (num == K_i) br case_K_i dispatch (the form for two handlers, where the optimizer doesn't reach for a switch). Cloned the default-exit before re-attaching it. Fixtures: HeterogeneousMultiCatch1/2/3.

Plus one preparatory cleanup commit:

• FindFlagInitStore drop the per-call Predicate<StLoc> once all three callers converged on the same slot/kind/type + value-match shape
  (3b28533f2 Drop the per-call predicate from FindFlagInitStore.)

Out of scope (general decompiler limitations)

Left as documented in the original report:

• ternary ?: with awaits in both arms
• is T name pattern inside when filter
• checked { ... } block decompiled as inline checked(...)
• 3+ awaits in a single arithmetic expression on non-Optimize

Coverage gaps mentioned in the report

The "pass today" list at the bottom of the original report — patterns that already round-trip but weren't in the fixtures — isn't covered by these commits. Worth a separate test-only PR.

Verification

All 3299 ICSharpCode.Decompiler.Tests tests pass against runtime-async HEAD (3284 succeeded, 15 skipped, 0 failed). Full PrettyTestRunner is 1799/1799 green (5 skipped). The 12 RuntimeAsync* pretty-test variants all pass.