Try fix static compilation of state machines

[kerams]

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

letwhat(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

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

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

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

and in the second

letcode=letgenerator=fun unitVar ->__debugPoint( a.fs(12,4)-(12,34))letresource= System.Collections.Generic.IEnumerable`1.GetEnumerator [f]letbody=fun enumer ->letbody=letgenerator=fun unitVar ->letpatternInput=
System.Collections.Generic.IEnumerator`1.get_Current [enumer]letname=
#0 patternInput
__debugPoint( a.fs(15,8)-(15,33))
System.Console.Write [name]{new ResumableCode <TaskStateMachineData <unit>,unit>
System.Object..ctor ()memberInvoke((sm))=true}{new TaskCode <unit,unit>
System.Object..ctor ()memberInvoke((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).

internalstructwhat@4:IAsyncStateMachine,IResumableStateMachine<TaskStateMachineData<Unit>>{publicvoidMoveNext(){intresumptionPoint=this.ResumptionPoint;Exceptionex;Exceptionex2;try{IEnumerator<Tuple<string,string>>enumerator=this.f.GetEnumerator();boolflag=false;try{IEnumerator<Tuple<string,string>>enumerator2=enumerator;boolflag2;boolflag3;for(flag2=true;flag2&&enumerator2.MoveNext();flag2=flag3){Tuple<string,string>tuple=enumerator2.Current;stringitem=tuple.Item1;stringitem2=tuple.Item2;// <---------- this is not neededConsole.Write(item);flag3=true;}boolflag4=flag2;flag=flag4;}
...

vs

internalstructwhat2@11:IAsyncStateMachine,IResumableStateMachine<TaskStateMachineData<Unit>>{publicvoidMoveNext(){intresumptionPoint=this.ResumptionPoint;Exceptionex;Exceptionex2;try{IEnumerator<Tuple<string,string>>enumerator=this.f.GetEnumerator();boolflag=false;try{IEnumerator<Tuple<string,string>>enumerator2=enumerator;boolflag2;boolflag3;for(flag2=true;flag2&&enumerator2.MoveNext();flag2=flag3){Tuple<string,string>tuple=enumerator2.Current;stringitem=tuple.Item1;Console.Write(item);flag3=true;}boolflag4=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 ----------------------letcode=letgenerator=fun unitVar ->__debugPoint( C:\code\a.fs(61,8)-(61,25))lettestStateMachine$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 ()memberInvoke((sm))=true}
T1 ():{new ResumableCode <TaskStateMachineData <unit>,unit>
System.Object..ctor ()memberInvoke((sm))=true}]
testStateMachine$cont (){new TaskCode <unit,unit>
System.Object..ctor ()memberInvoke((sm))=
Invoke (generator ()) sm
}...

vs this when one of the record fields is commented out

----------- OVERALL EXPRESSION FOR STATE MACHINE CONVERSION ----------------------letcode=letgenerator=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 ()memberInvoke((sm))=true}
T1 ():{new ResumableCode <TaskStateMachineData <unit>,unit>
System.Object..ctor ()memberInvoke((sm))=true}]{new TaskCode <unit,unit>
System.Object..ctor ()memberInvoke((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

letbad()=task{letres={| ResultSet2 =[|{| im = Some 1; lc =3|}|]|}match[||]with|[||]->letc= res.ResultSet2 |> Array.map (fun x ->{| Name = x.lc |})letc= res.ResultSet2 |> Array.map (fun x ->{| Name = x.lc |})letc= 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 ()).