Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 24, 2025

Description

Fix IL2091 trimming warning in FSharp.Core for LazyExtensions.Create method by adding DynamicallyAccessedMembers attribute to the generic type parameter.

The IL2091 warning occurs because System.Lazy<'T> has a trimming constraint requiring PublicParameterlessConstructor on its type parameter. By adding the DynamicallyAccessedMembers attribute to the generic type parameter 'T in the type augmentation, we inform the trimmer that the constraint is satisfied.

Changes Made

  1. src/FSharp.Core/prim-types.fs: Added [<DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)>] attribute to the type parameter 'T in the type augmentation: type Lazy<[<DynamicallyAccessedMembers(...)>]'T> with
  2. src/FSharp.Core/prim-types.fsi: Added matching attribute to the signature file
  3. tests/AheadOfTime/Trimming/Program.fs: Added test cases for lazy and seq try-with scenarios (testLazySimple and testSeqWithTryFinally)
  4. tests/AheadOfTime/Trimming/check.ps1: Improved error handling so that:
    • All 3 CheckTrim tests run even if earlier ones fail
    • All errors are collected and reported at the end
    • Uses Azure DevOps ##vso[task.logissue] integration to report errors to the correct caller line
    • Script fails overall if any test fails
  5. docs/release-notes/.FSharp.Core/10.0.200.md: Added release notes entry

Test Instructions

  1. Build and pack: ./build.sh --pack -c Release (Linux) or .\Build.cmd -pack -c Release (Windows)
  2. Run AOT tests: ./tests/AheadOfTime/check.ps1
  3. Verify no IL2091 warnings appear in the trimming output

Fixes #17356

Original prompt

Task:

  • Remove the use of F# lazy and .Value from the implementation of EnumerateTryWith in src/FSharp.Core/seqcore.fs (currently originalSource = lazy source.GetEnumerator()).
  • Replace with a manual, thread-safe, one-time-initialized cell so that the enumerator is only created once, matching lazy semantics, without using System.Lazy or F# lazy.
  • Ensure the change does NOT break thread safety and init semantics.
  • Avoid performance regressions and extra allocations.
  • Justify the change as required to fix trimming warnings (IL2091) as observed in F# using try .. with .. inside sequence expression raises trimming warning on publish. #17356.

Test instructions (must validate before and after):

  • Repro the trimming warning by building and publishing the test project tests/AheadOfTime/Trimming/SelfContained_Trimming_Test/SelfContained_Trimming_Test.fsproj in release, with trimming enabled.
    • Example: dotnet publish -c Release -r win-x64 --self-contained true (if possible, also test on Linux with correct RuntimeIdentifier)
    • The publish must show a trim warning (IL2091) with the original code.
  • After the fix, repeat publish and confirm trimming warning is gone.
  • The fix must be compatible with both Windows (win-x64) and Linux (linux-x64) (adjust RuntimeIdentifier as needed for CI).

References:


Expected PR content:

  • Code change in src/FSharp.Core/seqcore.fs
  • Test instructions in PR summary for reviewer steps.
  • References to repro and verify, including CI trimming test job and windows/linux runtime identifiers.

Reminder:

  • Do not accidentally break or skip the existing trimming tests (tests/AheadOfTime/Trimming). Ensure CI trimming jobs can validate change.

This pull request was created as a result of the following prompt from Copilot chat.

Task:

  • Remove the use of F# lazy and .Value from the implementation of EnumerateTryWith in src/FSharp.Core/seqcore.fs (currently originalSource = lazy source.GetEnumerator()).
  • Replace with a manual, thread-safe, one-time-initialized cell so that the enumerator is only created once, matching lazy semantics, without using System.Lazy or F# lazy.
  • Ensure the change does NOT break thread safety and init semantics.
  • Avoid performance regressions and extra allocations.
  • Justify the change as required to fix trimming warnings (IL2091) as observed in F# using try .. with .. inside sequence expression raises trimming warning on publish. #17356.

