Try fix static compilation of state machines

[kerams]

Attempt to fix the last case reported here #12839, specifically its minimal repro

let what (f: seq<string * string>) = task {
    for name, _whatever in f do
        System.Console.Write name
}

which doesn't get statically compiled, while the following equivalent snippet does

let what2 (f: seq<string * string>) = task {
    use enumer = f.GetEnumerator()
    while enumer.MoveNext () do
        let name, _whatever = enumer.Current
        System.Console.Write name
}

In the first case, the input to lowering looks like this:

let code =
      let generator =
            fun unitVar ->
              __debugPoint( a.fs(5,4)-(5,7) )
              let body@1 =
                    fun name ->
                      fun _arg11 ->
                        __debugPoint( a.fs(6,8)-(6,33) )
                        System.Console.Write [name]
                        {
                          new  ResumableCode <TaskStateMachineData <unit>,unit>
                            System.Object..ctor ()
                            member Invoke ((sm))=
                              true
                        }
              let resource = System.Collections.Generic.IEnumerable`1.GetEnumerator [f]
              let body =
                    fun e ->
                      let body =
                            {
                              new  ResumableCode <TaskStateMachineData <unit>,unit>
                                System.Object..ctor ()
                                member Invoke ((sm))=
                                  Invoke (let arg0 =
                                                System.Collections.Generic.IEnumerator`1.get_Current [e]
                                          body@1 #0 arg0 #1 arg0) sm
                            }
                      {
                        new  ResumableCode <TaskStateMachineData <unit>,unit>
                          ...
                      }
                      
      ...

and in the second

let code =
      let generator =
            fun unitVar ->
              __debugPoint( a.fs(12,4)-(12,34) )
              let resource = System.Collections.Generic.IEnumerable`1.GetEnumerator [f]
              let body =
                    fun enumer ->
                      let body =
                            let generator =
                                  fun unitVar ->
                                    let patternInput =
                                          System.Collections.Generic.IEnumerator`1.get_Current [enumer]
                                    let name =
                                          #0 patternInput
                                    __debugPoint( a.fs(15,8)-(15,33) )
                                    System.Console.Write [name]
                                    {
                                      new  ResumableCode <TaskStateMachineData <unit>,unit>
                                        System.Object..ctor ()
                                        member Invoke ((sm))=
                                          true
                                    }
                            {
                              new  TaskCode <unit,unit>
                                System.Object..ctor ()
                                member Invoke ((sm))=
                                  Invoke (generator ()) sm
                            }
                      {
                        new  ResumableCode <TaskStateMachineData <unit>,unit>
                          ...
                      }
      ...

The problem occurs in TryReduceApp, which isn't able to reduce

