ekmett / bifunctors

Haskell 98 bifunctors, bifoldables and bitraversables
Other
57 stars 42 forks source link

Require Functor over Bifunctor where possible #125

Open expipiplus1 opened 1 year ago

expipiplus1 commented 1 year ago

Fixes https://github.com/ekmett/bifunctors/issues/124

Note that this does add a Functor requirement where none was required before

expipiplus1 commented 1 year ago

Rebased onto 5.15: https://gist.github.com/expipiplus1/bdc5750447d979583702ce5d44592572

expipiplus1 commented 1 year ago

Thank you for the patient review, @RyanGlScott!

Still to fix is the instance context generation, as it still wants to generate Bifunctor instances for any type variable f : * -> * -> *. This should now depend on its usage.

For example this, although f and g both have kind * -> * -> *, f requires a Bifunctor instance and g a Functor instance:

data Baz f g a b where
  Baz1 :: f a b -> Baz f g a b
  Baz2 :: g Int b -> Baz f g a b

deriveBifunctor ''Baz

An alternative would be to not change this context generation heuristic and simply use the old behavior of requiring a Bifunctor instance when dealing with variables with two type variables (regardless of if they're ours) i.e. only use this new behavior when it's a concrete type constructor. (Nicest yet would be just put a wildcard for the instance context, and allow GHC to fill it with the inferred context (https://gitlab.haskell.org/ghc/ghc/-/issues/13324))

For now I've fallen back to requiring Bifunctor for type variables, it's still an improvement over what was before I think though.

expipiplus1 commented 1 year ago

Another alternative would be to allow configuring this behavior via Options, something like preferFmap, that way people who can tolerate writing their own instance head can use that and allow type variables with arity > 2 to still use Functor where possible.

RyanGlScott commented 1 year ago

Ugh, I completely forgot about instance context generation. I'll be the first person to admit that the way bifunctors infers instance contexts is imperfect, and that's largely to be expected. Doing It Right™ would require something akin to full-blown type inference, and that is so difficult to do with Template Haskell that I don't think it is worth bothering to try.

This is all to say: I'm not terribly bothered by the fact that deriveBifunctor ''Baz would attempt to generate instance (Bifunctor f, Bifunctor g) => Bifunctor (Baz f g). Examples like Baz2 :: g Int b -> Baz f g a b are not the common case, so I'm fine with making users manually write the instance context out for such instances (in conjunction with makeBimap). Moreover, this generated instance will actually typecheck with GHC 9.6 due to Bifunctor having a quantified Functor superclass, so this bothers me even less.

What does bother me a bit is the fact that these two instances:

data Scooby1 f a b = Scooby1 (f Int a)
data Scooby2 a b = Scooby2 (Either Int a)

Will interact quite differently with deriveBifunctor. The former would use bimap in its generated instance, while the latter will use fmap. I'm not a fan of this because I generally like things to be invariant under substitution, and Scooby2 is basically the same thing as Scooby1, where f is instantiated to Either. Ideally, calling bimap on Scooby1 Either would behave exactly the same as calling bimap on Scooby2, but that isn't the case with the way the patch is set up currently.

To summarize, I'd prefer using fmap uniformly for things that look like F Int a or f Int a, even if that doesn't always work with bifunctors' limited type inference capabilities. You could also guard this functionality behind Options if you'd like—I don't have a strong opinion on that one.