diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md index 6a30bf55364..d54b4a2e91f 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md @@ -14,6 +14,7 @@ * Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0, introduced by [#17706](https://github.com/dotnet/fsharp/pull/17706). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) * Fix FS3356 false positive for instance extension members with same name on different types, introduced by [#18821](https://github.com/dotnet/fsharp/pull/18821). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) * F# Scripts: Fix default reference paths resolving when an SDK directory is specified. ([PR #19270](https://github.com/dotnet/fsharp/pull/19270)) +* Improve static compilation of state machines. ([PR #19297](https://github.com/dotnet/fsharp/pull/19297)) ### Added * FSharpType: add ImportILType ([PR #19300](https://github.com/dotnet/fsharp/pull/19300)) diff --git a/src/Compiler/Optimize/LowerStateMachines.fs b/src/Compiler/Optimize/LowerStateMachines.fs index 90238e0dc3c..5d0bcad002f 100644 --- a/src/Compiler/Optimize/LowerStateMachines.fs +++ b/src/Compiler/Optimize/LowerStateMachines.fs @@ -189,6 +189,11 @@ type LowerStateMachine(g: TcGlobals) = if sm_verbose then printfn "eliminating 'if __useResumableCode...'" BindResumableCodeDefinitions env thenExpr + // Look through debug points to find resumable code bindings inside + | Expr.DebugPoint (_, innerExpr) -> + let envR, _ = BindResumableCodeDefinitions env innerExpr + (envR, expr) + | _ -> (env, expr) @@ -235,7 +240,16 @@ type LowerStateMachine(g: TcGlobals) = TryReduceApp env expandedExpr laterArgs | Expr.Let (bind, bodyExpr, m, _) -> - match TryReduceApp env bodyExpr args with + // If the binding returns resumable code, add it to the env so that + // references to it in the body can be resolved during reduction. + // This handles patterns like 'let cont = (fun () -> ...; Zero()) in cont()' + // generated by the optimizer for CE if-then branches. + let envR = + if isExpandVar g bind.Var then + { env with ResumableCodeDefns = env.ResumableCodeDefns.Add bind.Var bind.Expr } + else + env + match TryReduceApp envR bodyExpr args with | Some bodyExpr2 -> Some (mkLetBind m bind bodyExpr2) | None -> None @@ -308,6 +322,14 @@ type LowerStateMachine(g: TcGlobals) = | Some innerExpr2 -> Some (Expr.DebugPoint (dp, innerExpr2)) | None -> None + // Resolve variables known to the env, e.g. locally-bound resumable code continuations + | Expr.Val (vref, _, _) when env.ResumableCodeDefns.ContainsVal vref.Deref -> + TryReduceApp env env.ResumableCodeDefns[vref.Deref] args + + // Push through function applications by combining the arg lists + | Expr.App (f, _fty, _tyargs, fArgs, _m) -> + TryReduceApp env f (fArgs @ args) + | _ -> None @@ -349,7 +371,19 @@ type LowerStateMachine(g: TcGlobals) = // Repeated top-down rewrite let makeRewriteEnv (env: env) = - { PreIntercept = Some (fun cont e -> match TryReduceExpr env e [] id with Some e2 -> Some (cont e2) | None -> None) + { PreIntercept = Some (fun cont e -> + match e with + // Don't recurse into nested state machine expressions - they will be + // processed by their own LowerStateMachineExpr during codegen. + // This prevents modification of the nested machine's internal + // 'if __useResumableCode' patterns which select its dynamic fallback. + | _ when Option.isSome (IsStateMachineExpr g e) -> Some e + // Eliminate 'if __useResumableCode' - nested state machines are already + // guarded above, so any remaining occurrences at this level are from + // beta-reduced inline helpers and should take the static branch. + | IfUseResumableStateMachinesExpr g (thenExpr, _) -> Some (cont thenExpr) + | _ -> + match TryReduceExpr env e [] id with Some e2 -> Some (cont e2) | None -> None) PostTransform = (fun _ -> None) PreInterceptBinding = None RewriteQuotations=true diff --git a/tests/FSharp.Compiler.ComponentTests/Language/StateMachineTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/StateMachineTests.fs index 86e11cf2029..f7d2ebb906c 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/StateMachineTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/StateMachineTests.fs @@ -3,8 +3,40 @@ namespace Language open Xunit +open FSharp.Test.Assert open FSharp.Test.Compiler -open FSharp.Test + + +// Inlined helper containing a "if __useResumableCode ..." construct failed to expand correctly, +// executing the dynmamic branch at runtime even when the state machine was compiled statically. +// see https://github.com/dotnet/fsharp/issues/19296 +module FailingInlinedHelper = + open FSharp.Core.CompilerServices + open FSharp.Core.CompilerServices.StateMachineHelpers + open System.Runtime.CompilerServices + + let inline MoveOnce(x: byref<'T> when 'T :> IAsyncStateMachine and 'T :> IResumableStateMachine<'Data>) = + x.MoveNext() + x.Data + + let inline helper x = + ResumableCode(fun sm -> + if __useResumableCode then + sm.Data <- x + true + else + failwith "unexpected dynamic branch at runtime") + + #nowarn 3513 // Resumable code invocation. + let inline repro x = + if __useResumableCode then + __stateMachine + (MoveNextMethodImpl<_>(fun sm -> (helper x).Invoke(&sm) |> ignore)) + (SetStateMachineMethodImpl<_>(fun _ _ -> ())) + (AfterCode<_, _>(fun sm -> MoveOnce(&sm))) + else + failwith "dynamic state machine" + #warnon 3513 module StateMachineTests = @@ -21,6 +53,11 @@ module StateMachineTests = |> withOptions ["--nowarn:3511"] |> compileExeAndRun + [] + let ``Nested __useResumableCode is expanded correctly`` () = + FailingInlinedHelper.repro 42 + |> shouldEqual 42 + [] // https://github.com/dotnet/fsharp/issues/13067 let ``Local function with a flexible type``() = """ @@ -181,63 +218,212 @@ module TestStateMachine let test = task { return 42 } """ |> compile - |> verifyIL [ """ -.method public strict virtual instance void MoveNext() cil managed -{ - .override [runtime]System.Runtime.CompilerServices.IAsyncStateMachine::MoveNext - - .maxstack 4 - .locals init (int32 V_0, - class [runtime]System.Exception V_1, - bool V_2, - class [runtime]System.Exception V_3) - IL_0000: ldarg.0 - IL_0001: ldfld int32 TestStateMachine/test@3::ResumptionPoint - IL_0006: stloc.0 - .try - { - IL_0007: ldarg.0 - IL_0008: ldflda valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1 TestStateMachine/test@3::Data - IL_000d: ldc.i4.s 42 - IL_000f: stfld !0 valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1::Result - IL_0014: ldc.i4.1 - IL_0015: stloc.2 - IL_0016: ldloc.2 - IL_0017: brfalse.s IL_0036 - - IL_0019: ldarg.0 - IL_001a: ldflda valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1 TestStateMachine/test@3::Data - IL_001f: ldflda valuetype [runtime]System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1 valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1::MethodBuilder - IL_0024: ldarg.0 - IL_0025: ldflda valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1 TestStateMachine/test@3::Data - IL_002a: ldfld !0 valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1::Result - IL_002f: call instance void valuetype [netstandard]System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1::SetResult(!0) - IL_0034: leave.s IL_0042 - - IL_0036: leave.s IL_0042 - - } - catch [runtime]System.Object - { - IL_0038: castclass [runtime]System.Exception - IL_003d: stloc.3 - IL_003e: ldloc.3 - IL_003f: stloc.1 - IL_0040: leave.s IL_0042 - - } - IL_0042: ldloc.1 - IL_0043: stloc.3 - IL_0044: ldloc.3 - IL_0045: brtrue.s IL_0048 - - IL_0047: ret - - IL_0048: ldarg.0 - IL_0049: ldflda valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1 TestStateMachine/test@3::Data - IL_004e: ldflda valuetype [runtime]System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1 valuetype [FSharp.Core]Microsoft.FSharp.Control.TaskStateMachineData`1::MethodBuilder - IL_0053: ldloc.3 - IL_0054: call instance void valuetype [netstandard]System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1::SetException(class [netstandard]System.Exception) - IL_0059: ret -} -""" ] + |> verifyIL [ ".override [runtime]System.Runtime.CompilerServices.IAsyncStateMachine::MoveNext" ] + + // The original repro from https://github.com/dotnet/fsharp/pull/14930 + [] + let ``Task with for loop over tuples compiles statically`` () = + FSharp """ +module TestStateMachine +let what (f: seq) = task { + for name, _whatever in f do + System.Console.Write name +} + """ + |> compile + |> verifyIL [ ".override [runtime]System.Runtime.CompilerServices.IAsyncStateMachine::MoveNext" ] + + // The original repro from https://github.com/dotnet/fsharp/issues/12839#issuecomment-2562121004 + [] + let ``Task with for loop over tuples compiles statically 2`` () = + FSharp """ +module TestStateMachine +let test = task { + for _ in [ "a", "b" ] do + () +} + """ + |> compile + |> verifyIL [ ".override [runtime]System.Runtime.CompilerServices.IAsyncStateMachine::MoveNext" ] + + // see https://github.com/dotnet/fsharp/pull/14930#issuecomment-1528981395 + [] + let ``Task with some anonymous records`` () = + FSharp """ +module TestStateMachine +let bad () = task { + let res = {| ResultSet2 = [| {| im = Some 1; lc = 3 |} |] |} + + match [| |] with + | [| |] -> + let c = res.ResultSet2 |> Array.map (fun x -> {| Name = x.lc |}) + let c = res.ResultSet2 |> Array.map (fun x -> {| Name = x.lc |}) + let c = res.ResultSet2 |> Array.map (fun x -> {| Name = x.lc |}) + return Some c + | _ -> + return None +} +""" + |> compile + |> verifyIL [ ".override [runtime]System.Runtime.CompilerServices.IAsyncStateMachine::MoveNext" ] + + + + // repro of https://github.com/dotnet/fsharp/issues/12839 + [] + let ``Big record`` () = + FSharp """ +module TestStateMachine +type Foo = { X: int option } + +type BigRecord = + { + a1: string + a2: string + a3: string + a4: string + a5: string + a6: string + a7: string + a8: string + a9: string + a10: string + a11: string + a12: string + a13: string + a14: string + a15: string + a16: string + a17: string + a18: string + a19: string + a20: string + a21: string + a22: string + a23: string + a24: string + a25: string + a26: string + a27: string + a28: string + a29: string + a30: string + a31: string + a32: string + a33: string + a34: string + a35: string + a36: string // no warning if at least one field removed + + a37Optional: string option + } + +let testStateMachine (bigRecord: BigRecord) = + task { + match Some 5 with // no warn if this match removed and only inner one kept + | Some _ -> + match Unchecked.defaultof.X with // no warning if replaced with `match Some 5 with` + | Some _ -> + let d = { bigRecord with a37Optional = None } // no warning if d renamed as _ or ignore function used + () + | None -> () + | _ -> () + } +""" + |> compile + |> verifyIL [ ".override [runtime]System.Runtime.CompilerServices.IAsyncStateMachine::MoveNext" ] + + + [] // https://github.com/dotnet/fsharp/issues/12839#issuecomment-1292310944 + let ``Tasks with a for loop over tuples are statically compilable``() = + FSharp """ +module TestProject1 + +let ret i = task { return i } + +let one (f: seq) = task { + let mutable sum = 0 + + let! x = ret 1 + sum <- sum + x + + for name, _whatever, i in f do + let! x = ret i + sum <- sum + x + + System.Console.Write name + + let! x = ret i + sum <- sum + x + + let! x = ret 1 + sum <- sum + x + + return sum +} + +let two (f: seq) = task { + let mutable sum = 0 + + let! x = ret 1 + sum <- sum + x + + for name, _whatever, i in f do + let! x = ret i + sum <- sum + x + + System.Console.Write name + + let! x = ret 1 + sum <- sum + x + + return sum +} + +let three (f: seq) = task { + let mutable sum = 0 + + let! x = ret 1 + sum <- sum + x + + for name, _whatever, i in f do + let! x = ret i + sum <- sum + x + + System.Console.Write name + + return sum +} + +let four (f: seq) = task { + let mutable sum = 0 + + let! x = ret 5 + sum <- sum + x + + for name, _i in f do + System.Console.Write name + + let! x = ret 1 + sum <- sum + x + + return sum +} + +if (one [ ("", "", 1); ("", "", 2) ]).Result <> 8 then + failwith "unexpected result one" +if (one []).Result <> 2 then + failwith "unexpected result one" +if (two [ ("", "", 2) ]).Result <> 4 then + failwith "unexpected result two" +if (three [ ("", "", 5) ]).Result <> 6 then + failwith "unexpected result three" +if (four [ ("", 10) ]).Result <> 6 then + failwith "unexpected result four" +""" + |> withOptimize + |> compileExeAndRun + |> shouldSucceed + + + diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/NestedTaskFailures.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/NestedTaskFailures.fs index 7313ad61b87..e051ee0c40e 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/NestedTaskFailures.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/NestedTaskFailures.fs @@ -1,10 +1,8 @@ namespace FSharp.Core.UnitTests.Control.Tasks -// The tasks below fail state machine compilation. This failure was causing subsequent problems in code generation. +// The tasks below used to fail state machine compilation. This failure was causing subsequent problems in code generation. // See https://github.com/dotnet/fsharp/issues/13404 -#nowarn "3511" // state machine not statically compilable - this is a separate issue, see https://github.com/dotnet/fsharp/issues/13404 - open System open Microsoft.FSharp.Control open Xunit