Test instructions (must validate before and after):

  • Repro the trimming warning by building and publishing the test project tests/AheadOfTime/Trimming/SelfContained_Trimming_Test/SelfContained_Trimming_Test.fsproj in release, with trimming enabled.
    • Example: dotnet publish -c Release -r win-x64 --self-contained true (if possible, also test on Linux with correct RuntimeIdentifier)
    • The publish must show a trim warning (IL2091) with the original code.
  • After the fix, repeat publish and confirm trimming warning is gone.
  • The fix must be compatible with both Windows (win-x64) and Linux (linux-x64) (adjust RuntimeIdentifier as needed for CI).

References:


Expected PR content:

  • Code change in src/FSharp.Core/seqcore.fs
  • Test instructions in PR summary for reviewer steps.
  • References to repro and verify, including CI trimming test job and windows/linux runtime identifiers.

Reminder:

  • Do not accidentally break or skip the existing trimming tests (tests/AheadOfTime/Trimming). Ensure CI trimming jobs can validate change.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Remove F# lazy and .Value from EnumerateTryWith implementation Remove F# lazy from EnumerateTryWith to fix IL2091 trimming warning Nov 24, 2025
Copilot AI requested a review from T-Gro November 24, 2025 11:17
@T-Gro
Copy link
Member

T-Gro commented Nov 25, 2025

@copilot
The Inner try-finally's Dispose is executed before yielding from outer try-with fails now.

System.Exception : Eval or Execution has failed (expected to succeed): System.Exception: List is ["Before sum"; "Before try"; "Inside finally"; "Before try"; "Inside finally";
"Inside with pattern"; "Inside with pattern"; "Inside with body";
"End of with body"; "After sum"]

Copy link
Contributor Author

Copilot AI commented Nov 25, 2025

@copilot
The Inner try-finally's Dispose is executed before yielding from outer try-with fails now.

...

I've reverted the changes in 378bde0. The manual replacement of lazy causes this test failure due to complex interactions between F# closures, mutable variables, and the nested sequence expression structure. The lazy type has specific one-time evaluation semantics that are difficult to replicate correctly. This requires deeper investigation into the IL generation and closure semantics before proceeding.

Copilot AI changed the title Remove F# lazy from EnumerateTryWith to fix IL2091 trimming warning [WIP] Remove F# lazy from EnumerateTryWith to fix IL2091 trimming warning Nov 25, 2025
Copy link
Member

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

Fix the IL2091 trimming warning in FSharp.Core for LazyExtensions.Create method.

CRITICAL: Follow the exact CI build sequence to ensure the test project uses the freshly built FSharp.Core.

====================================================================================
STEP 1 - ADD REPRODUCTION TEST CASES

Location: tests/AheadOfTime/Trimming/Program.fs

Add the following two test cases from issue #17355 at the end of the file (before the main entry point):

let testLazySimple () =
let f23 () = let z = lazy (12345) in z.Force()
check "lazy_simple" 12345 (f23())

let testSeqWithTryFinally () =
let result =
seq {
try
yield 1
yield 2
with
| ex -> ()
} |> Seq.toList
check "seq_try_with" [1; 2] result

Then add calls to these test functions in the main entry point at the end of Program.fs where other tests are called.

====================================================================================
STEP 2 - REPRODUCE THE WARNING FOLLOWING CI SEQUENCE

Follow the exact sequence from azure-pipelines-PR.yml (Build_And_Test_AOT_Windows job):

On Windows:
.\Build.cmd -pack -c Release
.\tests\AheadOfTime\check.ps1

On Linux/Mac:
./Build.sh -pack -c Release
./tests/AheadOfTime/check.ps1

The check.ps1 script will automatically run:

  • Equality\check.ps1
  • Trimming\check.ps1

VERIFY the IL2091 warning appears in the build output:
"IL2091: Microsoft.FSharp.Control.LazyExtensions.Create(FSharpFunc2<Unit,!!0>): 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'System.Lazy1'."

====================================================================================
STEP 3 - APPLY THE FIX

Location: src/FSharp.Core/prim-types.fs (around line 7159)
Method: LazyExtensions.Create<'T>

Find the LazyExtensions.Create method definition and add the DynamicallyAccessedMembers attribute to the generic type parameter 'T':

Change from:
static member Create<'T>(creator: unit -> 'T) = ...

Change to:
static member Create<[<DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)>] 'T>(creator: unit -> 'T) = ...

Note: The DynamicallyAccessedMembersAttribute is already defined in the file for older .NET versions (search for "type internal DynamicallyAccessedMembersAttribute").

