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

Commit

Permalink
Force generation to create (un)picklers together
Browse files Browse the repository at this point in the history
* Macros generate `Pickler[T]`/`Unpickler[T]` together instead of
  creating them separately. This reduces final code size since now
  there's only one class instead of two and also improves the reuse of
  already generated picklers (see issue #409).

* As now the return type of the implicit method is
  `AbstractPicklerUnpickler`, there can be some errors because of
  ambiguous implicit values (e.g. sealed-trait-static) when (un)picklers
  are explicitly called via `generate`. The fix is to only use
  `PicklerUnpickler.generate` and deprecate separate explicit creation
  with `Pickler.generate` and `Unpickler.generate`.
  • Loading branch information
jvican committed Apr 17, 2016
1 parent 936430c commit 3ffb457
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 92 deletions.
7 changes: 4 additions & 3 deletions core/src/main/scala/scala/pickling/Pickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ trait Pickler[T] {
// Shim for Java code.
abstract class AbstractPickler[T] extends Pickler[T]
object Pickler {
def generate[T]: Pickler[T] = macro generator.Compat.genPickler_impl[T]
@deprecated("Use `PicklerUnpickler.generate` instead", "0.11")
def generate[T]: Pickler[T] = macro generator.Compat.genPicklerUnpickler_impl[T]
}

/** A dynamic pickler for type `T`. Its `pickle` method takes an object-to-be-pickled of
Expand Down Expand Up @@ -82,15 +83,15 @@ trait Unpickler[T] {
// Shim for Java code.
abstract class AbstractUnpickler[T] extends Unpickler[T]
object Unpickler {
def generate[T]: Unpickler[T] = macro generator.Compat.genUnpickler_impl[T]
@deprecated("Use `PicklerUnpickler.generate` instead", "0.11")
def generate[T]: Unpickler[T] = macro generator.Compat.genPicklerUnpickler_impl[T]
}
/* Shim for java code which wants to pickle/unpickle. */
abstract class AbstractPicklerUnpickler[T] extends Pickler[T] with Unpickler[T]
object PicklerUnpickler {
def apply[T](p: Pickler[T], u: Unpickler[T]): AbstractPicklerUnpickler[T] = new DelegatingPicklerUnpickler(p, u)
//def generate[T]: Pickler[T] with Unpickler[T] = macro Compat.PicklerUnpicklerMacros_impl[T]
def generate[T]: AbstractPicklerUnpickler[T] = macro generator.Compat.genPicklerUnpickler_impl[T]
def debugGenerate[T]: String = macro generator.Compat.genPicklerUnpickler_debug[T]
/** This is a private implementation of PicklerUnpickler that delegates pickle and unpickle to underlying. */
private class DelegatingPicklerUnpickler[T](p: Pickler[T], u: Unpickler[T]) extends AbstractPicklerUnpickler[T] {
// From Pickler
Expand Down
16 changes: 0 additions & 16 deletions core/src/main/scala/scala/pickling/generator/Compat.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,11 @@ import scala.reflect.macros.Context
import scala.reflect.runtime.{universe => ru}

private[pickling] object Compat {
def genPickler_impl[T: c.WeakTypeTag](c: Context): c.Expr[Pickler[T] with Generated] = {
val c0: c.type = c
val bundle = new { val c: c0.type = c0 } with PicklingMacros
c.Expr[Pickler[T] with Generated](bundle.genPickler[T])
}

def genPicklerUnpickler_impl[T: c.WeakTypeTag](c: Context): c.Expr[AbstractPicklerUnpickler[T] with Generated] = {
val c0: c.type = c
val bundle = new { val c: c0.type = c0 } with PicklingMacros
c.Expr[AbstractPicklerUnpickler[T] with Generated](bundle.genPicklerUnpickler[T])
}

def genUnpickler_impl[T: c.WeakTypeTag](c: Context): c.Expr[Unpickler[T] with Generated] = {
val c0: c.type = c
val bundle = new { val c: c0.type = c0 } with PicklingMacros
c.Expr[Unpickler[T] with Generated](bundle.genUnPickler[T])
}

def genPicklerUnpickler_debug[T: c.WeakTypeTag](c: Context): c.Expr[String] = {
val c0: c.type = c
val bundle = new { val c: c0.type = c0 } with PicklingMacros
c.Expr[String](bundle.debugPicklerUnpickler[T])
}
}
67 changes: 11 additions & 56 deletions core/src/main/scala/scala/pickling/generator/Macros.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,17 @@ package generator

private[pickling] object PicklingMacros {
import scala.language.experimental.macros
def genPickler[T]: Pickler[T] with Generated = macro scala.pickling.generator.Compat.genPickler_impl[T]
def genUnpickler[T]: Unpickler[T] with Generated = macro scala.pickling.generator.Compat.genUnpickler_impl[T]
def genPicklerUnpickler[T]: AbstractPicklerUnpickler[T] with Generated = macro scala.pickling.generator.Compat.genPicklerUnpickler_impl[T]

@deprecated("Use `genPicklerUnpickler` instead", "0.11")
def genPickler[T]: AbstractPicklerUnpickler[T] with Generated =
macro scala.pickling.generator.Compat.genPicklerUnpickler_impl[T]

@deprecated("Use `genPicklerUnpickler` instead", "0.11")
def genUnpickler[T]: AbstractPicklerUnpickler[T] with Generated =
macro scala.pickling.generator.Compat.genPicklerUnpickler_impl[T]

def genPicklerUnpickler[T]: AbstractPicklerUnpickler[T] with Generated =
macro scala.pickling.generator.Compat.genPicklerUnpickler_impl[T]
}
private[pickling] trait PicklingMacros extends Macro with SourceGenerator with TypeAnalysis {
import c.universe._
Expand Down Expand Up @@ -54,59 +62,6 @@ private[pickling] trait PicklingMacros extends Macro with SourceGenerator with T
}
}

def genPickler[T: c.WeakTypeTag]: c.Tree = preferringAlternativeImplicits {
val tpe = computeType[T]
checkClassType(tpe)
val sym = symbols.newClass(tpe)
val impl = PicklingAlgorithm.run(generator)(sym, logger)
val tree2 = impl map {
case PickleUnpickleImplementation(alg2, alg) => generatePicklerClass[T](alg2)
}
tree2 match {
case None =>
c.error(c.enclosingPosition, s"Failed to generate pickler for $tpe")
???
case Some(tree) =>
//System.err.println(s" --=== $tpe ===--\n$tree\n --=== / $tpe ===--")
tree
}
}
def genUnPickler[T: c.WeakTypeTag]: c.Tree = preferringAlternativeImplicits {
val tpe = computeType[T]
checkClassType(tpe)
val sym = symbols.newClass(tpe)
val impl = PicklingAlgorithm.run(generator)(sym, logger)
val tree2 = impl map {
case PickleUnpickleImplementation(alg2, alg) => generateUnpicklerClass[T](alg)
}
tree2 match {
case None =>
c.error(c.enclosingPosition, s"Failed to generate unpickler for $tpe")
???
case Some(tree) =>
//System.err.println(s" --=== $tpe ===--\n$tree\n --=== / $tpe ===--")
tree
}
}

def debugPicklerUnpickler[T: c.WeakTypeTag]: c.Tree = preferringAlternativeImplicits {
val tpe = computeType[T]
checkClassType(tpe)
val sym = symbols.newClass(tpe)
val impl = PicklingAlgorithm.run(generator)(sym, logger)
//System.err.println(impl)
val tree2 = impl map generatePicklerUnpicklerClass[T]
tree2 match {
case None =>
c.error(c.enclosingPosition, s"Failed to generate pickler/unpickler for $tpe")
???
case Some(tree) =>
//System.err.println(s" --=== $tpe ===--\n$tree\n --=== / $tpe ===--")
val treeString = s"$tree"
q"$treeString"
}
}

def genPicklerUnpickler[T: c.WeakTypeTag]: c.Tree = preferringAlternativeImplicits {
val tpe = computeType[T]
checkClassType(tpe)
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/scala/pickling/pickler/Any.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import scala.reflect.runtime.currentMirror
*
* It will look up pickler/unpickler at runtime, or generate it.
*/
object AnyPicklerUnpickler extends Pickler[Any] with Unpickler[Any]
object AnyPicklerUnpickler extends AbstractPicklerUnpickler[Any]
with AutoRegister[Any] {

override def tag: FastTypeTag[Any] = FastTypeTag.Any
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/scala/scala/pickling/pickler/GenPicklers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ import scala.language.experimental.macros
* See also `Pickler.generate`.
*/
trait GenPicklers {
implicit def genPickler[T]: Pickler[T] = macro generator.Compat.genPickler_impl[T]
implicit def genPickler[T]: AbstractPicklerUnpickler[T] =
macro generator.Compat.genPicklerUnpickler_impl[T]
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ trait GenOpenSumUnpicklers {
}

trait GenUnpicklers extends GenOpenSumUnpicklers {
implicit def genUnpickler[T]: Unpickler[T] with Generated = macro generator.Compat.genUnpickler_impl[T ]
implicit def genUnpickler[T]: AbstractPicklerUnpickler[T] with Generated =
macro generator.Compat.genPicklerUnpickler_impl[T ]
}
9 changes: 5 additions & 4 deletions core/src/main/scala/scala/pickling/pickler/Primitives.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ class PrimitivePickler[T: FastTypeTag](name: String)
}
override def unpickle(tag: String, reader: PReader): Any = {
try {
reader.beginEntry()
val readValue = reader.readPrimitive()
reader.endEntry()
readValue
// Don't use beginEntry/endEntry because
// tag has to be elided and there's some
// incompatibilities between java and scala
// primitive types (like String or Int)
reader.readPrimitive()
} catch { case PicklingException(msg, cause) =>
throw PicklingException(
s"""error in unpickle of primitive unpickler '$name':
Expand Down
19 changes: 19 additions & 0 deletions core/src/test/scala/pickling/run/issue409.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package scala.pickling.generation

import org.scalatest.FunSuite
import scala.pickling._, Defaults._, json._

class CheckGenPicklerUnpicklerTogether extends FunSuite {

case class Pi(number: Int)

test("check that pickler and unpickler are generated together (I)") {

val p = implicitly[Pickler[Pi]]
val u = implicitly[Unpickler[Pi]]

assert(p === u)

}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package scala.pickling.test.sealedtraitstaticannotated

import scala.pickling.{PicklingException, directSubclasses, Pickler, Unpickler, Defaults }
import scala.pickling._
import scala.pickling.static._
import scala.pickling.json._
import Defaults.{ stringPickler, intPickler, refUnpickler, nullPickler }
Expand All @@ -13,15 +13,13 @@ import org.scalatest.FunSuite
trait Fruit

object Banana {
implicit val pickler = Defaults.genPickler[Banana]
implicit val unpickler = Defaults.genUnpickler[Banana]
implicit val picklerUnpickler = Defaults.genPickler[Banana]
}

// this is BEFORE the subtypes below so directKnownSubclasses probably
// won't work and this would break without the directSubclasses annotation.
object Fruit {
implicit val pickler = Defaults.genPickler[Fruit]
implicit val unpickler = Defaults.genUnpickler[Fruit]
implicit val picklerUnpickler = Defaults.genPickler[Fruit]
}

sealed trait RedOrOrangeFruit extends Fruit
Expand Down Expand Up @@ -86,8 +84,7 @@ class SealedTraitStaticAnnotatedTest extends FunSuite {
}

test("manually generate") {
implicit val pumpkinPickler = Pickler.generate[Pumpkin]
implicit val pumpkinUnpickler = Unpickler.generate[Pumpkin]
implicit val pumpkinPicklerUnpickler = PicklerUnpickler.generate[Pumpkin]
val pumpkin = Pumpkin("Kabocha")
val pumpkinString = pumpkin.pickle.value
assert(JSONPickle(pumpkinString).unpickle[Pumpkin] == pumpkin)
Expand Down
5 changes: 2 additions & 3 deletions core/src/test/scala/pickling/run/sealed-trait-static.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package scala.pickling.test.sealedtraitstatic

import scala.pickling.{ Pickler, Unpickler, PicklingException }
import scala.pickling.{PicklerUnpickler, Pickler, Unpickler, PicklingException}
import scala.pickling.static._
import scala.pickling.json._
import scala.pickling.Defaults._
Expand All @@ -18,8 +18,7 @@ final case class Banana(something: Int) extends Fruit
final case class Cucumber(something: Int) // does not extend Fruit but same shape as Banana

object Fruit {
implicit val pickler = Pickler.generate[Fruit]
implicit val unpickler = Unpickler.generate[Fruit]
implicit val picklerUnpickler = PicklerUnpickler.generate[Banana]
}

class SealedTraitStaticTest extends FunSuite {
Expand Down

0 comments on commit 3ffb457

Please sign in to comment.