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

Use strict types for applicatives #207

Closed
wants to merge 23 commits into from
Closed

Use strict types for applicatives #207

wants to merge 23 commits into from

Conversation

gusty
Copy link
Member

@gusty gusty commented Dec 7, 2019

This will switch to use the internal technique of tupling wrapping in order to avoid flexible types in overloads for the Applicative dispatcher.

Right now there are 2 issues:

  • Types implementing IEnumerable<_> would default to the seq<_> instance. This might be arguably desired.

  • Types implementing IEnumerable<_> would default to the seq<_> instance, even if they implement their own applicative instance. This is not acceptable.

@gusty gusty changed the title [WIP] Use strict types for applicatives Use strict types for applicatives Dec 9, 2019
@cannorin
Copy link
Member

cannorin commented Dec 9, 2019

I tested with fea4b08 but it seems to failing to redirect.

Microsoft (R) F# Interactive version 10.2.3 for F# 4.5
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> open FSharpPlus.TypeLits;;
> let v = vector ((fun i -> i + 1), (fun i -> i * 2));;
[<Struct>]
val v : Vector<(int -> int),S<S<Z>>> = [|<fun:v@2>; <fun:v@2-1>|]

> let u = vector (2, 3);;
[<Struct>]
val u : Vector<int,S<S<Z>>> = [|2; 3|]

> v <*> u;;
[<Struct>]
val it : Vector<int,S<S<Z>>> = [|3; 6|]

> open FSharpPlus;;
> v <*> u;;
val it : seq<int> = seq [3; 4; 4; 6]

wallymathieu
wallymathieu previously approved these changes Dec 10, 2019
src/FSharpPlus/Functor.fs Outdated Show resolved Hide resolved
@gusty
Copy link
Member Author

gusty commented Dec 12, 2019

Another update.
The other techinique that can be used in place or combined with wrapping is to add a higher DefaultX marker to the interface overload, the problem here if I do that is that type inference gets worst when resolving to those overloads and needs type annotation. This is currently a noticeable problem with the monad overload.
I'm coming to the conclusion that if there's no way to solve that type inference problem, a good compromise would be to let interfaces type inference degrade, that problem is preferred to not having external instances being resolved, but it would clearly be a breaking change.

@wallymathieu wallymathieu self-requested a review December 14, 2019 09:47
@wallymathieu wallymathieu dismissed their stale review December 14, 2019 09:51

Hasty approval

@gusty
Copy link
Member Author

gusty commented Dec 15, 2019

This is passing now, but there are some breaking specially in the idiom brackets section.
Also the Apply instance for KeyValuePair is removed.

Now, if we put in the balance this regression against not having externally defined applicatives implementing interfaces working decently (namely something along a plain f <*> x <*> y) it should be clear that this is priority.

How we rollout this change? Possibly we would have to skip a 1.1 version and move directly to a 2.0

The open question is whether this could be improved in order minimize the regression. I spent a crazy amount of time looking at this, if I add the time accumulated in fighting applicatives it's probably the time required to fix the F# compiler to make them work properly for once.

If I have more time, I will keep trying, but otherwise this would have to be merged.

Thoughts?

@cannorin
Copy link
Member

It's working now!

Microsoft (R) F# Interactive version 10.2.3 for F# 4.5
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> open FSharpPlus.TypeLits;;
> let v = vector ((fun i -> i + 1), (fun i -> i * 2));;
[<Struct>]
val v : Vector<(int -> int),S<S<Z>>> = [|<fun:v@2>; <fun:v@2-1>|]

> let u = vector (2, 3);;
[<Struct>]
val u : Vector<int,S<S<Z>>> = [|2; 3|]

> v <*> u;;
[<Struct>]
val it : Vector<int,S<S<Z>>> = [|3; 6|]

>  open FSharpPlus;;
> v <*> u;;
[<Struct>]
val it : Vector<int,S<S<Z>>> = [|3; 6|]

I suppose very few people seriously uses idiom brackets, so these breaking changes are acceptable.
Since we are introducing big changes such as #185 and #192 and other breaking change #205 I think it is better to move to 2.0 skipping 1.1.

@gusty
Copy link
Member Author

gusty commented Dec 16, 2019

Thanks for your feedback. #185 and #192 are not minor things, but at the same time they don't break anything.

Regarding #205 it's not breaking 1.0 since it didn't exists at that time.

So far this is the first serious reason to break 1.0, however I can think of other upcoming breaking changes:

  • A new ListT monad transformer I have on drafts, that won't be compatible with the existing one.

  • A redesign of the computation expression without the "autosense" functionality which users of this library have demonstrated it works only on certain scenarios and create more confusion than other thing.

@gusty gusty force-pushed the master branch 8 times, most recently from 105ed2c to d80a4ad Compare February 22, 2023 07:56
@wallymathieu wallymathieu force-pushed the master branch 5 times, most recently from f92d910 to 39638fb Compare February 22, 2023 17:36
@gusty gusty force-pushed the master branch 6 times, most recently from 9b34ece to b2f3c8c Compare October 15, 2023 05:01
@gusty gusty force-pushed the strict-applicative branch from 961285c to 7979273 Compare November 22, 2023 07:40
@gusty gusty mentioned this pull request Nov 22, 2023
4 tasks
@gusty
Copy link
Member Author

gusty commented Nov 23, 2023

Continued in #569

@gusty gusty closed this Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants