Skip to content
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

Bijection + Typeclass = Bijected Typeclass #182

Closed
wants to merge 4 commits into from

Conversation

jcoveney
Copy link
Contributor

This builds off of #179

So, the idea is this... we have a lot of case classes, and with #179, we can auto-generate bijections to and from tuples with those case classes. Next, we have a lot of type classes, and it would be nice to auto-derive THOSE... now, one thing that we could do would be to have a macro per typeclass we care about (Ordering, Monoid, and on and on). Or we can try and nail it all in one fell swoop. This is an attempt at that.

Here is an example of usage:

import com.twitter.bijection._
import com.twitter.bijection.macros.TypeclassBijection
import com.twitter.bijection.macros.common.Derived
import TypeclassBijection._
import Derived._

trait Semigroup[T] {
  def plus(left: T, right: T): T
}

implicit val intSemigroup: Semigroup[Int] = new Semigroup[Int] {
  override def plus(left: Int, right: Int) = left + right
}

case class Wrapper(get: Int) extends AnyVal

implicit val intWrapperBij: Bijection[Int, Wrapper] = Bijection.build[Int, Wrapper] { Wrapper(_) } { _.get }

implicit val semigroupTypeclassBijection: TypeclassBijection[Semigroup] = new TypeclassBijection[Semigroup] {
  def apply[A, B](tc: Semigroup[A], bij: Bijection[A, B]) =
    new Semigroup[B] {
      override def plus(left: B, right: B) = bij(tc.plus(bij.invert(left), bij.invert(right)))
    }
}

derive[Semigroup[Wrapper]]

This introduces a new construct: derive. Derive is meant to be a superset of implicitly, in that it will always work if implicitly works, but introduces a new type. The purpose of this new type is to have better control over the implicit graph... this is necessary because as we get more generic, creating diverging implicits is easier and easier. In this case, I couldn't figure out a good way to get around the following implicit divering:

implicit def typeclassBijection[T[_], A, B](implicit tcBij: TypeclassBijection[T], typeclass: T[A], bij: ImplicitBijection[A, B]): Derived[T[B]] = Derived(tcBij(typeclass, bij.bijection))

If we can make it not diverge that would be awesome. @travisbrown is there perhaps a way to make a macro that does the exact same lookup you would expect, except it "hides" a function (in this case typeclassBijection itself)? The problem, I think, is that typeclassBijection can fulfill both tcBij and typeclass (since they are both T[_]), and so it diverges... unless it returns type Derived, which breaks the cycle. I'd love to get rid of that but if it can't be gotten rid of, I think the derive method is a fine way to say "we are going to use macros to give me something good, and the type checker can't do much more than that."

Open to thoughts, though, especially on ways to make it so that the following won't diverge:

implicit def typeclassBijection[T[_], A, B](implicit tcBij: TypeclassBijection[T], typeclass: T[A], bij: ImplicitBijection[A, B]):T[B] = tcBij(typeclass, bij.bijection)

@jcoveney
Copy link
Contributor Author

Oh and I'd love to add tests, and will, once a couple things are ironed out

IMHO this + #179 is the huge win we have wanted from macros. Every typeclass users use can now work seamlessly with case classes.

@travisbrown
Copy link

Hey @jcoveney, I'm taking a look at your gist now, and I'm not sure the issue there is that typeclassBijection is over-generating—if you :paste everything in the gist except the last two lines, and then paste the last two lines (either together or separately) it will work as expected.

I'll take a look at Derived—my feeling is that you shouldn't need macros for this part at all.

@jcoveney
Copy link
Contributor Author

What the heck! Now it works? that is so bizarre! But I will take it!! If we can get it not to diverge then I'd love to throw away the rest...that was all to get it to work. Experimenting now!

@travisbrown
Copy link

Yeah, if I remove the Derived stuff from TypeclassBijection.scala your example above works for me with s/derive/implicitly—whatever weird problem was turning up in the gist is fixed by compiling stuff together.

Having to tag stuff as derived (or even macro-generated) makes me a little nervous—it's likely to lead to weird problems down the line and there's almost certainly a better way to accomplish what you want.

@jcoveney
Copy link
Contributor Author

Yeah, I was hoping there would be a better way... I think I'll go ahead and start trying to make tests and see if it's just REPL weirdness

@travisbrown
Copy link

Quick side question: is there a reason to prefer the macro-based type inequality implementation over the ambiguous instances approach from Shapeless, etc.?

@jcoveney jcoveney mentioned this pull request Sep 17, 2014
@jcoveney
Copy link
Contributor Author

Closing this, moving conversation to #183

@jcoveney jcoveney closed this Sep 17, 2014
@jcoveney jcoveney deleted the jco/macro_case_class_plus branch September 17, 2014 15:55
@jcoveney
Copy link
Contributor Author

@travisbrown re: the macro one vs the shapeless one, no preference. I had taken a look at the shapeless one in the past and I often got the dreaded diverging implicits error, but if it fills the exact same role, I'd defer to that. Though I doubt that @johnynek or anyone would want a dependency on shapeless (a pretty heavy lib we don't use) in something as bottom level as bijection, thoguh for the macros package it might be ok

@travisbrown
Copy link

Yeah, I definitely wouldn't recommend a Shapeless dependency just for the type inequality implementation, which is only a few lines of code.

At some point I'd like to take a stab at a project that'd provide the functionality in the macros package via Shapeless, since there's getting to be a fair amount of overlap between what you're doing here and what Generic provides, but that can be a separate effort.

@jcoveney
Copy link
Contributor Author

I would wholeheartedly support such a project :)

@travisbrown
Copy link

@jcoveney—I started playing around with this last night for fun. Not much there at the moment (I spent most of the time getting serializable Generic instances) but I'll try to fill it out a bit more this weekend.

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.

2 participants