Skip to content

Conversation

amesgen
Copy link

@amesgen amesgen commented Jan 29, 2021

Allows to newtype-derive Uniform/UniformRange on GHC 8.6+, using QuantifiedConstraints (similar to this).

Also allows DerivingVia, applicable to #92. As an example, we revive #71.

  • For some reason, it only works with StandaloneDeriving.
  • RepLike might not be the best name. Should RepLike be exported, even though that is not strictly necessary?
  • On GHC < 8.6, there will be "unused import" warnings about StateT (we need the constructor to be available for Coercible instances). Could resolve with more CPP.
  • Missing documentation (will add if desired).

Feel free to close this if you think this is not worth it.

@Bodigrim
Copy link
Contributor

I do not particularly like polluting all classes with RepLike m constraint. It's not particularly restricting, just unsure if it is worth it.

Defining trivial RepLike for GHC < 8.6, which does not help to derive instance UniformRange, could be very much confusing for users. It could be easier to drop support of old GHCs entirely than provide a conditional API.

@Bodigrim
Copy link
Contributor

There is already a default definition of instance Uniform which works fine for enumeratable data:

default uniformM :: (StatefulGen g m, Generic a, GUniform (Rep a)) => g -> m a
uniformM = fmap to . (`runContT` pure) . guniformM

And #92 brings the same functionality for UniformRange.

On the other hand, using Enum interface when possible is faster (both in compile and in run time) than Generic.

@amesgen
Copy link
Author

amesgen commented Jan 30, 2021

I do not particularly like polluting all classes with RepLike m constraint. It's not particularly restricting, just unsure if it is worth it.

Yeah, it is certainly not elegant. In a better™ world, forall a b. Coercible a b => Coercible (f a) (f b) would be a superclass constraint on Functor (see e.g. here for an argument).

Defining trivial RepLike for GHC < 8.6, which does not help to derive instance UniformRange, could be very much confusing for users. It could be easier to drop support of old GHCs entirely than provide a conditional API.

Note that if one is only interested in < GHC 8.6 (e.g. one has an application on GHC 8.4) one can simply ignore that RepLike exists. You only have to worry about RepLike if you also want to support GHC 8.6+.


One thing that would need more exploration is whether DerivingVia would allow customization of the derived instances (e.g. different probabilities for different constructors), similar to deriving-aeson.


That said, I am also still not sure if this is worth it. The Generic instances are very nice as an alternative, but you are right that they are less performant (see e.g. https://gitlab.haskell.org/ghc/ghc/-/issues/5642 for compile time issues, deriving (Ord, Eq, Enum, Bounded) for theBigSum type is instant for me).

@Bodigrim
Copy link
Contributor

Note that if one is only interested in < GHC 8.6 (e.g. one has an application on GHC 8.4) one can simply ignore that RepLike exists.

If you are an app developer with GHC 8.4 then you see UniformEnum in the documentation, the symbol itself is available - but it does not work, you receive a cryptic error about roles.


How about a low-tech solution? We can just export

uniformEnumM :: forall g m a. (Enum a, Bounded a) => StatefulGen g m => g -> m a 
uniformEnumM g = pure . toEnum =<< uniformRM (fromEnum (minBound :: a), fromEnum (maxBound :: a)) g

so that clients could write

instance Uniform Things where uniformM = uniformEnumM

This is two-symbols shorter than

deriving via UniformEnum Things instance Uniform Things

No fancy extensions involved, no new type classes sticking out of everything.

@amesgen
Copy link
Author

amesgen commented Jan 30, 2021

Exporting uniformEnumM (and probably also uniformEnumRM) is a very good idea.

Also, I now lean in favor of closing this PR, and revisiting it when random has dropped support for GHC < 8.6, as the possible confusion right now is probably not worth the cost.

@lehins
Copy link
Contributor

lehins commented Jan 30, 2021

instance Uniform Things where uniformM = uniformEnumM

I also very much prefer this approach.

I now lean in favor of closing this PR

Yes, feel free to close it. I don't think all the complexity is really worth it

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.

3 participants