Invoke (let arg0 = System.Collections.Generic.IEnumerator`1.get_Current [e]
        body@1 #0 arg0 #1 arg0) sm

I addressed this by making TryReduceApp call back into TryReduceExpr in the case of Expr.App. I am not sure if this is sound and I suspect there's a good reason why it hasn't been done before.

In any case, while there might be other constructs that will still fail, the 2 above produce an almost identical state machine (the difference is just one superfluous assignment).

internal struct what@4 : IAsyncStateMachine, IResumableStateMachine<TaskStateMachineData<Unit>>
{
    public void MoveNext()
    {
        int resumptionPoint = this.ResumptionPoint;
        Exception ex;
        Exception ex2;
        try
        {
            IEnumerator<Tuple<string, string>> enumerator = this.f.GetEnumerator();
            bool flag = false;
            try
            {
                IEnumerator<Tuple<string, string>> enumerator2 = enumerator;
                bool flag2;
                bool flag3;
                for (flag2 = true; flag2 && enumerator2.MoveNext(); flag2 = flag3)
                {
                    Tuple<string, string> tuple = enumerator2.Current;
                    string item = tuple.Item1;
                    string item2 = tuple.Item2;  //   <---------- this is not needed
                    Console.Write(item);
                    flag3 = true;
                }
                bool flag4 = flag2;
                flag = flag4;
            }
    ...

vs

internal struct what2@11 : IAsyncStateMachine, IResumableStateMachine<TaskStateMachineData<Unit>>
{
    public void MoveNext()
    {
        int resumptionPoint = this.ResumptionPoint;
        Exception ex;
        Exception ex2;
        try
        {
            IEnumerator<Tuple<string, string>> enumerator = this.f.GetEnumerator();
            bool flag = false;
            try
            {
                IEnumerator<Tuple<string, string>> enumerator2 = enumerator;
                bool flag2;
                bool flag3;
                for (flag2 = true; flag2 && enumerator2.MoveNext(); flag2 = flag3)
                {
                    Tuple<string, string> tuple = enumerator2.Current;
                    string item = tuple.Item1;
                    Console.Write(item);
                    flag3 = true;
                }
                bool flag4 = flag2;
                flag = flag4;
            }
        ...

[kerams]

I'll convert the 2 snippets above into a test if the fix is deemed appropriate.

[T-Gro]

I'll convert the 2 snippets above into a test if the fix is deemed appropriate.

Yes.
I would be also curious if debugging works for the body of the for loop.

[edgarfgp]

@dsyme would be awesome to have this change in the F# 8 timeline. Thanks, @kerams for taking a look at this 👍

[dsyme]

@kerams @T-Gro I'm totally good with having this change it, but it needs quite a lot of testing - I think all the tasks in the FSharp.Core tassks should be placed at top-level for example.

[kerams]

I could not reproduce #12839 (comment) on main.

#12839 (comment) requires further changes:

----------- OVERALL EXPRESSION FOR STATE MACHINE CONVERSION ----------------------
let code =
      let generator =
            fun unitVar ->
              __debugPoint( C:\code\a.fs(61,8)-(61,25) )
              let testStateMachine$cont =
                    fun unitVar ->
                      [Switch ((#EI_ilzero !0#)).# X
                         is None // Success T1 ()
                         dflt: Success T0 ()
                       T0 (): __debugPoint( C:\code\a.fs(65,16)-(65,61) )
                              {
                                new  ResumableCode <TaskStateMachineData <unit>,unit>
                                  System.Object..ctor ()
                                  member Invoke ((sm))=
                                    true
                              }
                       T1 (): {
                                new  ResumableCode <TaskStateMachineData <unit>,unit>
                                  System.Object..ctor ()
                                  member Invoke ((sm))=
                                    true
                              }]
              testStateMachine$cont ()
      {
        new  TaskCode <unit,unit>
          System.Object..ctor ()
          member Invoke ((sm))=
            Invoke (generator ()) sm
      }
...

vs this when one of the record fields is commented out

----------- OVERALL EXPRESSION FOR STATE MACHINE CONVERSION ----------------------
let code =
      let generator =
            fun unitVar ->
              __debugPoint( C:\code\a.fs(60,8)-(60,25) )
              [Switch ((#EI_ilzero !0#)).# X
                 is None // Success T1 ()
                 dflt: Success T0 ()
               T0 (): __debugPoint( C:\code\a.fs(64,16)-(64,61) )
                      {
                        new  ResumableCode <TaskStateMachineData <unit>,unit>
                          System.Object..ctor ()
                          member Invoke ((sm))=
                            true
                      }
               T1 (): {
                        new  ResumableCode <TaskStateMachineData <unit>,unit>
                          System.Object..ctor ()
                          member Invoke ((sm))=
                            true
                      }]
      {
        new  TaskCode <unit,unit>
          System.Object..ctor ()
          member Invoke ((sm))=
            Invoke (generator ()) sm
      }
...

The conversion fails on trying to reduce the testStateMachine$cont () application, because testStateMachine$cont is not recognized as being a resumable code definition binding. My latest changes extend the traversal in BindResumableCodeDefinitions, so that more relevant bindings that are nested further down are also added to the environment. If there are other expressions around the match clause, static compilation will fail again.

I can't shake the feeling that this is a bit of a bandaid, and I'm also concerned about opening the doors to bugs and creating invalid state machines (which is a lot worse than the dynamic invocation fallback). It's puzzling how adding one too many record fields can have such a snowball effect - why is let testStateMachine$cont = fun unitVar -> ..... in testStateMachine$cont () even created in the first place?

[kerams]

Here's a trivial piece of code that is still failing

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
}

[vzarytovskii]

@kerams do you need any help with further debugging it? I can take over if you'd like, and see if I can figure it ou.

[kerams]

I don't know where to go with this. I managed to add support for the static compilation of a couple of additional constructs, but it feels like these are just hotfixes on a case-by-case basis; perhaps a more systematic change is needed.

In any case, I'm not interested in pursuing this further.

[vzarytovskii]

Thanks. Let's park it then, I will try to revisit it in future in a separate PR

[edgarfgp]

@vzarytovskii @dsyme Any plans to revive this effort ?. There are more and more people facing this issue.

[vzarytovskii]

@vzarytovskii @dsyme Any plans to revive this effort ?. There are more and more people facing this issue.

No specific plans as for now. This was not planned for the current iteration and we are way out of capacity. Unless someone finds time (and probably postpone some other work like LSP or perf to next year), this is on hold now.

[vzarytovskii]

why is let testStateMachine$cont = fun unitVar -> ..... in testStateMachine$cont () even created in the first place

Just for the information, it's happening because the expression is crossing a threshold in ComputeSplitToMethodCondition, and ends up being a non-reducible resumable invocation.
One obvious "solution" would be to check for resumable code when we decide to split to continuation. Another - try reducing the continuation itself (since we can generally reduce the let foo = fun _ -> ... in foo ()).