From 090d5565b74e312ddcbf25e69594ca0084426057 Mon Sep 17 00:00:00 2001 From: Ian O Connell Date: Sun, 14 Dec 2014 13:50:53 -0800 Subject: [PATCH 1/6] Collapse out duplicate macros/code between here and bijection-macros --- project/Build.scala | 13 +- .../scalding/macros/MacroImplicits.scala | 15 ++ .../com/twitter/scalding/macros/Macros.scala | 12 ++ .../scalding/macros/impl/MacroImpl.scala | 131 ++++++++++++++++++ .../scalding/macros/MacroDepHygiene.scala | 56 ++++++++ .../scalding/macros/MacrosUnitTests.scala | 92 ++++++++++++ 6 files changed, 318 insertions(+), 1 deletion(-) create mode 100644 scalding-macros/src/main/scala/com/twitter/scalding/macros/MacroImplicits.scala create mode 100644 scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala create mode 100644 scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala create mode 100644 scalding-macros/src/test/scala/com/twitter/scalding/macros/MacroDepHygiene.scala create mode 100644 scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala diff --git a/project/Build.scala b/project/Build.scala index a7704edb4e..5142e4196a 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -24,7 +24,7 @@ object ScaldingBuild extends Build { val scalaCheckVersion = "1.11.5" val hadoopVersion = "1.2.1" val algebirdVersion = "0.8.2" - val bijectionVersion = "0.7.0" + val bijectionVersion = "0.7.1-t1418593302000-98d44477c18e16145012a23beb0a58775a0fbe75" val chillVersion = "0.5.1" val slf4jVersion = "1.6.6" val parquetVersion = "1.6.0rc4" @@ -200,6 +200,7 @@ object ScaldingBuild extends Build { scaldingJson, scaldingJdbc, scaldingHadoopTest, + scaldingMacros, maple ) @@ -401,6 +402,16 @@ object ScaldingBuild extends Build { } ).dependsOn(scaldingCore) + lazy val scaldingMacros = module("macros").settings( + libraryDependencies <++= (scalaVersion) { scalaVersion => Seq( + "org.scala-lang" % "scala-library" % scalaVersion, + "org.scala-lang" % "scala-reflect" % scalaVersion, + "com.twitter" %% "bijection-macros" % bijectionVersion + ) ++ (if(isScala210x(scalaVersion)) Seq("org.scalamacros" %% "quasiquotes" % "2.0.1") else Seq()) + }, + addCompilerPlugin("org.scalamacros" % "paradise" % "2.0.1" cross CrossVersion.full) + ).dependsOn(scaldingCore, scaldingHadoopTest) + // This one uses a different naming convention lazy val maple = Project( id = "maple", diff --git a/scalding-macros/src/main/scala/com/twitter/scalding/macros/MacroImplicits.scala b/scalding-macros/src/main/scala/com/twitter/scalding/macros/MacroImplicits.scala new file mode 100644 index 0000000000..87be46da8d --- /dev/null +++ b/scalding-macros/src/main/scala/com/twitter/scalding/macros/MacroImplicits.scala @@ -0,0 +1,15 @@ +package com.twitter.scalding.macros + +import scala.language.experimental.macros + +import com.twitter.scalding._ +import com.twitter.scalding.macros.impl.MacroImpl +import com.twitter.bijection.macros.IsCaseClass + +object MacroImplicits { + /** + * This method provides proof that the given type is a case class. + */ + implicit def materializeCaseClassTupleSetter[T: IsCaseClass]: TupleSetter[T] = macro MacroImpl.caseClassTupleSetterImpl[T] + implicit def materializeCaseClassTupleConverter[T: IsCaseClass]: TupleConverter[T] = macro MacroImpl.caseClassTupleConverterImpl[T] +} diff --git a/scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala b/scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala new file mode 100644 index 0000000000..5c5ad8c106 --- /dev/null +++ b/scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala @@ -0,0 +1,12 @@ +package com.twitter.scalding.macros + +import scala.language.experimental.macros + +import com.twitter.scalding._ +import com.twitter.scalding.macros.impl.MacroImpl +import com.twitter.bijection.macros.IsCaseClass + +object Macros { + def caseClassTupleSetter[T: IsCaseClass]: TupleSetter[T] = macro MacroImpl.caseClassTupleSetterImpl[T] + def caseClassTupleConverter[T: IsCaseClass]: TupleConverter[T] = macro MacroImpl.caseClassTupleConverterImpl[T] +} diff --git a/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala new file mode 100644 index 0000000000..9bc26fc2a6 --- /dev/null +++ b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala @@ -0,0 +1,131 @@ +package com.twitter.scalding.macros.impl + +import scala.collection.mutable.{ Map => MMap } +import scala.language.experimental.macros +import scala.reflect.macros.Context +import scala.reflect.runtime.universe._ +import scala.util.{ Try => BasicTry } + +import cascading.tuple.{ Tuple, TupleEntry } + +import com.twitter.scalding._ +import com.twitter.bijection.macros.{ IsCaseClass, MacroGenerated } +import com.twitter.bijection.macros.impl.IsCaseClassImpl +/** + * This class contains the core macro implementations. This is in a separate module to allow it to be in + * a separate compilation unit, which makes it easier to provide helper methods interfacing with macros. + */ +object MacroImpl { + def caseClassTupleSetterNoProof[T]: TupleSetter[T] = macro caseClassTupleSetterNoProofImpl[T] + + def caseClassTupleSetterImpl[T](c: Context)(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T]): c.Expr[TupleSetter[T]] = + caseClassTupleSetterNoProofImpl(c)(T) + + def caseClassTupleSetterNoProofImpl[T](c: Context)(implicit T: c.WeakTypeTag[T]): c.Expr[TupleSetter[T]] = { + import c.universe._ + //TODO get rid of the mutability + val cachedTupleSetters: MMap[Type, Int] = MMap.empty + var cacheIdx = 0 + val set = + T.tpe.declarations + .collect { case m: MethodSymbol if m.isCaseAccessor => m } + .zipWithIndex + .map { + case (m, idx) => + m.returnType match { + case tpe if tpe =:= typeOf[String] => q"""tup.setString(${idx}, t.$m)""" + case tpe if tpe =:= typeOf[Boolean] => q"""tup.setBoolean(${idx}, t.$m)""" + case tpe if tpe =:= typeOf[Short] => q"""tup.setShort(${idx}, t.$m)""" + case tpe if tpe =:= typeOf[Int] => q"""tup.setInteger(${idx}, t.$m)""" + case tpe if tpe =:= typeOf[Long] => q"""tup.setLong(${idx}, t.$m)""" + case tpe if tpe =:= typeOf[Float] => q"""tup.setFloat(${idx}, t.$m)""" + case tpe if tpe =:= typeOf[Double] => q"""tup.setDouble(${idx}, t.$m)""" + case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => + val id = cachedTupleSetters.getOrElseUpdate(tpe, { cacheIdx += 1; cacheIdx }) + q"""tup.set(${idx}, ${newTermName("ts_" + id)}(t.$m))""" + case _ => q"""tup.set(${idx}, t.$m)""" + } + } + val tupleSetters = cachedTupleSetters.map { + case (tpe, id) => + q"""val ${newTermName("ts_" + id)} = _root_.com.twitter.scalding.macros.impl.MacroImpl.caseClassTupleSetterNoProof[$tpe]""" + }.toList + val res = q""" + _root_.com.twitter.scalding.macros.impl.MacroGeneratedTupleSetter[$T]( + { t: $T => + ..$tupleSetters + val tup = _root_.cascading.tuple.Tuple.size(${set.size}) + ..$set + tup + }, + ${set.size} + ) + """ + c.Expr[TupleSetter[T]](res) + } + + def caseClassTupleConverterNoProof[T]: TupleConverter[T] = macro caseClassTupleConverterNoProofImpl[T] + + def caseClassTupleConverterImpl[T](c: Context)(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T]): c.Expr[TupleConverter[T]] = + caseClassTupleConverterNoProofImpl(c)(T) + + def caseClassTupleConverterNoProofImpl[T](c: Context)(implicit T: c.WeakTypeTag[T]): c.Expr[TupleConverter[T]] = { + import c.universe._ + //TODO get rid of the mutability + val cachedTupleConverters: MMap[Type, Int] = MMap.empty + var cacheIdx = 0 + val get = + T.tpe.declarations + .collect { case m: MethodSymbol if m.isCaseAccessor => m.returnType } + .zipWithIndex + .map { + case (returnType, idx) => + returnType match { + case tpe if tpe =:= typeOf[String] => q"""tup.getString(${idx})""" + case tpe if tpe =:= typeOf[Boolean] => q"""tup.getBoolean(${idx})""" + case tpe if tpe =:= typeOf[Short] => q"""tup.getShort(${idx})""" + case tpe if tpe =:= typeOf[Int] => q"""tup.getInteger(${idx})""" + case tpe if tpe =:= typeOf[Long] => q"""tup.getLong(${idx})""" + case tpe if tpe =:= typeOf[Float] => q"""tup.getFloat(${idx})""" + case tpe if tpe =:= typeOf[Double] => q"""tup.getDouble(${idx})""" + case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => + val id = cachedTupleConverters.getOrElseUpdate(tpe, { cacheIdx += 1; cacheIdx }) + q""" + ${newTermName("tc_" + id)}( + new _root_.cascading.tuple.TupleEntry(tup.getObject(${idx}) + .asInstanceOf[_root_.cascading.tuple.Tuple]) + ) + """ + case tpe => q"""tup.getObject(${idx}).asInstanceOf[$tpe]""" + } + } + + val tupleConverters = cachedTupleConverters.map { + case (tpe, id) => + q"""val ${newTermName("tc_" + id)} = _root_.com.twitter.scalding.macros.impl.MacroImpl.caseClassTupleConverterNoProof[$tpe]""" + }.toList + val companion = T.tpe.typeSymbol.companionSymbol + val res = q""" + _root_.com.twitter.scalding.macros.impl.MacroGeneratedTupleConverter[$T]( + { t: _root_.cascading.tuple.TupleEntry => + ..$tupleConverters + val tup = t.getTuple() + $companion(..$get) + }, + ${get.size} + ) + """ + c.Expr[TupleConverter[T]](res) + } +} + +/** + * These traits allow us to inspect if a given TupleSetter of TupleConverter was generated. This is useful for + * avoiding LowPriorityTupleConverters.singleConverter + */ +case class MacroGeneratedTupleSetter[T](fn: T => Tuple, override val arity: Int) extends TupleSetter[T] with MacroGenerated { + override def apply(t: T) = fn(t) +} +case class MacroGeneratedTupleConverter[T](fn: TupleEntry => T, override val arity: Int) extends TupleConverter[T] with MacroGenerated { + override def apply(t: TupleEntry) = fn(t) +} diff --git a/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacroDepHygiene.scala b/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacroDepHygiene.scala new file mode 100644 index 0000000000..d06decc070 --- /dev/null +++ b/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacroDepHygiene.scala @@ -0,0 +1,56 @@ +package com.twitter.scalding.macros + +import org.scalatest.WordSpec +import com.twitter.scalding.macros.{ _ => _ } + +/** + * This test is intended to ensure that the macros do not require any imported code in scope. This is why all + * references are via absolute paths. + */ +class MacroDepHygiene extends WordSpec { + import com.twitter.bijection.macros.impl.IsCaseClassImpl + + case class A(x: Int, y: String) + case class B(x: A, y: String, z: A) + class C + + def isMg(a: Any) = a.isInstanceOf[com.twitter.bijection.macros.MacroGenerated] + + "TupleSetter macro" should { + def isTupleSetterAvailable[T](implicit proof: com.twitter.scalding.TupleSetter[T]) = isMg(proof) + + "work fine without any imports" in { + com.twitter.scalding.macros.Macros.caseClassTupleSetter[A] + com.twitter.scalding.macros.Macros.caseClassTupleSetter[B] + } + + "implicitly work fine without any imports" in { + import com.twitter.scalding.macros.MacroImplicits.materializeCaseClassTupleSetter + assert(isTupleSetterAvailable[A]) + assert(isTupleSetterAvailable[B]) + } + + "fail if not a case class" in { + assert(!isTupleSetterAvailable[C]) + } + } + + "TupleConverter macro" should { + def isTupleConverterAvailable[T](implicit proof: com.twitter.scalding.TupleConverter[T]) = isMg(proof) + + "work fine without any imports" in { + com.twitter.scalding.macros.Macros.caseClassTupleConverter[A] + com.twitter.scalding.macros.Macros.caseClassTupleConverter[B] + } + + "implicitly work fine without any imports" in { + import com.twitter.scalding.macros.MacroImplicits.materializeCaseClassTupleConverter + assert(isTupleConverterAvailable[A]) + assert(isTupleConverterAvailable[B]) + } + + "fail if not a case class" in { + assert(!isTupleConverterAvailable[C]) + } + } +} diff --git a/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala b/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala new file mode 100644 index 0000000000..3f4de59196 --- /dev/null +++ b/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala @@ -0,0 +1,92 @@ +package com.twitter.scalding.macros + +import cascading.tuple.{ Tuple => CTuple, TupleEntry } + +import org.scalatest.{ Matchers, WordSpec } + +import com.twitter.scalding._ +import com.twitter.scalding.macros._ +import com.twitter.scalding.macros.impl._ +import com.twitter.scalding.serialization.Externalizer + +import com.twitter.bijection.macros.{ IsCaseClass, MacroGenerated } + +// We avoid nesting these just to avoid any complications in the serialization test +case class A(x: Int, y: String) +case class B(a1: A, a2: A, y: String) +case class C(a: A, b: B, c: A, d: B, e: B) + +class MacrosUnitTests extends WordSpec with Matchers { + import MacroImplicits._ + def isMg[T](t: T): T = { + t shouldBe a[MacroGenerated] + t + } + + def mgConv[T](te: TupleEntry)(implicit conv: TupleConverter[T]): T = isMg(conv)(te) + def mgSet[T](t: T)(implicit set: TupleSetter[T]): TupleEntry = new TupleEntry(isMg(set)(t)) + + def shouldRoundTrip[T: IsCaseClass: TupleSetter: TupleConverter](t: T) { + t shouldBe mgConv(mgSet(t)) + } + + def shouldRoundTripOther[T: IsCaseClass: TupleSetter: TupleConverter](te: TupleEntry, t: T) { + val inter = mgConv(te) + inter shouldBe t + mgSet(inter) shouldBe te + } + + def canExternalize(t: AnyRef) { Externalizer(t).javaWorks shouldBe true } + + "MacroGenerated TupleSetter" should { + def doesJavaWork[T](implicit set: TupleSetter[T]) { canExternalize(isMg(set)) } + "be serializable for case class A" in { doesJavaWork[A] } + "be serializable for case class B" in { doesJavaWork[B] } + "be serializable for case class C" in { doesJavaWork[C] } + } + + "MacroGenerated TupleConverter" should { + def doesJavaWork[T](implicit conv: TupleConverter[T]) { canExternalize(isMg(conv)) } + "be serializable for case class A" in { doesJavaWork[A] } + "be serializable for case class B" in { doesJavaWork[B] } + "be serializable for case class C" in { doesJavaWork[C] } + } + + "MacroGenerated TupleSetter and TupleConverter" should { + "round trip class -> tupleentry -> class" in { + shouldRoundTrip(A(100, "onehundred")) + shouldRoundTrip(B(A(100, "onehundred"), A(-1, "zero"), "what")) + val a = A(73, "hrm") + val b = B(a, a, "hrm") + shouldRoundTrip(b) + shouldRoundTrip(C(a, b, A(123980, "hey"), B(a, A(-1, "zero"), "zoo"), b)) + } + + "round trip tupleentry -> class -> tupleEntry" in { + val a_tup = CTuple.size(2) + a_tup.setInteger(0, 100) + a_tup.setString(1, "onehundred") + val a_te = new TupleEntry(a_tup) + val a = A(100, "onehundred") + shouldRoundTripOther(a_te, a) + + val b_tup = CTuple.size(3) + b_tup.set(0, a_tup) + b_tup.set(1, a_tup) + b_tup.setString(2, "what") + val b_te = new TupleEntry(b_tup) + val b = B(a, a, "what") + shouldRoundTripOther(b_te, b) + + val c_tup = CTuple.size(5) + c_tup.set(0, a_tup) + c_tup.set(1, b_tup) + c_tup.set(2, a_tup) + c_tup.set(3, b_tup) + c_tup.set(4, b_tup) + val c_te = new TupleEntry(c_tup) + val c = C(a, b, a, b, b) + shouldRoundTripOther(c_te, c) + } + } +} From 510dc668acfccb4a741bdb9b309488487dfb028e Mon Sep 17 00:00:00 2001 From: Ian O Connell Date: Mon, 15 Dec 2014 10:40:51 -0800 Subject: [PATCH 2/6] Convert over to fully unrolling the case class --- .../scalding/macros/impl/MacroImpl.scala | 157 ++++++++++-------- .../scalding/macros/MacrosUnitTests.scala | 88 +++++++--- 2 files changed, 145 insertions(+), 100 deletions(-) diff --git a/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala index 9bc26fc2a6..1ba671e032 100644 --- a/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala +++ b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala @@ -26,40 +26,44 @@ object MacroImpl { //TODO get rid of the mutability val cachedTupleSetters: MMap[Type, Int] = MMap.empty var cacheIdx = 0 + + def expandMethod(mSeq: Iterable[MethodSymbol], pTree: Tree): Iterable[Int => Tree] = { + mSeq.flatMap { accessorMethod => + accessorMethod.returnType match { + case tpe if tpe =:= typeOf[String] => List((idx: Int) => q"""tup.setString(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Boolean] => List((idx: Int) => q"""tup.setBoolean(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Short] => List((idx: Int) => q"""tup.setShort(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Int] => List((idx: Int) => q"""tup.setInteger(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Long] => List((idx: Int) => q"""tup.setLong(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Float] => List((idx: Int) => q"""tup.setFloat(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Double] => List((idx: Int) => q"""tup.setDouble(${idx}, $pTree.$accessorMethod)""") + case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => + expandMethod(tpe.declarations + .collect { case m: MethodSymbol if m.isCaseAccessor => m }, + q"""$pTree.$accessorMethod""") + case _ => List((idx: Int) => q"""tup.set(${idx}, t.$accessorMethod)""") + } + } + } + val set = - T.tpe.declarations - .collect { case m: MethodSymbol if m.isCaseAccessor => m } + expandMethod(T.tpe.declarations + .collect { case m: MethodSymbol if m.isCaseAccessor => m }, q"t") .zipWithIndex .map { - case (m, idx) => - m.returnType match { - case tpe if tpe =:= typeOf[String] => q"""tup.setString(${idx}, t.$m)""" - case tpe if tpe =:= typeOf[Boolean] => q"""tup.setBoolean(${idx}, t.$m)""" - case tpe if tpe =:= typeOf[Short] => q"""tup.setShort(${idx}, t.$m)""" - case tpe if tpe =:= typeOf[Int] => q"""tup.setInteger(${idx}, t.$m)""" - case tpe if tpe =:= typeOf[Long] => q"""tup.setLong(${idx}, t.$m)""" - case tpe if tpe =:= typeOf[Float] => q"""tup.setFloat(${idx}, t.$m)""" - case tpe if tpe =:= typeOf[Double] => q"""tup.setDouble(${idx}, t.$m)""" - case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => - val id = cachedTupleSetters.getOrElseUpdate(tpe, { cacheIdx += 1; cacheIdx }) - q"""tup.set(${idx}, ${newTermName("ts_" + id)}(t.$m))""" - case _ => q"""tup.set(${idx}, t.$m)""" - } + case (treeGenerator, idx) => + treeGenerator(idx) } - val tupleSetters = cachedTupleSetters.map { - case (tpe, id) => - q"""val ${newTermName("ts_" + id)} = _root_.com.twitter.scalding.macros.impl.MacroImpl.caseClassTupleSetterNoProof[$tpe]""" - }.toList + val res = q""" - _root_.com.twitter.scalding.macros.impl.MacroGeneratedTupleSetter[$T]( - { t: $T => - ..$tupleSetters + new _root_.com.twitter.scalding.TupleSetter[$T] with _root_.com.twitter.bijection.macros.MacroGenerated { + override def apply(t: $T): _root_.cascading.tuple.Tuple = { val tup = _root_.cascading.tuple.Tuple.size(${set.size}) ..$set tup - }, - ${set.size} - ) + } + override val arity: scala.Int = ${set.size} + } """ c.Expr[TupleSetter[T]](res) } @@ -74,58 +78,65 @@ object MacroImpl { //TODO get rid of the mutability val cachedTupleConverters: MMap[Type, Int] = MMap.empty var cacheIdx = 0 - val get = - T.tpe.declarations + case class AccessorBuilder(builder: Tree, size: Int) + + def getPrimitive(strAccessor: Tree): Int => AccessorBuilder = + { (idx: Int) => + AccessorBuilder(q"""${strAccessor}(${idx})""", 1) + } + + def flattenAccessorBuilders(tpe: Type, idx: Int, childGetters: List[(Int => AccessorBuilder)]): AccessorBuilder = { + val (_, accessors) = childGetters.foldLeft((idx, List[AccessorBuilder]())) { + case ((curIdx, eles), t) => + val nextEle = t(curIdx) + val idxIncr = nextEle.size + (curIdx + idxIncr, eles :+ nextEle) + } + + val builder = q""" + ${tpe.typeSymbol.companionSymbol}(..${accessors.map(_.builder)}) + """ + val size = accessors.map(_.size).reduce(_ + _) + AccessorBuilder(builder, size) + } + + def expandMethod(outerTpe: Type): List[(Int => AccessorBuilder)] = { + outerTpe.declarations .collect { case m: MethodSymbol if m.isCaseAccessor => m.returnType } - .zipWithIndex - .map { - case (returnType, idx) => - returnType match { - case tpe if tpe =:= typeOf[String] => q"""tup.getString(${idx})""" - case tpe if tpe =:= typeOf[Boolean] => q"""tup.getBoolean(${idx})""" - case tpe if tpe =:= typeOf[Short] => q"""tup.getShort(${idx})""" - case tpe if tpe =:= typeOf[Int] => q"""tup.getInteger(${idx})""" - case tpe if tpe =:= typeOf[Long] => q"""tup.getLong(${idx})""" - case tpe if tpe =:= typeOf[Float] => q"""tup.getFloat(${idx})""" - case tpe if tpe =:= typeOf[Double] => q"""tup.getDouble(${idx})""" - case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => - val id = cachedTupleConverters.getOrElseUpdate(tpe, { cacheIdx += 1; cacheIdx }) - q""" - ${newTermName("tc_" + id)}( - new _root_.cascading.tuple.TupleEntry(tup.getObject(${idx}) - .asInstanceOf[_root_.cascading.tuple.Tuple]) - ) - """ - case tpe => q"""tup.getObject(${idx}).asInstanceOf[$tpe]""" - } + .toList + .map { accessorMethod => + accessorMethod match { + case tpe if tpe =:= typeOf[String] => getPrimitive(q"t.getString") + case tpe if tpe =:= typeOf[Boolean] => getPrimitive(q"t.Boolean") + case tpe if tpe =:= typeOf[Short] => getPrimitive(q"t.getShort") + case tpe if tpe =:= typeOf[Int] => getPrimitive(q"t.getInteger") + case tpe if tpe =:= typeOf[Long] => getPrimitive(q"t.getLong") + case tpe if tpe =:= typeOf[Float] => getPrimitive(q"t.getFloat") + case tpe if tpe =:= typeOf[Double] => getPrimitive(q"t.getDouble") + case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => + { (idx: Int) => + { + val childGetters = expandMethod(tpe) + flattenAccessorBuilders(tpe, idx, childGetters) + } + } + case tpe => + ((idx: Int) => + AccessorBuilder(q"""t.getObject(${idx}).asInstanceOf[$tpe]""", 1)) + } } + } + + val accessorBuilders = flattenAccessorBuilders(T.tpe, 0, expandMethod(T.tpe)) - val tupleConverters = cachedTupleConverters.map { - case (tpe, id) => - q"""val ${newTermName("tc_" + id)} = _root_.com.twitter.scalding.macros.impl.MacroImpl.caseClassTupleConverterNoProof[$tpe]""" - }.toList - val companion = T.tpe.typeSymbol.companionSymbol val res = q""" - _root_.com.twitter.scalding.macros.impl.MacroGeneratedTupleConverter[$T]( - { t: _root_.cascading.tuple.TupleEntry => - ..$tupleConverters - val tup = t.getTuple() - $companion(..$get) - }, - ${get.size} - ) + new _root_.com.twitter.scalding.TupleConverter[$T] with _root_.com.twitter.bijection.macros.MacroGenerated { + override def apply(t: _root_.cascading.tuple.TupleEntry): $T = { + ${accessorBuilders.builder} + } + override val arity: scala.Int = ${accessorBuilders.size} + } """ c.Expr[TupleConverter[T]](res) } } - -/** - * These traits allow us to inspect if a given TupleSetter of TupleConverter was generated. This is useful for - * avoiding LowPriorityTupleConverters.singleConverter - */ -case class MacroGeneratedTupleSetter[T](fn: T => Tuple, override val arity: Int) extends TupleSetter[T] with MacroGenerated { - override def apply(t: T) = fn(t) -} -case class MacroGeneratedTupleConverter[T](fn: TupleEntry => T, override val arity: Int) extends TupleConverter[T] with MacroGenerated { - override def apply(t: TupleEntry) = fn(t) -} diff --git a/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala b/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala index 3f4de59196..f08f99b699 100644 --- a/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala +++ b/scalding-macros/src/test/scala/com/twitter/scalding/macros/MacrosUnitTests.scala @@ -12,9 +12,9 @@ import com.twitter.scalding.serialization.Externalizer import com.twitter.bijection.macros.{ IsCaseClass, MacroGenerated } // We avoid nesting these just to avoid any complications in the serialization test -case class A(x: Int, y: String) -case class B(a1: A, a2: A, y: String) -case class C(a: A, b: B, c: A, d: B, e: B) +case class SampleClassA(x: Int, y: String) +case class SampleClassB(a1: SampleClassA, a2: SampleClassA, y: String) +case class SampleClassC(a: SampleClassA, b: SampleClassB, c: SampleClassA, d: SampleClassB, e: SampleClassB) class MacrosUnitTests extends WordSpec with Matchers { import MacroImplicits._ @@ -40,26 +40,39 @@ class MacrosUnitTests extends WordSpec with Matchers { "MacroGenerated TupleSetter" should { def doesJavaWork[T](implicit set: TupleSetter[T]) { canExternalize(isMg(set)) } - "be serializable for case class A" in { doesJavaWork[A] } - "be serializable for case class B" in { doesJavaWork[B] } - "be serializable for case class C" in { doesJavaWork[C] } + "be serializable for case class A" in { doesJavaWork[SampleClassA] } + "be serializable for case class B" in { doesJavaWork[SampleClassB] } + "be serializable for case class C" in { doesJavaWork[SampleClassC] } } "MacroGenerated TupleConverter" should { def doesJavaWork[T](implicit conv: TupleConverter[T]) { canExternalize(isMg(conv)) } - "be serializable for case class A" in { doesJavaWork[A] } - "be serializable for case class B" in { doesJavaWork[B] } - "be serializable for case class C" in { doesJavaWork[C] } + "be serializable for case class A" in { doesJavaWork[SampleClassA] } + "be serializable for case class B" in { doesJavaWork[SampleClassB] } + "be serializable for case class C" in { doesJavaWork[SampleClassC] } } "MacroGenerated TupleSetter and TupleConverter" should { "round trip class -> tupleentry -> class" in { - shouldRoundTrip(A(100, "onehundred")) - shouldRoundTrip(B(A(100, "onehundred"), A(-1, "zero"), "what")) - val a = A(73, "hrm") - val b = B(a, a, "hrm") + shouldRoundTrip(SampleClassA(100, "onehundred")) + shouldRoundTrip(SampleClassB(SampleClassA(100, "onehundred"), SampleClassA(-1, "zero"), "what")) + val a = SampleClassA(73, "hrm") + val b = SampleClassB(a, a, "hrm") shouldRoundTrip(b) - shouldRoundTrip(C(a, b, A(123980, "hey"), B(a, A(-1, "zero"), "zoo"), b)) + shouldRoundTrip(SampleClassC(a, b, SampleClassA(123980, "hey"), SampleClassB(a, SampleClassA(-1, "zero"), "zoo"), b)) + } + + "Case Class should form expected tuple" in { + val input = SampleClassC(SampleClassA(1, "asdf"), + SampleClassB(SampleClassA(2, "bcdf"), SampleClassA(5, "jkfs"), "wetew"), + SampleClassA(9, "xcmv"), + SampleClassB(SampleClassA(23, "ck"), SampleClassA(13, "dafk"), "xcv"), + SampleClassB(SampleClassA(34, "were"), SampleClassA(654, "power"), "adsfmx")) + val setter = implicitly[TupleSetter[SampleClassC]] + val tup = setter(input) + assert(tup.size == 19) + assert(tup.get(0) === 1) + assert(tup.get(18) === "adsfmx") } "round trip tupleentry -> class -> tupleEntry" in { @@ -67,25 +80,46 @@ class MacrosUnitTests extends WordSpec with Matchers { a_tup.setInteger(0, 100) a_tup.setString(1, "onehundred") val a_te = new TupleEntry(a_tup) - val a = A(100, "onehundred") + val a = SampleClassA(100, "onehundred") shouldRoundTripOther(a_te, a) - val b_tup = CTuple.size(3) - b_tup.set(0, a_tup) - b_tup.set(1, a_tup) - b_tup.setString(2, "what") + val b_tup = CTuple.size(5) + b_tup.setInteger(0, 100) + b_tup.setString(1, "onehundred") + b_tup.setInteger(2, 100) + b_tup.setString(3, "onehundred") + b_tup.setString(4, "what") val b_te = new TupleEntry(b_tup) - val b = B(a, a, "what") + val b = SampleClassB(a, a, "what") shouldRoundTripOther(b_te, b) - val c_tup = CTuple.size(5) - c_tup.set(0, a_tup) - c_tup.set(1, b_tup) - c_tup.set(2, a_tup) - c_tup.set(3, b_tup) - c_tup.set(4, b_tup) + val c_tup = CTuple.size(19) + c_tup.setInteger(0, 100) + c_tup.setString(1, "onehundred") + + c_tup.setInteger(2, 100) + c_tup.setString(3, "onehundred") + c_tup.setInteger(4, 100) + c_tup.setString(5, "onehundred") + c_tup.setString(6, "what") + + c_tup.setInteger(7, 100) + c_tup.setString(8, "onehundred") + + c_tup.setInteger(9, 100) + c_tup.setString(10, "onehundred") + c_tup.setInteger(11, 100) + c_tup.setString(12, "onehundred") + c_tup.setString(13, "what") + + c_tup.setInteger(14, 100) + c_tup.setString(15, "onehundred") + c_tup.setInteger(16, 100) + c_tup.setString(17, "onehundred") + c_tup.setString(18, "what") + val c_te = new TupleEntry(c_tup) - val c = C(a, b, a, b, b) + val c = SampleClassC(a, b, a, b, b) shouldRoundTripOther(c_te, c) } } From c4a86cf90f1740a09cc3175ebd7a0ccfc084201a Mon Sep 17 00:00:00 2001 From: Ian O Connell Date: Mon, 15 Dec 2014 12:06:36 -0800 Subject: [PATCH 3/6] Review comment --- .../scalding/macros/impl/MacroImpl.scala | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala index 1ba671e032..4aa7dcc2f8 100644 --- a/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala +++ b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala @@ -27,28 +27,29 @@ object MacroImpl { val cachedTupleSetters: MMap[Type, Int] = MMap.empty var cacheIdx = 0 - def expandMethod(mSeq: Iterable[MethodSymbol], pTree: Tree): Iterable[Int => Tree] = { - mSeq.flatMap { accessorMethod => - accessorMethod.returnType match { - case tpe if tpe =:= typeOf[String] => List((idx: Int) => q"""tup.setString(${idx}, $pTree.$accessorMethod)""") - case tpe if tpe =:= typeOf[Boolean] => List((idx: Int) => q"""tup.setBoolean(${idx}, $pTree.$accessorMethod)""") - case tpe if tpe =:= typeOf[Short] => List((idx: Int) => q"""tup.setShort(${idx}, $pTree.$accessorMethod)""") - case tpe if tpe =:= typeOf[Int] => List((idx: Int) => q"""tup.setInteger(${idx}, $pTree.$accessorMethod)""") - case tpe if tpe =:= typeOf[Long] => List((idx: Int) => q"""tup.setLong(${idx}, $pTree.$accessorMethod)""") - case tpe if tpe =:= typeOf[Float] => List((idx: Int) => q"""tup.setFloat(${idx}, $pTree.$accessorMethod)""") - case tpe if tpe =:= typeOf[Double] => List((idx: Int) => q"""tup.setDouble(${idx}, $pTree.$accessorMethod)""") - case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => - expandMethod(tpe.declarations - .collect { case m: MethodSymbol if m.isCaseAccessor => m }, - q"""$pTree.$accessorMethod""") - case _ => List((idx: Int) => q"""tup.set(${idx}, t.$accessorMethod)""") + def expandMethod(outerTpe: Type, pTree: Tree): Iterable[Int => Tree] = { + outerTpe + .declarations + .collect { case m: MethodSymbol if m.isCaseAccessor => m } + .flatMap { accessorMethod => + accessorMethod.returnType match { + case tpe if tpe =:= typeOf[String] => List((idx: Int) => q"""tup.setString(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Boolean] => List((idx: Int) => q"""tup.setBoolean(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Short] => List((idx: Int) => q"""tup.setShort(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Int] => List((idx: Int) => q"""tup.setInteger(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Long] => List((idx: Int) => q"""tup.setLong(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Float] => List((idx: Int) => q"""tup.setFloat(${idx}, $pTree.$accessorMethod)""") + case tpe if tpe =:= typeOf[Double] => List((idx: Int) => q"""tup.setDouble(${idx}, $pTree.$accessorMethod)""") + case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => + expandMethod(tpe, + q"""$pTree.$accessorMethod""") + case _ => List((idx: Int) => q"""tup.set(${idx}, t.$accessorMethod)""") + } } - } } val set = - expandMethod(T.tpe.declarations - .collect { case m: MethodSymbol if m.isCaseAccessor => m }, q"t") + expandMethod(T.tpe, q"t") .zipWithIndex .map { case (treeGenerator, idx) => From 2e50e021b3a61625145cec47c194ab3450db6268 Mon Sep 17 00:00:00 2001 From: Ian O Connell Date: Mon, 15 Dec 2014 12:09:52 -0800 Subject: [PATCH 4/6] Abort on unknown type --- .../scala/com/twitter/scalding/macros/impl/MacroImpl.scala | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala index 4aa7dcc2f8..74c09c0c22 100644 --- a/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala +++ b/scalding-macros/src/main/scala/com/twitter/scalding/macros/impl/MacroImpl.scala @@ -43,7 +43,7 @@ object MacroImpl { case tpe if IsCaseClassImpl.isCaseClassType(c)(tpe) => expandMethod(tpe, q"""$pTree.$accessorMethod""") - case _ => List((idx: Int) => q"""tup.set(${idx}, t.$accessorMethod)""") + case _ => c.abort(c.enclosingPosition, s"Case class ${T} is not pure primitives or nested case classes") } } } @@ -121,9 +121,7 @@ object MacroImpl { flattenAccessorBuilders(tpe, idx, childGetters) } } - case tpe => - ((idx: Int) => - AccessorBuilder(q"""t.getObject(${idx}).asInstanceOf[$tpe]""", 1)) + case _ => c.abort(c.enclosingPosition, s"Case class ${T} is not pure primitives or nested case classes") } } } From c82ab8cb09c804e5f0a85d69f862a689471be850 Mon Sep 17 00:00:00 2001 From: Ian O Connell Date: Mon, 15 Dec 2014 12:27:33 -0800 Subject: [PATCH 5/6] Add a comment about restrictions on the type --- .../src/main/scala/com/twitter/scalding/macros/Macros.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala b/scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala index 5c5ad8c106..6e733dbf94 100644 --- a/scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala +++ b/scalding-macros/src/main/scala/com/twitter/scalding/macros/Macros.scala @@ -7,6 +7,8 @@ import com.twitter.scalding.macros.impl.MacroImpl import com.twitter.bijection.macros.IsCaseClass object Macros { + // These only work for simple types inside the case class + // Nested case classes are allowed, but only: Int, Boolean, String, Long, Short, Float, Double of other types are allowed def caseClassTupleSetter[T: IsCaseClass]: TupleSetter[T] = macro MacroImpl.caseClassTupleSetterImpl[T] def caseClassTupleConverter[T: IsCaseClass]: TupleConverter[T] = macro MacroImpl.caseClassTupleConverterImpl[T] } From af7cb5240b4fa0c47465575b5ec4913fdb972bca Mon Sep 17 00:00:00 2001 From: Ian O Connell Date: Mon, 15 Dec 2014 13:08:46 -0800 Subject: [PATCH 6/6] Released bijection --- project/Build.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Build.scala b/project/Build.scala index 5142e4196a..013d4eea0d 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -24,7 +24,7 @@ object ScaldingBuild extends Build { val scalaCheckVersion = "1.11.5" val hadoopVersion = "1.2.1" val algebirdVersion = "0.8.2" - val bijectionVersion = "0.7.1-t1418593302000-98d44477c18e16145012a23beb0a58775a0fbe75" + val bijectionVersion = "0.7.1" val chillVersion = "0.5.1" val slf4jVersion = "1.6.6" val parquetVersion = "1.6.0rc4"