Skip to content
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

[FS-1045] F# 4.x: Add ToFSharpFunc overloads for Func<...>, especially for .NET Core #3013

Closed
wants to merge 8 commits into from

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented May 9, 2017

RFC https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1045-func-to-fsharpfunc-overloads.md

Addresses #1847 - ToFSharpFunc(Func) missing in .NET Core.

Adds

    type FuncConvert = 
        static member  inline ToFSharpFunc       : converter:Func<'T,'U>          -> ('T -> 'U)
        static member  inline ToFSharpFunc       : converter:Func<'T1,'T2,'U>          -> ('T1 * 'T2 -> 'U)
        static member  inline ToFSharpFunc       : converter:Func<'T1,'T2,'T3,'U>          -> ('T1 * 'T2 * 'T3 -> 'U)
        static member  inline ToFSharpFunc       : converter:Func<'T1,'T2,'T3,'T4,'U>          -> ('T1 * 'T2 * 'T3 * 'T4 -> 'U)
        static member  inline ToFSharpFunc       : converter:Func<'T1,'T2,'T3,'T4,'T5,'U>          -> ('T1 * 'T2 * 'T3 * 'T4 * 'T5 -> 'U)
        static member  inline ToFSharpFunc       : converter:Func<'T1,'T2,'T3,'T4,'T5,'T6,'U>          -> ('T1 * 'T2 * 'T3 * 'T4 * 'T5 * 'T6 -> 'U)
        static member  inline ToFSharpFunc       : converter:Func<'T1,'T2,'T3,'T4,'T5,'T6,'T7,'U>          -> ('T1 * 'T2 * 'T3 * 'T4 * 'T5 * 'T6 * 'T7 -> 'U)

There was no good way to create an FSharpFunc object from C# on .NET core with a method call.

@dsyme
Copy link
Contributor Author

dsyme commented May 9, 2017

@cartermp @neoeinstein Just to note this will need an RFC since this also adds more overloads for ToFSharpFunc on .NET Framework

Also adds this on the Converter overload

    [<Obsolete("Please use the FuncConvert.ToFSharpFunc(Func<...>) overload of this method")>]

@KevinRansom
Copy link
Member

Surface area change ... FSharp 4.2?

@dsyme
Copy link
Contributor Author

dsyme commented May 10, 2017

Hmmm... Yes in principle. But we should get the missing functionality into our FSharp.Core for .NET Core sooner than that I think. I'll look if we can add just the one overload somehow

@dsyme dsyme changed the title Add ToFSharpFunc overloads for Func<...>, especially for .NET Core F# 4.x: Add ToFSharpFunc overloads for Func<...>, especially for .NET Core May 10, 2017
@KevinRansom
Copy link
Member

@dsyme I'm okay with reving the version number, there are a number of PR's we would take if we do this.

@dsyme dsyme changed the title F# 4.x: Add ToFSharpFunc overloads for Func<...>, especially for .NET Core [RFC Needed] F# 4.x: Add ToFSharpFunc overloads for Func<...>, especially for .NET Core Dec 1, 2017
@KevinRansom KevinRansom changed the base branch from master to dev15.6 December 7, 2017 19:05
@KevinRansom
Copy link
Member

@dsyme can you resolve the conflicts pls.

@dsyme
Copy link
Contributor Author

dsyme commented Dec 8, 2017

@KevinRansom resolved

@dsyme dsyme changed the title [RFC Needed] F# 4.x: Add ToFSharpFunc overloads for Func<...>, especially for .NET Core [FS-1045] F# 4.x: Add ToFSharpFunc overloads for Func<...>, especially for .NET Core Dec 8, 2017
@dsyme
Copy link
Contributor Author

dsyme commented Dec 8, 2017

@cartermp @KevinRansom I've written the RFC https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1045-func-to-fsharpfunc-overloads.md

However as noted in the RFC this is a breaking change for some C# code, so don't merge this until we resolve that design question

@dsyme
Copy link
Contributor Author

dsyme commented Dec 8, 2017

See comment here and in the RFC - I think we should not go ahead with this because it is a breaking change, and instead we should provide a .NET Standard 2.0 build of FSHarp.Core.dll where System.Converter is available. Then we can keep the FuncConvert API the same as it has always been.

@0x53A
Copy link
Contributor

0x53A commented Dec 9, 2017

Everything except System.Func<'T,'U> can be safely taken, right?

In that case, why not just skip it?

IF the user proves an inline lambda, then the C# compiler will happily map it to a Converter instead of a Func.

IF the user wants to pass-through an existing Func delegate, then he would need to wrap it once - but I don't think that would be a killer-downside, and it wouldn't be worse than the current state.

@KevinRansom
Copy link
Member

@dsyme ... what do you want to do with this PR?

@brettfo
Copy link
Member

brettfo commented Apr 26, 2018

@dsyme, can you retarget this to master? I deleted the dev15.6 branch (which this was targeting) because it's old and we have a tag for it.

@dsyme dsyme changed the base branch from dev15.6 to master May 2, 2018 11:15
@dsyme dsyme reopened this May 2, 2018
@dsyme
Copy link
Contributor Author

dsyme commented May 2, 2018

@brettfo Are you sure you don't want this at dev15.8? It needs to target a branch that can bump the FSharp.Core version number

@dsyme
Copy link
Contributor Author

dsyme commented May 2, 2018

Continued at #4815

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

Successfully merging this pull request may close these issues.

6 participants