ekmett / free

free monads
http://hackage.haskell.org/package/free
Other
159 stars 65 forks source link

Relax constraints for Free monad from applicative #225

Closed turion closed 1 year ago

turion commented 1 year ago

Some instances for Functor and Applicative for free monad transformers needlessly require Monad m for the inner monad. This PR relaxes these constraints to Functor and Applicative, respectively, and makes some trivial code changes.

turion commented 1 year ago

This is a smaller version of https://github.com/ekmett/free/pull/186.

turion commented 1 year ago

It's a bit ridiculous to write & debug for 7.* GHCs which I can't even install locally, but well.

RyanGlScott commented 1 year ago

I would be in favor of dropping support for pre-8.0 GHCs, given that they are increasingly becoming a barrier to contributors. I have already been dropping pre-8.0 support on other @ekmett libraries whenever this sort of situation arises, and I think it is time to do the same for free.

Would you be willing to wait until that happens before proceeding with this PR? (Alternatively, you could even submit a PR that drops pre-8.0 support if you'd like, but if you'd prefer, I could also do this myself.)

turion commented 1 year ago

I would be in favor of dropping support for pre-8.0 GHCs, given that they are increasingly becoming a barrier to contributors. I have already been dropping pre-8.0 support on other @ekmett libraries whenever this sort of situation arises, and I think it is time to do the same for free.

Yes :tada: highly in favour of this.

Would you be willing to wait until that happens before proceeding with this PR?

Yes sure!

(Alternatively, you could even submit a PR that drops pre-8.0 support if you'd like, but if you'd prefer, I could also do this myself.)

I would in principle, but I don't know how to find all the places where there are needless Functor m constraints. I don't know the codebase as well, so I'd rather leave that part to you.

RyanGlScott commented 1 year ago

I don't know the codebase as well, so I'd rather leave that part to you.

That is entirely reasonable—thank you for confirming. As a first pass, I think I will build free with -Wredundant-constraints to see which constraints can simply be removed without needing further code changes. After that, I'll hand it over to you.

(Also, my apologies for not noticing #154/#186 previously. I think these came about before I become a co-maintainer of the library, so they slipped under my radar.)

RyanGlScott commented 1 year ago

I've submitted #227 to drop support for pre-8.0 GHCs and remove "trivially" redundant constraints (i.e., constraints that are redundant without requiring any further code changes).

RyanGlScott commented 1 year ago

To help improve my own understanding, I've collected all of the places in #154, #186, and this PR where the implementation of class methods can be changed to relax superclass constraints:

186 contains all of the changes above plus the following:

There are likely other constraints downstream that can be relaxed as a result of these changes, but the changes above are the primary ones.

turion commented 1 year ago

I agree with your assessment. I've rebased and pushed.

turion commented 1 year ago

CI weather isn't good it seems... can we rerun?

RyanGlScott commented 1 year ago

Excellent, thanks for being patient! Although CI is being temperamental, the code itself looks great. I'll go ahead and land this.