Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

instance (MonadRandom m, MonadTrans t, Monad (t m)) => MonadRandom (t m) #310

Conversation

frasertweedale
Copy link
Contributor

It is common to use monad transformer stacks in applications, with
IO at the heart. For applications using cryptonite, it is useful to
have MonadRandom instances for the common transformers. Add the
instance

instance (MonadRandom m, MonadTrans t, Monad (t m)) => MonadRandom (t m)

which achieves this with little code and one small additional
dependency, 'transformers', with no new transitive dependencies and
upon which many programs are likely to end up depending anyway.
Although new dependencies are discouraged, this change is motivated
by the usefulness of the instance and the desire to avoid orphan
instances in applications.

Also drive-by remove (Functor m) from MonadRandom class constraints,
because Functor is now a superclass of Monad and cryptonite no
longer supports pre-AMP GHC versions.

Fixes: #304

It is common to use monad transformer stacks in applications, with
IO at the heart.  For applications using cryptonite, it is useful to
have MonadRandom instances for the common transformers.  Add the
instance

  instance (MonadRandom m, MonadTrans t, Monad (t m)) => MonadRandom (t m)

which achieves this with little code and one small additional
dependency, 'transformers', with no new transitive dependencies and
upon which many programs are likely to end up depending anyway.
Although new dependencies are discouraged, this change is motivated
by the usefulness of the instance and the desire to avoid orphan
instances in applications.

Also drive-by remove (Functor m) from MonadRandom class constraints,
because Functor is now a superclass of Monad and cryptonite no
longer supports pre-AMP GHC versions.

Fixes: haskell-crypto#304
frasertweedale added a commit to frasertweedale/hs-jose that referenced this pull request Feb 13, 2020
This reverts commit cfad613.

See discussion:
#91 (comment)

See also cryptonite pull request:
haskell-crypto/cryptonite#310
@ocheron
Copy link
Contributor

ocheron commented Feb 13, 2020

What if I want:

instance Monad m => MonadRandom (StateT ChaChaDRG m) where
    getRandomBytes n = StateT (return . randomBytesGenerate n)

Lifting is not the only valid implementation.
And an instance for t m would overlap with almost everything.

@ocheron
Copy link
Contributor

ocheron commented Feb 16, 2020

The goal of the typeclass is to abstract over different implementations.
We can't force an instance defined for all parameterized types.

@ocheron ocheron closed this Feb 16, 2020
@frasertweedale frasertweedale deleted the MonadTrans-MonadRandom branch February 17, 2020 01:09
@frasertweedale
Copy link
Contributor Author

@ocheron thanks for the consideration... and a justified rejection.

@ocharles
Copy link

ocharles commented Feb 26, 2020

What if I want:

instance Monad m => MonadRandom (StateT ChaChaDRG m) where
   getRandomBytes n = StateT (return . randomBytesGenerate n)

AFAICT, this pull request does not stop you doing that. You simply need to mark your instance as overlapping. (Though typically you mark the general instance as overlappable, but it's the same idea).

@ocheron
Copy link
Contributor

ocheron commented Feb 29, 2020

Yes but it's not terribly good to rely on instance resolution when the impact of a change is so easily unnoticed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MonadRandom needs instances for transformers
3 participants