Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
abklearnhere opened this issue Jun 27, 2024 Discussed in #17323 · 7 comments
Closed
Milestone

Comments

@abklearnhere
Copy link

Discussed in #17323

Originally posted by @abklearnhere June 18, 2024
F# using seq { try .. with .. } raises trimming warning on publish. For example,

let f24 () = seq { try ignore (1/0); 111 with e -> 222 }
[<EntryPoint>]
let main _ =
    f24() |> Seq.head |> System.Console.WriteLine
    0

I am creating an F#, .NET 8 (SDK 8.0.302) console app and publish as a single file with AOT, trimming. I run dotnet publish command as below:

dotnet publish -c Release -r win-x64 --self-contained true -p:PublishAot=true -p:PublishTrimmed=true -p:ReflectionFree=false -o ./publish

This results in warning:

D:\a_work\1\s\src\FSharp.Core\prim-types.fs(7159): Trim analysis warning IL2091: Microsoft.FSharp.Control.LazyExtensions.Create(FSharpFunc`2<Unit,!!0>): 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'System.Lazy`1'. The generic parameter 'T' of 'Micros
oft.FSharp.Control.LazyExtensions.Create(FSharpFunc`2<Unit,!!0>)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

The fsproj file has following settings:

    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <IsAotCompatible>true</IsAotCompatible>
    <PublishAot>true</PublishAot>
    <PublishTrimmed>true</PublishTrimmed>
    <TrimmerSingleWarn>false</TrimmerSingleWarn>
@vzarytovskii
Copy link
Member

Duplicate of #17355

@vzarytovskii vzarytovskii marked this as a duplicate of #17355 Jun 27, 2024
@vzarytovskii vzarytovskii closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2024
@abklearnhere
Copy link
Author

@vzarytovskii I guess this is marked duplicate because fixing lazy would fix this. Instead, can this be fixed by removing dependency of lazy from seq { try .. with .. } ? I found exactly one use of lazy in entire file seqcore.fs. That reference causes this issue. Would you agree? Would this be preferred?

@vzarytovskii
Copy link
Member

@vzarytovskii I guess this is marked duplicate because fixing lazy would fix this. Instead, can this be fixed by removing dependency of lazy from seq { try .. with .. } ? I found exactly one use of lazy in entire file seqcore.fs. That reference causes this issue. Would you agree? Would this be preferred?

Probably not, since getting rid of lazy will mean it will be a substantial change and will need to be replaced with something to emulate it, which might have the same issue, which will need to be addressed.

@abklearnhere
Copy link
Author

Ah, okay, good to know. I was thinking of something simple like

        let mutable sourceEnumerator : IEnumerator<'T> = null
        let originalSource () =
            if isNull sourceEnumerator then
                sourceEnumerator <- source.GetEnumerator()

            sourceEnumerator

would be sufficient to replace let originalSource = lazy (source.GetEnumerator()). There are three originalSource.Value calls that could be replaced by originalSource ().

But, if there's more to it, I can understand. Thanks for taking a look.

@vzarytovskii
Copy link
Member

Ah, okay, good to know. I was thinking of something simple like

        let mutable sourceEnumerator : IEnumerator<'T> = null

        let originalSource () =

            if isNull sourceEnumerator then

                sourceEnumerator <- source.GetEnumerator()



            sourceEnumerator

would be sufficient to replace let originalSource = lazy (source.GetEnumerator()). There are three originalSource.Value calls that could be replaced by originalSource ().

But, if there's more to it, I can understand. Thanks for taking a look.

Lazy is also thread safe, mutable variable and check aren't, so this also needed to be solved, so own replacement solution will quickly grow in complexity and size, as well as it will require significantly more testing to make sure its behaviour is as before in both single and miltithreaded scenarios.

@abklearnhere
Copy link
Author

Got it! Thanks for the explanation!

@vzarytovskii
Copy link
Member

Got it! Thanks for the explanation!

Sure, np, thanks for testing out different aspects of AOT and reporting those back!

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

No branches or pull requests

2 participants