suzaku-io / boopickle

Binary serialization library for efficient network communication
Apache License 2.0
365 stars 41 forks source link

Macros fail on path-dependent types #29

Open japgolly opened 9 years ago

japgolly commented 9 years ago

Consider this:

  sealed trait AtomBase {
    sealed trait Atom
    case class Zero(value: String) extends Atom
  }
  trait Atom1 extends AtomBase {
    case class One(value: String) extends Atom
  }
  trait Atom2 extends AtomBase {
    case class Two(value: String) extends Atom
  }
  object Atoms01 extends AtomBase with Atom1
  object Atoms02 extends AtomBase with Atom2

Atoms01 contains only Zero and One; Atoms02 contains only Zero and Two. This structure is interpreted correctly by Scala. You can add or remove cases from the snippet below to see Scala generates warnings/errors correctly.

  def testExhaustiveness01: Atoms01.Atom => Unit = {
    case _: Atoms01.Zero => ()
    case _: Atoms01.One => ()
  }

  def testExhaustiveness02: Atoms02.Atom => Unit = {
    case _: Atoms02.Zero => ()
    case _: Atoms02.Two => ()
  }

This crashses:

materializePickler[Atoms01.Atom]

It should expect two cases:

But instead it tries to generate a pickler with cases:

Not wanting to generate double the boilerplate code, instead of using the built-in boopickle macros I'm generating Pickler with Unpicklers using my own macro code I wrote for other things, I'm getting the same error there too. I'll keep playing with it and write back if I get it working. If you know how or beat me to it, save me! I'm tired.

japgolly commented 9 years ago

FYI I got this working. Super tired. Need to get away from computer so I'll just paste what I've got with no explanation for now.

  def adtMaybeDebug[T: c.WeakTypeTag](c: Context, debug: Boolean): c.Expr[RW[T]] = {
    import c.universe._

    val T     = weakTypeOf[T]
    val types = findConcreteTypesNE(c)(T, DirectOnly).toList.sortBy(_.fullName)

    val typeArgLen = T.typeArgs.length

    val ctx = weakTypeOf[T] match {
      case ExistentialType(_, TypeRef(pre, _, _)) => pre
      case TypeRef(pre, _, _) => pre
    }

    var ignored: List[ClassSymbol] = Nil
    var pathDependent = false

    val defn = types.foldLeft(q"CompositePickler[$T]"){(q, t) =>

      // Try to deduce types from type constructors
      val t2: Type =
        t.typeParams match {

          // Already a type
          case Nil => t.toType

          // Type holes match that in base type constructor
          case ps if ps.length == typeArgLen => appliedType(t, T.typeArgs)

          // Give up
          case _ => fail(c, s"Don't know how to turn a $t into a $T.")
        }

      if (t2 <:< T)
        q"$q.addConcreteType[$t2]"
      else {
        // Check if type is a direct path-dependent type
        val tname = t.toType.typeSymbol.name.toTypeName
        val child = ctx.member(tname)
        if (child != NoSymbol) {
          pathDependent = true
          val full = Select(Ident(ctx.termSymbol), tname)
          q"$q.addConcreteType[$full]"
        } else {
          // Assume ok for now
          ignored ::= t
          q
        }
      }
    }

    if (ignored.nonEmpty && !pathDependent)
      fail(c, s"Failed to resolve the following: $ignored")

    // TODO type check for exhaustiveness

    val impl = q"$defn.done"

    if (debug) println("\n" + impl + "\n")
    c.Expr[RW[T]](impl)
  }

Also /cc @milessabin as you referenced this

milessabin commented 9 years ago

Your logic for extracting the path dependent prefix looks pretty fragile to me ... I suggest running it past @retronym to see if he can come up with something more general and robust ... I'm wary of recommending my own solution, because I'm not 100% confident that it's correct.

japgolly commented 9 years ago

That would be appreciated - I'm very new to macros and only got the above working after much trial-and-error. @retronym :bow:

drewhk commented 7 years ago

I think I have hit this exact same issue. Unfortunately I can't get rid of path dependent types there... Is there a workaround for this issue?

ochrons commented 7 years ago

Workaround is not to use the macros πŸ˜„

drewhk commented 7 years ago

Unfortunately then I have to hand-define picklers, in which case I can just use the other serialization approaches we have elsewhere.

japgolly commented 7 years ago

I don't use the macros that come with boopickle itself; for years now I've been using my own macros. I keep thinking I should contribute them back to boopickle itself. Later this week I'll create something showing what I've got (and what's been working smoothly for myself for a while). If it's useful let's talk about getting it into a PR.

ochrons commented 7 years ago

Dude! Sharing is caring! 😏

ochrons commented 7 years ago

One alternative would be to provide shapeless bindings for BooPickle, like they exist for Circe. But I haven't used shapeless, so don't have the knowledge to do it.

japgolly commented 7 years ago

Dude! Sharing is caring!

Haha yeah I know! I haven't shared yet because in a certain regard, they're less powerful and more explicit that those provided by the library, and so I've been expecting that most people won't like them. Now that's 2.12.2 is out that can be addressed but yeah, soon I'll share what I've got. You might be like them, you might not.

japgolly commented 7 years ago

Ok here you go, these are the macros I use in my project. I can't remember anymore why I created them or what advantages they have over the macros that come out-of-the-box; all I remember is that I found the built-in ones fatally inadequate for my needs (one of which I think was a certain level explicitness (?)) and have maintained my own over the years.

https://gist.github.com/japgolly/6d80cfd1353d92e2c5db9c27d263a1c9

  final def  pickleObject[T]: Pickler[T] = macro BoopickleMacroImpls.quietObject[T]
  final def _pickleObject[T]: Pickler[T] = macro BoopickleMacroImpls.debugObject[T]

  final def  pickleCaseClass[T]: Pickler[T] = macro BoopickleMacroImpls.quietCaseClass[T]
  final def _pickleCaseClass[T]: Pickler[T] = macro BoopickleMacroImpls.debugCaseClass[T]

  // Not deep, expects leaves to exist
  final def  pickleADT[T]: Pickler[T] = macro BoopickleMacroImpls.quietADT[T]
  final def _pickleADT[T]: Pickler[T] = macro BoopickleMacroImpls.debugADT[T]

  // Full auto. If ADT, also does generates leaves that don't yet have instances
  final def  derivePickler[T]: Pickler[T] = macro BoopickleMacroImpls.quietDerive[T]
  final def _derivePickler[T]: Pickler[T] = macro BoopickleMacroImpls.debugDerive[T]

There 's also this if you're using fixpoint types:

https://gist.github.com/japgolly/ec3210f93095b41decd46487f271bed0

// type Fix[F[_]] β‰ˆ F[Fix[F]]
// See japgolly/microlibs, Scalaz8 or slamdata/matryoshka for real type

def pickleFix[F[_]: scalaz.Functor](implicit p: Pickler[F[Unit]]): Pickler[Fix[F]]
japgolly commented 7 years ago

Also, while most of my code is nice and neat, unfortunately pretty much all of my macro code is messy, eg. there are sections commented out and sometimes I re-enable for debugging.

It also depends on a micro-library of mine: macro-utils.

ochrons commented 7 years ago

@japgolly is it ok if I just copypaste the relevant code from your macro-utils to keep boopickle dependency-free?

japgolly commented 7 years ago

Yeah no problem at all. Anything that you think would be useful, feel free to copy and modify as you like.