====================================================================================
STEP 4 - VERIFICATION FOLLOWING CI SEQUENCE

After applying the fix, rebuild and test using the same CI sequence:

On Windows:
.\Build.cmd -pack -c Release
.\tests\AheadOfTime\check.ps1

On Linux/Mac:
./Build.sh -pack -c Release
./tests/AheadOfTime/check.ps1

VERIFY:

  1. The IL2091 warning is no longer present in the build output
  2. All tests pass successfully including testLazySimple() and testSeqWithTryFinally()
  3. No new trimming warnings are introduced
  4. Build logs are available in: ./artifacts/log/Release/AheadOfTime/Trimming/

Test Projects (automatically built by check.ps1):

  • tests/AheadOfTime/Trimming/SelfContained_Trimming_Test/SelfContained_Trimming_Test.fsproj
  • tests/AheadOfTime/Trimming/StaticLinkedFSharpCore_Trimming_Test/StaticLinkedFSharpCore_Trimming_Test.fsproj
  • tests/AheadOfTime/Trimming/FSharpMetadataResource_Trimming_Test/FSharpMetadataResource_Trimming_Test.fsproj

All have:

  • PublishTrimmed=true
  • TreatWarningsAsErrors=true
  • Target net9.0

====================================================================================
CONTEXT

The warning occurs because System.Lazy requires DynamicallyAccessedMemberTypes.PublicParameterlessConstructor on its generic type parameter. When F#'s LazyExtensions.Create (used by the 'lazy' keyword) creates a Lazy, the trimmer cannot guarantee that the parameterless constructor won't be trimmed away unless the same annotation is propagated.

The fix annotates the generic parameter 'T' to declare it requires a public parameterless constructor, satisfying the trimmer's flow analysis.

Reference issue: #17355

====================================================================================
ADDITIONAL NOTES

  • The CI pipeline (azure-pipelines-PR.yml) already has a Build_And_Test_AOT_Windows job that runs these exact steps
  • The build sequence ensures the test projects use the freshly built compiler and FSharp.Core package
  • The Build.cmd -pack step creates NuGet packages in artifacts/packages/Release/Shipping/
  • The test projects reference these packages via eng/Versions.props and FSharpCorePreviewPackageVersionValue

Copy link
Contributor Author

Copilot AI commented Nov 25, 2025

@copilot

...

Implemented in 2ba6ed9. Added [<return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)>] attribute to LazyExtensions.Create and CreateFromValue methods in both prim-types.fs and prim-types.fsi. Also added the test cases to Program.fs as specified.

Copilot AI changed the title [WIP] Remove F# lazy from EnumerateTryWith to fix IL2091 trimming warning Fix IL2091 trimming warning in FSharp.Core LazyExtensions.Create Nov 25, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 26, 2025

❗ Release notes required


✅ Found changes and release notes in following paths:

Warning

No PR link found in some release notes, please consider adding it.

Change path Release notes path Description
src/FSharp.Core docs/release-notes/.FSharp.Core/10.0.200.md No current pull request URL (#19114) found, please consider adding it

Copilot AI requested a review from T-Gro November 28, 2025 10:16
@T-Gro
Copy link
Member

T-Gro commented Nov 28, 2025

Copy link
Contributor Author

Copilot AI commented Nov 28, 2025

@copilot :

Looks good, please add release notes to https://github.com/dotnet/fsharp/tree/main/docs/release-notes/.FSharp.Core/10.0.200.md

Added release notes in 3740fd7. Created docs/release-notes/.FSharp.Core/10.0.200.md with an entry under "Fixed" for the IL2091 trimming warning fix.

Copilot AI requested a review from T-Gro November 28, 2025 10:37
@T-Gro T-Gro marked this pull request as ready for review November 28, 2025 11:45
@T-Gro T-Gro requested a review from a team as a code owner November 28, 2025 11:45
@T-Gro T-Gro merged commit 7367446 into main Dec 1, 2025
40 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in F# Compiler and Tooling Dec 1, 2025
@T-Gro T-Gro deleted the copilot/remove-fsharp-lazy-use branch December 1, 2025 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

F# using try .. with .. inside sequence expression raises trimming warning on publish.

3 participants