-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better accommodate MTL style #1210
Comments
Related issue, it seems we also currently suffer from ambiguous implicits: import cats.{MonadReader, MonadState}
import cats.implicits._
def foo[F[_], R, S](implicit R: MonadReader[F, R], S: MonadState[F, S]): F[Boolean] =
for {
int <- S.get
char <- R.ask
} yield false
// Exiting paste mode, now interpreting.
<console>:16: error: value flatMap is not a member of type parameter F[S]
int <- S.get
^
<console>:17: error: value map is not a member of type parameter F[R]
char <- R.ask
^ The above doesn't work because both import cats.{MonadReader, MonadState}
import cats.implicits._
def foo[F[_], R, S](implicit R: MonadReader[F, R]): F[Boolean] =
for {
int <- R.ask
char <- R.ask
} yield false
// Exiting paste mode, now interpreting.
import cats.{MonadReader, MonadState}
import cats.implicits._
foo: [F[_], R, S](implicit R: cats.MonadReader[F,R])F[Boolean] |
we would not need implicit conversion would we? For instance this would work: trait MonadMagic[F[_]] {
def monad: Monad[F]
}
object MonadMagic {
implicit def mmmonad[F[_]](implicit mm: MonadMagic[F]): Monad[F] = mm.monad
} The ambiguous implicits problem of the second example is a bigger problem if you ask me. That said, I'm pessimistic about solving it. Cats uses extension pretty heavily throughout the design. One alternative could be something like: trait Functor[F[_]] {
def map[A, B](f: F[A])(fn: A => B): F[B]
}
trait Applicative[F[_]] {
def functor: Functor[F]
def pure[A](a: A): F[A]
}
trait Monad[F[_]] {
def applicative: Applicative[F]
def flatMap[A, B](f: F[A])(fn: A => F[B]): F[B]
}
trait LowPriorityApplicative {
// look up the hierarchy
implicit def appFromMonad[F[_]](implicit m: Monad[F]): Applicative[F] = m.applicative
}
object Applicative extends LowPriorityApplicative {
...
} This is anti-modular because subtypes implicit resolution looks up, while the super types give instances of the subtypes, so they also know of the existence. That said, it might be just a linear chain, so that it is actually manageable within cats. But it is a big departure (and speculative in that there may be other issues). |
I think the approach you sketched out is similar to the approach taken in scato which is also the approach taken in scalaz8. Completely agree on the ambiguous implicits being more serious, but I do think it needs fixing/looking into. The small example I used is a completely reasonable use case, it just takes abstracting via type classes one step further. Cats already provides |
Hey @adelbertc, Yes this is exactly the approach taken initially in The only thing is that I don't think it's realistic to expect being able to encode the hierarchy in the companion of the type classes, we basically use a big This way we can fine grain the resolution among all typeclasses in our Hope that helps, feel free to ping me if you have any specific question. Cheers |
So I randomly tried this: import cats._
import cats.implicits._
trait MonadMagic[F[_]] { deps: Monad[F] => // self-type instead of extends
def magic: F[Int]
}
def foo[F[_]](implicit A: MonadMagic[F], B: MonadReader[F, Int]): F[Int] = for {
a <- A.magic
b <- B.ask
} yield a + b and it seems to work? I'm expecting this to fail somewhere else you would want it to work (disregarding the fact that this encoding is different from the encoding for all the other type classes)... leaving here for feedback. EDIT So one thing I've noticed is that if you have access (from the outside) to a subclass (e.g. scala> def foo[F[_]](implicit F: MonadMagic[F]): F[Int] = F.map(F.magic)(identity)
<console>:36: error: value map is not a member of MonadMagic[F]
def foo[F[_]](implicit F: MonadMagic[F]): F[Int] = F.map(F.magic)(identity)
^ |
@adelbertc we did experiment a bit with this, but due to the limitation you are facing we decided to keep this approach only for the template system. You can find the latest revision of the template system (designed mainly by @jbgi) that should be merged soon in this PR: scalaz/scalaz#1217 |
I wonder if the following would work for the existing subtype encoding of type classes in Cats (and Scalaz 7): For MTL type classes (e.g. trait MonadReader[F[_], R] { self: Monad[F] =>
...
} but don't do the whole Scato encoding, we just stop there. I believe clients can then say things like: def foo[F[_], R, S](implicit F: Monad[F], R: MonadReader[F, R], S: MonadState[F, S]): F[Boolean] =
for {
int <- S.get
char <- R.ask
} yield false Note that Example of defining instances: trait Foo
object Foo {
// Note having to explicitly specify Monad instead of getting it transitively through MTL
implicit def instance[A, B]: Monad[Foo] with MonadReader[Foo, A] with MonadState[Foo, B] = ...
} cc @non since I know you didn't like this for BTW I am proposing this as a change to Cats. The one bit that bothers me is it special cases the treatment of type class encoding for MTL - we effectively split the world of type classes into "the fundamental blessed type classes" and "MTL." But a similar ambiguous implicit situation arises in the case of say, That being said, this change does make (monadic) MTL much nicer to use. |
The thing that would personally bother me, is that you can't actually get a This can be mitigated by introducing a Hope that helps! |
In Monix we have this package called monix.types in which I compiled a bunch of needed type-classes to support the types exposed in Monix and to allow me to translate them in corresponding Cats and Scalaz type instances, an idea I got from djspiewak/shims. I recently switched the type-class encoding used to that in Scato / Scalaz 8. I instantly got rid of things like |
@alexandru Hm I'm not sure I follow - why did switching the encoding allow you to get rid of That being said, that's awesome to hear. I'm a fan of the encoding as well :-) |
@adelbertc what I wanted to say is that if you have |
@alexandru Ah I see - I suppose you would still need something similar to do law checking, but yeah you can just ask for trait MonadReader[F[_], R] { self: Monad[F] =>
implicit def instance: Monad[F]
} This solves:
My main concern is we're treating MTL type classes as "special".. but maybe in the interest of getting a usable MTL experience in Scala that's not a huge deal? |
I'll have to think about it, I never tried it with a typed self. On exposing the instances, we could also expose different fields, for each type involved. trait MonadError[F[_], E] { self: Monad[F] with ApplicativeError[F,E] =>
def applicativeError: ApplicativeError[F,E] = self
def monad: Monad[F] = self
} Btw, I hope ordering of inheritance doesn't matter for those self types, because in Scala |
@alexandru nice to hear you had good use of it :-) I intuitively feel that the encoding should be used only if the instances which are define with it are coherent. I don't have a counter example, but it does not seems too hard to find one. For the self type, the hierarchy of scalaz8 is not complex enough to exercise this case, but I would highly surprised if self annotation constraints the order of linearisation, but oh well it's scalac after all ;) May the force be with you. |
Interestingly the approach @johnynek suggests above (and apparently the one taken in Scalaz 8) is similar to the approach I took in export-hook for what I called "subclass instances". It's probably not obvious because the boilerplate is hidden behind export-hooks annotation macros. |
You might want to checkout the progress I've been making in my experiment: |
@alexandru Looks interesting, is it similar to the approaches outlined above minus the scaffolding that allows you to treat, say, Would appreciate your feedback on #1379 as I think it's important we discuss what we want to do moving forward. It's also blocking stuff like #1337 |
@adelbertc so yes, the only inheritance that happens is to support composition, such that a It's not perfect, but when abstracting over |
One thing to keep in mind though, my type-class hierarchy is meant only to support the types I'm building, along with conversions for Cats and Scalaz, which means it is much simpler than what Cats is trying to support. Applying this encoding only to types such as |
I'm curious what the status is on this? I'm interesting in this and willing to put some effort into finding a solution (though I'm not sure how I can help - everyone seems to have chimed in and there's no clear winner). In thinking about the MTL syntax problem, it seems like the core of the issue is in def foo[M[_]](implicit me: MonadError[M, String], mr: MonadReader[M, Int]): M[Boolean] = {
mtl {
foo <- mr.ask
bar <- mr.ask
} yield False
} I recognize I might have a fairly naive understanding of the problem. I'd love to get MTL supported in cats in a usable way. |
(one way I might be able to help is to do the grunt work of putting together a proposal with the different approaches, their implementations, trade-offs, examples, etc. I just need to sync with whatever the status is and whoever is leading this / interested in leading this) |
@aaronlevin I outlined the problem in full here. The current status is bursts of discussion here and there. I had a PR that implemented the above proposal but that's long since rotted in the presence of other Cats activity. The principled version of me would like to see a general solution to this for all implicits - for instance I've filed an issue with Dotty. But for the language we're given.. I don't know. Scato solves it, but it's bulky and "untested in the wild" (though I'm fairly confident it would work.. don't know how performant it would be though). Jury's still out :/ |
@adelbertc how do you feel about a |
I would not be opposed to it - it might also open the doors for us to encode the MTL classes in Scato style instead of via subtyping. That being said it is probably easier said than done. The Scato encoding has certain implications w.r.t. how users need to import things to get the appropriate conversions in scope. This may confuse users and involve breaking changes for existing users - I believe Circe uses the MTL classes in some places. |
@edmundnoble, according to this comment, you are working on this, right? |
@kailuowang Yes. I'm working on disentangling the typeclass hierarchy to remove typeclasses which just exist to agglomerate laws. |
@edmundnoble , fantastic! Shall we assign this issue to you and mark it as "in progress" , or are you still experimenting? |
@adelbertc shall we close this since the decision is made on #1616? |
Yeppers |
EDIT Better written post about why the current (at the time of this writing, Oct 9, 2016) type class encoding is insufficient: http://typelevel.org/blog/2016/09/30/subtype-typeclasses.html )
One issue with the current approach is if you want to do MTL style programming, you end up having duplication. A short, contrived, example:
Now I want to define
MonadMagic[Option]
.. but that means I also need to re-implement theMonad
flatMap
andpure
methods, which leads to duplication and makes me uncomfortable.I suppose it could be solved by putting the
Monad[Option]
definition in atrait
and exposing it for people to mix-in.. but then we'd have to do it for every data type. Which maybe isn't a big deal? But perhaps a deficiency of the current approach.I suppose you could also do something like
and provide an implicit conversion
[F]MonadMagic[F] => Monad[F]
but that departs from how we are currently encoding things.I find some solution to this is important so as to not hinder MTL style programming which I find is an elegant way to encode EDSLs like
Free
modulo the need to explicitly reify the AST and thus presumably more performant.The text was updated successfully, but these errors were encountered: