Skip to content

Commit 25dd46b

Browse files
authored
Resumable state machines: fix "builder@" variable leak (#19630)
1 parent 7487bd1 commit 25dd46b

3 files changed

Lines changed: 71 additions & 39 deletions

File tree

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
* Fix signature generation: `private` keyword placement for prefix-style type abbreviations. ([Issue #15560](https://github.com/dotnet/fsharp/issues/15560), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
3737
* Fix signature generation: missing `[<Class>]` attribute for types without visible constructors. ([Issue #16531](https://github.com/dotnet/fsharp/issues/16531), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
3838
* Fix methods being tagged as `Member` instead of `Method` in tooltips. ([Issue #10540](https://github.com/dotnet/fsharp/issues/10540), [PR #19507](https://github.com/dotnet/fsharp/pull/19507))
39+
* Fix Debug-mode compilation when mixing resumable and standard computation expressions. ([Issue #19625](https://github.com/dotnet/fsharp/issues/19625), [PR #19630](https://github.com/dotnet/fsharp/pull/19630))
3940
* IlxGen: fix missing CompilationMapping attribute for generic values ([PR #19643](https://github.com/dotnet/fsharp/pull/19643))
4041

4142
### Added

src/Compiler/Optimize/LowerStateMachines.fs

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,13 @@ type LowerStateMachine(g: TcGlobals, outerResumableCodeDefns: ValMap<Expr>) =
338338
if sm_verbose then printfn "expanding defns and reducing %A..." expr
339339
//if sm_verbose then printfn "checking %A for possible resumable code application..." expr
340340
match expr with
341+
// Reduce helper-local 'if __useResumableCode then ... else ...' after inlining,
342+
// but preserve real nested state machines so their own lowering can still choose
343+
// the dynamic fallback if static compilation fails.
344+
| IfUseResumableStateMachinesExpr g (thenExpr, _) when Option.isNone (IsStateMachineExpr g thenExpr) ->
345+
if sm_verbose then printfn "reducing helper-local 'if __useResumableCode ...' to static branch"
346+
Some (remake thenExpr)
347+
341348
// defn --> [expand_code]
342349
| Expr.Val (defnRef, _, _) when env.ResumableCodeDefns.ContainsVal defnRef.Deref ->
343350
let defn = env.ResumableCodeDefns[defnRef.Deref]
@@ -372,22 +379,13 @@ type LowerStateMachine(g: TcGlobals, outerResumableCodeDefns: ValMap<Expr>) =
372379
// Repeated top-down rewrite
373380
let makeRewriteEnv (env: env) =
374381
{ PreIntercept = Some (fun cont e ->
375-
match e with
376-
// Don't recurse into nested state machine expressions - they will be
377-
// processed by their own LowerStateMachineExpr during codegen.
378-
// This prevents modification of the nested machine's internal
379-
// 'if __useResumableCode' patterns which select its dynamic fallback.
380-
| _ when Option.isSome (IsStateMachineExpr g e) -> Some e
381-
// Eliminate 'if __useResumableCode' - nested state machines are already
382-
// guarded above, so any remaining occurrences at this level are from
383-
// beta-reduced inline helpers and should take the static branch.
384-
| IfUseResumableStateMachinesExpr g (thenExpr, _) -> Some (cont thenExpr)
385-
| _ ->
386-
match TryReduceExpr env e [] id with Some e2 -> Some (cont e2) | None -> None)
382+
match TryReduceExpr env e [] id with
383+
| Some e2 -> Some (cont e2)
384+
| None -> None)
387385
PostTransform = (fun _ -> None)
388386
PreInterceptBinding = None
389387
RewriteQuotations=true
390-
StackGuard = StackGuard("LowerStateMachineStackGuardDepth") }
388+
StackGuard = StackGuard("LowerStateMachineStackGuard") }
391389

392390
let ConvertStateMachineLeafExpression (env: env) expr =
393391
if sm_verbose then printfn "ConvertStateMachineLeafExpression for %A..." expr

tests/FSharp.Compiler.ComponentTests/Language/StateMachineTests.fs

Lines changed: 59 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,32 @@ open Xunit
66
open FSharp.Test.Assert
77
open FSharp.Test.Compiler
88

9+
module StateMachineTests =
910

10-
// Inlined helper containing a "if __useResumableCode ..." construct failed to expand correctly,
11-
// executing the dynmamic branch at runtime even when the state machine was compiled statically.
12-
// see https://github.com/dotnet/fsharp/issues/19296
13-
module FailingInlinedHelper =
14-
open FSharp.Core.CompilerServices
15-
open FSharp.Core.CompilerServices.StateMachineHelpers
16-
open System.Runtime.CompilerServices
11+
let verify3511AndRun code =
12+
Fsx code
13+
|> withNoOptimize
14+
|> compile
15+
|> shouldFail
16+
|> withWarningCode 3511
17+
|> ignore
18+
19+
Fsx code
20+
|> withNoOptimize
21+
|> withOptions ["--nowarn:3511"]
22+
|> compileExeAndRun
1723

24+
// Inlined helper containing a "if __useResumableCode ..." construct failed to expand correctly,
25+
// executing the dynmamic branch at runtime even when the state machine was compiled statically.
26+
// see https://github.com/dotnet/fsharp/issues/19296
27+
[<Fact>]
28+
let ``Nested __useResumableCode is expanded correctly`` () =
29+
Fsx """
30+
open FSharp.Core.CompilerServices
31+
open FSharp.Core.CompilerServices.StateMachineHelpers
32+
open System.Runtime.CompilerServices
33+
34+
module FailingInlinedHelper =
1835
let inline MoveOnce(x: byref<'T> when 'T :> IAsyncStateMachine and 'T :> IResumableStateMachine<'Data>) =
1936
x.MoveNext()
2037
x.Data
@@ -27,7 +44,7 @@ module FailingInlinedHelper =
2744
else
2845
failwith "unexpected dynamic branch at runtime")
2946
30-
#nowarn 3513 // Resumable code invocation.
47+
#nowarn 3513
3148
let inline repro x =
3249
if __useResumableCode then
3350
__stateMachine<int, int>
@@ -38,25 +55,10 @@ module FailingInlinedHelper =
3855
failwith "dynamic state machine"
3956
#warnon 3513
4057
41-
module StateMachineTests =
42-
43-
let verify3511AndRun code =
44-
Fsx code
45-
|> withNoOptimize
46-
|> compile
47-
|> shouldFail
48-
|> withWarningCode 3511
49-
|> ignore
50-
51-
Fsx code
52-
|> withNoOptimize
53-
|> withOptions ["--nowarn:3511"]
58+
if FailingInlinedHelper.repro 42 <> 42 then failwith "unexpected result"
59+
"""
5460
|> compileExeAndRun
55-
56-
[<Fact>]
57-
let ``Nested __useResumableCode is expanded correctly`` () =
58-
FailingInlinedHelper.repro 42
59-
|> shouldEqual 42
61+
|> shouldSucceed
6062

6163
[<Fact>] // https://github.com/dotnet/fsharp/issues/13067
6264
let ``Local function with a flexible type``() =
@@ -467,3 +469,34 @@ if result[0].x <> 1 then failwith $"unexpected result {result[0]}"
467469
|> asExe
468470
|> compileExeAndRun
469471
|> shouldSucceed
472+
473+
[<Fact>]
474+
let ``Debug-mode: mixing resumable and standard computation expressions compiles``() =
475+
FSharp """
476+
module ReproMixedBuilders
477+
open System.Threading.Tasks
478+
479+
type TaskMaybeBuilder() =
480+
481+
member inline _.Zero() = Task.FromResult None
482+
483+
member inline _.Delay([<InlineIfLambda>] f) = task { return! f () }
484+
485+
member inline _.Bind(value, [<InlineIfLambda>] f) =
486+
task {
487+
match value with
488+
| None -> return None
489+
| Some result -> return! f result
490+
}
491+
492+
let taskMaybe = TaskMaybeBuilder()
493+
494+
let trigger() =
495+
taskMaybe {
496+
do! None
497+
}
498+
"""
499+
|> withDebug
500+
|> withNoOptimize
501+
|> compile
502+
|> shouldSucceed

0 commit comments

Comments
 (0)