Module functions for 'T option ↔ 'T voption

[brianrourkeboll]

Description

• Add module functions for converting between 'T option and 'T voption.
• Remove the multiple existing internal copies of these functions in this repo and replace their usage with the FSharp.Core ones. See Module functions for 'T option ↔ 'T voption #17436 (comment).

module Option =
    val inline ofValueOption : voption : 'T voption -> 'T option
    val inline toValueOption : option  : 'T option  -> 'T voption

module ValueOption =
    val inline ofOption : option  : 'T option  -> 'T voption
    val inline toOption : voption : 'T voption -> 'T option

Checklist

• Test cases added.
• Release notes entry updated.

Notes

It seems to me like this ought to have been part of fsharp/fslang-suggestions#703, as @Thorium pointed out in fsharp/fslang-suggestions#703 (comment).

I can open a new language suggestion and RFC for this if needed — but perhaps we could just update RFC FS-1065 instead?

[github-actions]

❗ Release notes required

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/FSharp.Core	docs/release-notes/.FSharp.Core/9.0.100.md

[brianrourkeboll]

It looks like this bit of the CI

fsharp/azure-pipelines-PR.yml

Line 636 in 2eab49c

- script: dotnet build ./FSharp.Compiler.Service.sln /bl:\"artifacts/log/$(_BuildConfig)/ServiceRegularBuild.binlog\"

means that f4ca9f4 might need to go in a separate PR?

[psfinaki]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[psfinaki]

@brianrourkeboll, I don't see how the CI stuff can be directly related here... can you maybe rerevert the changes so that we can investigate a bit? It would be nice to get rid of all those dupes of this. And yeah, our CI is quite flaky recently.

[brianrourkeboll]

@psfinaki The CI failed when it was directly invoking dotnet build on FSharp.Compiler.Service.sln (the "Regular rebuild of FSharp.Compiler.Service.sln" step in the Plain_Build_* runs), which meant that it wasn't using the modified version of FSharp.Core.

Since f4ca9f4 replaced the internal functions with calls to the new ones in FSharp.Core, but the new version of FSharp.Core wasn't being used, it wouldn't compile.

I believe this was the run: https://dev.azure.com/dnceng-public/public/_build/results?buildId=752388&view=results

Unfortunately, I guess my cancelling of the run by force-pushing cleared the results away.

[psfinaki]

Right, yeah I was looking at the wrong results then.
That's clear, I think we can remove the dupe in a followup.

[psfinaki]

@vzarytovskii @abelbraaksma should we update the RFC (before or after the merge)?

[T-Gro]

Thanks for this addition.
I will merge this in. @brianrourkeboll : Could you please update the existing RFC FS-1065 with the added signatures?

[psfinaki]

Thanks for the RFC as well @brianrourkeboll. Would be cool to remove all the dupes from the codebase now. If you don't have time for this, I can probably create a PR from the commit with those changes that you had in this PR.

[brianrourkeboll]

@psfinaki All right, I opened #17457.

If the CI step mentioned in #17436 (comment) is using a shipped version of FSharp.Core, though, then this change may need to wait (or we may need to modify that step, depending on its intent).