Skip to content

Improve static compilation of state machines#19297

Merged
T-Gro merged 36 commits intodotnet:mainfrom
majocha:resumable-fix-19296
Feb 27, 2026
Merged

Improve static compilation of state machines#19297
T-Gro merged 36 commits intodotnet:mainfrom
majocha:resumable-fix-19296

Conversation

@majocha
Copy link
Contributor

@majocha majocha commented Feb 16, 2026

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:

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

⚠️ 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

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.
@majocha
Copy link
Contributor Author

majocha commented Feb 16, 2026

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
Copy link
Contributor Author

majocha commented Feb 16, 2026

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 majocha changed the title Resumable state machines: fix #19296 Improve static compilation of state machines Feb 17, 2026
@majocha majocha marked this pull request as ready for review February 17, 2026 09:56
@majocha majocha requested a review from a team as a code owner February 17, 2026 09:56
@majocha
Copy link
Contributor Author

majocha commented Feb 18, 2026

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:

@T-Gro
Copy link
Member

T-Gro commented Feb 18, 2026

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
Copy link
Contributor Author

majocha commented Feb 18, 2026

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.

@T-Gro
Copy link
Member

T-Gro commented Feb 19, 2026

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)

@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Feb 20, 2026
@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Feb 20, 2026
@T-Gro T-Gro enabled auto-merge (squash) February 20, 2026 10:03
@majocha
Copy link
Contributor Author

majocha commented Feb 20, 2026

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

@majocha
Copy link
Contributor Author

majocha commented Feb 24, 2026

image 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 majocha marked this pull request as draft February 24, 2026 11:56
auto-merge was automatically disabled February 24, 2026 11:56

Pull request was converted to draft

@majocha majocha marked this pull request as ready for review February 27, 2026 16:47
@majocha
Copy link
Contributor Author

majocha commented Feb 27, 2026

It seems to be good apart of the flaky FSharp.Data test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

State machines: low-level resumable code not always expanded correctly, without warning

2 participants