From eaa7c2a5858acbbea7a63a5a8f266f4dca7d5fd2 Mon Sep 17 00:00:00 2001 From: jvican Date: Sun, 17 Apr 2016 20:28:07 +0200 Subject: [PATCH] Force automatic generation of (un)pickler together * 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`. --- .../main/scala/scala/pickling/Pickler.scala | 7 +- .../scala/pickling/generator/Compat.scala | 16 ----- .../scala/pickling/generator/Macros.scala | 67 +++---------------- .../scala/pickling/generator/sourcegen.scala | 65 ------------------ .../scala/scala/pickling/pickler/Any.scala | 2 +- .../scala/pickling/pickler/GenPicklers.scala | 3 +- .../pickling/pickler/GenUnpicklers.scala | 3 +- .../scala/pickling/pickler/Primitives.scala | 9 +-- .../test/scala/pickling/run/issue409.scala | 19 ++++++ .../run/sealed-trait-static-annotated.scala | 11 ++- .../pickling/run/sealed-trait-static.scala | 5 +- 11 files changed, 50 insertions(+), 157 deletions(-) create mode 100644 core/src/test/scala/pickling/run/issue409.scala diff --git a/core/src/main/scala/scala/pickling/Pickler.scala b/core/src/main/scala/scala/pickling/Pickler.scala index 2a1eca45e5..72a540b974 100644 --- a/core/src/main/scala/scala/pickling/Pickler.scala +++ b/core/src/main/scala/scala/pickling/Pickler.scala @@ -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 @@ -82,7 +83,8 @@ 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] @@ -90,7 +92,6 @@ 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 diff --git a/core/src/main/scala/scala/pickling/generator/Compat.scala b/core/src/main/scala/scala/pickling/generator/Compat.scala index 9148223506..012b632221 100644 --- a/core/src/main/scala/scala/pickling/generator/Compat.scala +++ b/core/src/main/scala/scala/pickling/generator/Compat.scala @@ -9,11 +9,6 @@ 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 @@ -21,15 +16,4 @@ private[pickling] object Compat { 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]) - } } diff --git a/core/src/main/scala/scala/pickling/generator/Macros.scala b/core/src/main/scala/scala/pickling/generator/Macros.scala index 2aa91c7388..d60769118f 100644 --- a/core/src/main/scala/scala/pickling/generator/Macros.scala +++ b/core/src/main/scala/scala/pickling/generator/Macros.scala @@ -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._ @@ -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) diff --git a/core/src/main/scala/scala/pickling/generator/sourcegen.scala b/core/src/main/scala/scala/pickling/generator/sourcegen.scala index 377ac9a803..7693847ea9 100644 --- a/core/src/main/scala/scala/pickling/generator/sourcegen.scala +++ b/core/src/main/scala/scala/pickling/generator/sourcegen.scala @@ -387,18 +387,6 @@ private[pickling] trait SourceGenerator extends Macro with tags.FastTypeTagMacr val picklersRegistry = q"$picklingPath.internal.currentRuntime.picklers" val generated = tq"$picklingPath.Generated" - def lazyInit(lookup: c.Tree, key: c.Tree, tpe: c.Tree, - notInitialized: c.TermName): c.Tree = { - q""" - if($picklersRegistry.isLookupEnabled) { - $lookup($key) match { - case Some(p) => p.asInstanceOf[$tpe] - case None => $notInitialized: $tpe - } - } else $notInitialized: $tpe - """ - } - def picklerUnpicklerLazyInit(lookup1: c.Tree, lookup2: c.Tree, key: c.Tree, tpe: c.Tree, notInitialized: c.TermName) = { q""" @@ -411,59 +399,6 @@ private[pickling] trait SourceGenerator extends Macro with tags.FastTypeTagMacr """ } - /** generates the tree which will construct + return a new instance of a Pickler class, capable of - * pickling an instance of type T, using the behavior outlined by the PicklerAst. - */ - def generatePicklerClass[T: c.WeakTypeTag](picklerAst: PicklerAst): c.Tree = { - - val tpe = computeType[T] - val picklerName = c.fresh(newTermName(syntheticBaseName(tpe) + "Pickler")) - val createTagTree = super[FastTypeTagMacros].impl[T] - val picklerType = tq"$picklingPath.Pickler[$tpe]" - val key = q"$createTagTree.key" - val lookup = q"$picklersRegistry.lookupExistingPickler" - - q""" - _root_.scala.Predef.locally { - implicit object $picklerName extends $picklerType with $picklingPath.Generated - with $picklingPath.AutoRegisterPickler[$tpe]{ - - lazy val tag: $picklingPath.FastTypeTag[$tpe] = $createTagTree - def pickle(picklee: $tpe, builder: $picklingPath.PBuilder): _root_.scala.Unit = ${genPicklerLogic[T](picklerAst)} - - } - ${lazyInit(lookup, key, picklerType, picklerName)} - } - """ - - } - - def generateUnpicklerClass[T: c.WeakTypeTag](unpicklerAst: UnpicklerAst): c.Tree = { - - val tpe = computeType[T] - val unpicklerName = c.fresh(newTermName(syntheticBaseName(tpe) + "Unpickler")) - val createTagTree = super[FastTypeTagMacros].impl[T] - val unpickleLogic = genUnpicklerLogic[T](unpicklerAst) - val unpicklerType = tq"$picklingPath.Unpickler[$tpe]" - val genUnpicklerType = tq"$unpicklerType with $generated" - val key = q"$createTagTree.key" - val lookup = q"$picklersRegistry.lookupExistingUnpickler" - - q""" - _root_.scala.Predef.locally { - implicit object $unpicklerName extends $unpicklerType with $generated - with $picklingPath.AutoRegisterUnpickler[$tpe] { - - lazy val tag: $picklingPath.FastTypeTag[$tpe] = $createTagTree - def unpickle(tagKey: _root_.java.lang.String, reader: $picklingPath.PReader): _root_.scala.Any = $unpickleLogic - - } - ${lazyInit(lookup, key, genUnpicklerType, unpicklerName)} - } - """ - - } - def generatePicklerUnpicklerClass[T: c.WeakTypeTag](impl: PickleUnpickleImplementation): c.Tree = { val tpe = computeType[T] diff --git a/core/src/main/scala/scala/pickling/pickler/Any.scala b/core/src/main/scala/scala/pickling/pickler/Any.scala index 05a7f2eda2..bb3600a5a9 100644 --- a/core/src/main/scala/scala/pickling/pickler/Any.scala +++ b/core/src/main/scala/scala/pickling/pickler/Any.scala @@ -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 diff --git a/core/src/main/scala/scala/pickling/pickler/GenPicklers.scala b/core/src/main/scala/scala/pickling/pickler/GenPicklers.scala index bc19b86f82..15aeeddc05 100644 --- a/core/src/main/scala/scala/pickling/pickler/GenPicklers.scala +++ b/core/src/main/scala/scala/pickling/pickler/GenPicklers.scala @@ -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] } diff --git a/core/src/main/scala/scala/pickling/pickler/GenUnpicklers.scala b/core/src/main/scala/scala/pickling/pickler/GenUnpicklers.scala index 17499f9c46..c15a867368 100644 --- a/core/src/main/scala/scala/pickling/pickler/GenUnpicklers.scala +++ b/core/src/main/scala/scala/pickling/pickler/GenUnpicklers.scala @@ -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 ] } diff --git a/core/src/main/scala/scala/pickling/pickler/Primitives.scala b/core/src/main/scala/scala/pickling/pickler/Primitives.scala index 80884c4c95..6c088af50d 100644 --- a/core/src/main/scala/scala/pickling/pickler/Primitives.scala +++ b/core/src/main/scala/scala/pickling/pickler/Primitives.scala @@ -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': diff --git a/core/src/test/scala/pickling/run/issue409.scala b/core/src/test/scala/pickling/run/issue409.scala new file mode 100644 index 0000000000..12dbae22d9 --- /dev/null +++ b/core/src/test/scala/pickling/run/issue409.scala @@ -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) + + } + +} diff --git a/core/src/test/scala/pickling/run/sealed-trait-static-annotated.scala b/core/src/test/scala/pickling/run/sealed-trait-static-annotated.scala index e158b5ca7f..2de23d4e8c 100644 --- a/core/src/test/scala/pickling/run/sealed-trait-static-annotated.scala +++ b/core/src/test/scala/pickling/run/sealed-trait-static-annotated.scala @@ -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 } @@ -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 @@ -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) diff --git a/core/src/test/scala/pickling/run/sealed-trait-static.scala b/core/src/test/scala/pickling/run/sealed-trait-static.scala index b9d6f55a59..cf7ff607f8 100644 --- a/core/src/test/scala/pickling/run/sealed-trait-static.scala +++ b/core/src/test/scala/pickling/run/sealed-trait-static.scala @@ -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._ @@ -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 {