search

ekmett / kan-extensions

Kan extensions, Kan lifts, the Yoneda lemma, and (co)monads generated by a functor
Other
79 stars 33 forks source link

Remove a bunch of flexible instances #53

Closed treeowl closed 11 months ago

treeowl commented 6 years ago

I don't know for sure if this is a good idea. It should improve inference, but the documentation is pretty gross. a little less clear.

treeowl commented 6 years ago

Another idea, not yet tested:

instance (m ~~ m', MonadIO m') => MonadIO (Codensity (m :: k -> TYPE rep)) where ...

It's a little more verbose, but should probably make for more readable documentation.

treeowl commented 6 years ago

Yes, that's better.

treeowl commented 6 years ago

@RyanGlScott, what's your opinion?

RyanGlScott commented 6 years ago

I'll be frank—I'm not sure if the cure is worse than the disease. This introduces a metric truckload of CPP, which will definitely make this trickier to maintain in the future.

Is the hit to type inference imposed by FlexibleInstances really that bad as to warrant this?

treeowl commented 6 years ago

@RyanGlScott, I'm not sure. @ekmett expressed frustration about that effect of being polykinded, so I figured it might be worth finding a solution. I don't know if there are realistic situations where inference will suffer here. I'm not quite as concerned about the amount of CPP as you are. It's ugly as heck, for sure, but it's only for instance heads. How many of those are we ever likely to deal with?

I'm a little more concerned about the readability of the documentation. I imagine a fair number of people will be scared of it. We could lie a bit by using && !defined(__HADDOCK__) in the CPP, but that's also not so wonderful.

RyanGlScott commented 6 years ago

How many of those are we ever likely to deal with?

In my experience, more often than you'd expect. Superclasses get added. New instances get created. Each of these will require rediscovering the right balance of CPP to use. It might be possible for us to juggle everything in our heads, but we also the time it will take for other contributors to wade through the swamp.

I'm not so worried about how the Haddocks will look—I just want to ensure that we're willing to walk the path it takes to get to that point.

treeowl commented 6 years ago

@Icelandjack, unfortunately, something about this approach causes GHC versions 8.0 and 8.2 to panic. Only 8.4 seems up to the job!

Icelandjack commented 6 years ago

Can't GHC do this automatically instead of failing?

instance Applicative f => Monoid (Codensity (f :: k -> Type) a)
-- -- --> 
instance (f ~~ f', Applicative f') => Monoid (Codensity (f :: k -> Type) a)
ekmett commented 11 months ago

Now that we're far enough into the future, we should probably broadly adopt something like this, even if it's just the standalone version @treeowl proposed with

instance (m ~~ m', MonadIO m') => MonadIO (Codensity (m :: k -> TYPE rep)) where ...

If our version floor is >= 8.2, no CPP need apply.

RyanGlScott commented 11 months ago

I forgot about https://github.com/ekmett/kan-extensions/pull/53#issuecomment-373832618 (which the CI just reminded me of), so we will need to use GHC >= 8.4 as the floor to avoid an old bug in GHC 8.2.