Improve static compilation of state machines

[majocha]

Fixes #19296, #12839 also includes test cases from #14930.

Also fixes FS3511 The resumable code value(s) 'code' does not have a definition. we have no issue open for it, but it was suppressed in this repo.

Brazenly vibecoded, however I think the risk of changes here is under control, given that we have quite a few relevant projects and their tests in the regression matrix.

OK here goes AI summary:

This pull request improves the F# compiler's handling of state machine lowering, especially for cases involving resumable code and control flow constructs like if __useResumableCode. It fixes issues where inlined helpers or nested resumable code constructs could incorrectly fall back to dynamic branches at runtime, and adds comprehensive test coverage for these scenarios. The main focus is on ensuring that statically-compilable state machines are correctly recognized and optimized, and that code generation remains robust for complex patterns.

Key changes include:

Compiler logic improvements:

• Enhanced the lowering pass in LowerStateMachines.fs to correctly handle resumable code bindings found inside debug points, ensuring that nested resumable code constructs are properly expanded.
• Updated the reduction logic to track locally-bound resumable code continuations in the environment, allowing correct resolution and inlining of these continuations during optimization. [1] [2]
• Modified the rewrite environment to avoid recursing into nested state machine expressions, preventing unintended modification of their internal logic and ensuring that only the appropriate static branches are taken for if __useResumableCode.

Test suite enhancements:

• Added a new test module (FailingInlinedHelper) and test case to verify that inlined helpers containing if __useResumableCode are expanded correctly, addressing a real-world bug. [1] [2]
• Expanded tests to cover additional scenarios involving for-loops over tuples and various statically-compilable task patterns, ensuring robust handling of these constructs.

Documentation/test comments:

• Updated comments in the NestedTaskFailures.fs test to reflect that certain state machine failures are now fixed, clarifying the status of known issues.Fixes State machines: low-level resumable code not always expanded correctly, without warning #19296, also includes test cases from Try fix static compilation of state machines #14930

[github-actions]

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

[majocha]

Looks like this now to some point reinvents #14930. I'll try to include some of the relevant repros in tests. Too bad they are scattered across issues and comments.

[majocha]

Another observation: Lack of FS3511 warning does not mean the state machine actually compiled statically. Sometimes the fallback to dynamic implementation gives no warning.

[majocha]

I tried to include in tests as many old repros as I could find across the issues.
My methodology was: paste the repro in sharplab.io, if it does not statically compile, include it in StateMachineTests.fs

Turns out a lot of those repros already compile statically. If not in sharplab (no idea what version it uses) than with current compiler from main.

3 cases remained that are fixed specifically by this PR:

• State machines: low-level resumable code not always expanded correctly, without warning #19296,
• large record from Unexpected warning '[FS3511] This state machine is not statically compilable' #12839,
• and this:

  fsharp/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/NestedTaskFailures.fs

  Line 6 in 6c7aa5b

  #nowarn "3511" // state machine not statically compilable - this is a separate issue, see https://github.com/dotnet/fsharp/issues/13404

[T-Gro]

Is it right to close #12839 with this ? It looks like an umbrella issue for all sorts of problems.
I am definitely happy if all of them are resolved 👍 , well done.

I am also thinking if there is a way we could add an end2end test here, e.g. using the library regression testing pipeline?
We do have iced tasks in already, is there something we could run as verification or a special flag/prop/check/IL detection perhaps, to spot regressions ?
(regression = code still compiles, but will fallback to dynamic implementation)

[majocha]

Is it right to close #12839 with this ? It looks like an umbrella issue for all sorts of problems. I am definitely happy if all of them are resolved 👍 , well done.

As far as I can tell all of them are resolved now, few have been resolved for some time.

I am also thinking if there is a way we could add an end2end test here, e.g. using the library regression testing pipeline? We do have iced tasks in already, is there something we could run as verification or a special flag/prop/check/IL detection perhaps, to spot regressions ? (regression = code still compiles, but will fallback to dynamic implementation)

I have nothing apart from the few tests I added :)

One thing that comes to mind is to disable some of the #nowarn 3511 we have in this repo, or at least make them scoped to the exact place, like using let rec in tasks. But that depends on the warning being reliable.

[majocha]

OK, it seems removing nowarn 3511 in this repo uncovers some more cases that need fixing.

[majocha]

I removed the few catch all nowarns 3511. This revealed a nice small repro of

The resumable code value(s) 'code' does not have a definition.

Let's see if it's fixed.

[T-Gro]

Removing the nowarns here is good dogfooding 👍 .

IcedTasks have 3 of them in tests https://github.com/search?q=repo%3ATheAngryByrd%2FIcedTasks%203511&type=code that appear to be intentionally dynamic (for testing), otherwise I do not see it - all good ( I will just check the regression pipeline outputs if there are any warnings from it)

[majocha]

There's an odd timeout, but there is no information which test hanged. Previously we got this info with --blame-hang-timeout.

[majocha]

It seems one of the test cases in Compiler.Service deadlocked. This is probably caused by xUnit v3 synchronization context but I'll switch this PR to draft until this is investigated and resolved, just in case.

[majocha]

The restriction of top level values is no longer, #18817. BTW the tests in this file currently run only in DESKTOP in FSharpSuite. Would be good to have them migrated.

[T-Gro]

Indeed, I have that on my radar once we have the existing suite stabilized and sharded 👍 (we need macOS to get under timing limits before growing it further).

[T-Gro]

So most of these tests just compare runtime provided CurrentMethod and compare that against MoveNext .

If I am going to migrate these tests, is this the most solid assertion?
(i.e. not attempting to assert at type check level, but really going via full compileAndRun and checking if inside MoveNext just as well?)

[majocha]

fsharp/tests/fsharp/tests.fs

Lines 135 to 170 in 242bedb

	[<Fact>]
	let ``state-machines-non-optimized`` () =
	let cfg = testConfig "core/state-machines"
	fsc cfg "%s -o:test.exe -g --tailcalls- --optimize-" cfg.fsc_flags ["test.fsx"]
	peverify cfg "test.exe"
	execAndCheckPassed cfg ("." ++ "test.exe") ""
	[<Fact>]
	let ``state-machines-optimized`` () =
	let cfg = testConfig "core/state-machines"
	fsc cfg "%s -o:test.exe -g --tailcalls+ --optimize+" cfg.fsc_flags ["test.fsx"]
	peverify cfg "test.exe"
	exec cfg ("." ++ "test.exe") ""
	[<Fact>]
	let ``state-machines neg-resumable-01`` () =
	let cfg = testConfig "core/state-machines"
	singleVersionedNegTest cfg "preview" "neg-resumable-01"
	[<Fact>]
	let ``state-machines neg-resumable-02`` () =
	let cfg = testConfig "core/state-machines"
	singleVersionedNegTest cfg "preview" "neg-resumable-02"

Yes, just singleNegTest and compileAndRun would probably do. Not sure what to do about the peverify step.

[T-Gro]

The closest we can do is replace it with ilverify on the compiled program, that shall be possible.
( like a 'compileAndVerifyAndRun' -style helper into the test framework. Possibly we could have an |> verifyNotUsesDynamicInvocation or similar, but maybe the reflection based runtime check is really the best)