Improve static compilation of state machines#19297
Improve static compilation of state machines#19297majocha wants to merge 26 commits intodotnet:mainfrom
Conversation
❗ Release notes requiredCaution No release notes found for the changed paths (see table below). Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format. The following format is recommended for this repository:
If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request. You can open this PR in browser to add release notes: open in github.dev
|
Improve reduction of resumable code in state machines Enhance application reduction in state machine lowering for F# computation expressions by tracking let-bound resumable code in the environment and resolving references during reduction. This enables correct handling of optimizer-generated continuations and deeper reduction of nested applications. Also, update test comments to reflect resolved state machine compilation issues.
|
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. |
|
Another observation: Lack of FS3511 warning does not mean the state machine actually compiled statically. Sometimes the fallback to dynamic implementation gives no warning. |
| // 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 |
There was a problem hiding this comment.
This issue is fixed now.
| } | ||
| """ | ||
| |> compile | ||
| |> verifyIL [ ".override [runtime]System.Runtime.CompilerServices.IAsyncStateMachine::MoveNext" ] |
There was a problem hiding this comment.
State machines sometimes fallback to dynamic implmentation without FS3511 warning. Checking for generated MoveNext is more reliable.
|
I tried to include in tests as many old repros as I could find across the issues. 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: |
|
Is it right to close #12839 with this ? It looks like an umbrella issue for all sorts of problems. I am also thinking if there is a way we could add an end2end test here, e.g. using the library regression testing pipeline? |
As far as I can tell all of them are resolved now, few have been resolved for some time.
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 |
|
OK, it seems removing nowarn 3511 in this repo uncovers some more cases that need fixing. |
…es not have a definition.") consider a Run(code: ResumableCode<...>) method. In some cases code can get optimized away resulting in warning 3511 "The resumable code value(s) 'code' does not have a definition."
… into resumable-fix-19296
|
I removed the few catch all nowarns 3511. This revealed a nice small repro of
Let's see if it's fixed. |
| type IFSharpCodeFixProvider with | ||
|
|
||
| member private provider.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray<Diagnostic>) = | ||
| cancellableTask { |
There was a problem hiding this comment.
This was previously giving FS3511 with The resumable code value(s) 'code' does not have a definition.
| <!-- Turn off 3511: state machine not compilable - expected for inlined functions defining state machine generators --> | ||
| <OtherFlags>$(OtherFlags) --nowarn:3511</OtherFlags> |
There was a problem hiding this comment.
this is the only change to FSharp.Core. I guess no release notes needed.
|
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) |
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:
LowerStateMachines.fsto correctly handle resumable code bindings found inside debug points, ensuring that nested resumable code constructs are properly expanded.if __useResumableCode.Test suite enhancements:
FailingInlinedHelper) and test case to verify that inlined helpers containingif __useResumableCodeare expanded correctly, addressing a real-world bug. [1] [2]Documentation/test comments:
NestedTaskFailures.fstest 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