-
Notifications
You must be signed in to change notification settings - Fork 123
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
Ianoc/refactor macros #186
Changes from 6 commits
07bbf81
efa1e57
0704c04
a16676f
536e789
17cce9f
7106f33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,9 @@ package com.twitter.bijection.macros | |
import scala.language.experimental.macros | ||
|
||
import com.twitter.bijection._ | ||
import com.twitter.bijection.macros.common.IsCaseClass | ||
import com.twitter.bijection.macros.impl.MacroImpl | ||
import com.twitter.bijection.macros.impl.{ CaseClassToTuple, CaseClassToMap } | ||
|
||
object Macros { | ||
def caseClassToTuple[T: IsCaseClass, Tup](recursivelyApply: Boolean = true): Bijection[T, Tup] = macro MacroImpl.caseClassToTupleImplWithOption[T, Tup] | ||
def caseClassToMap[T: IsCaseClass](recursivelyApply: Boolean = true): Injection[T, Map[String, Any]] = macro MacroImpl.caseClassToMapImplWithOption[T] | ||
def caseClassToTuple[T: IsCaseClass, Tup](recursivelyApply: Boolean = true): Bijection[T, Tup] = macro CaseClassToTuple.caseClassToTupleImplWithOption[T, Tup] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when I tried to call these macros, it says it could not use a default argument in the repl. Does this work for you in the REPL There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you verify that calling using the default argument works (in the REPL)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't... default args don't work in macros i believe... Will nuke. |
||
def caseClassToMap[T: IsCaseClass](recursivelyApply: Boolean = true): Injection[T, Map[String, Any]] = macro CaseClassToMap.caseClassToMapImplWithOption[T] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
package com.twitter.bijection.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 | ||
|
||
import com.twitter.bijection._ | ||
import com.twitter.bijection.macros.{ IsCaseClass, MacroGenerated } | ||
|
||
private[bijection] object CaseClassToMap { | ||
def caseClassToMapImplWithOption[T](c: Context)(recursivelyApply: c.Expr[Boolean])(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T]): c.Expr[Injection[T, Map[String, Any]]] = { | ||
import c.universe._ | ||
recursivelyApply.tree match { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the right way to match? Can't we also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i guess most of the ones i'd seen were around pattering mattering against the expression we've gotten. We could also evaluate it. I'm not really sure which is preferable. Here i guess its a toss up? |
||
case q"""true""" => caseClassToMapNoProofImpl(c)(T) | ||
case q"""false""" => caseClassToMapNoProofImplNonRecursive(c)(T) | ||
} | ||
} | ||
|
||
def caseClassToMapImpl[T](c: Context)(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T]): c.Expr[Injection[T, Map[String, Any]]] = | ||
caseClassToMapNoProofImpl(c)(T) | ||
|
||
def caseClassToMapImplNonRecursive[T](c: Context)(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T]): c.Expr[Injection[T, Map[String, Any]]] = | ||
caseClassToMapNoProofImplNonRecursive(c)(T) | ||
|
||
def caseClassToMapNoProofImplNonRecursive[T](c: Context)(implicit T: c.WeakTypeTag[T]): c.Expr[Injection[T, Map[String, Any]]] = caseClassToMapNoProofImplCommon(c, false)(T) | ||
|
||
// TODO the only diff between this and the above is the case match and the converters. it's easy to gate this on the boolean | ||
def caseClassToMapNoProofImpl[T](c: Context)(implicit T: c.WeakTypeTag[T]): c.Expr[Injection[T, Map[String, Any]]] = caseClassToMapNoProofImplCommon(c, true)(T) | ||
|
||
def caseClassToMapNoProofImplCommon[T](c: Context, recursivelyApply: Boolean)(implicit T: c.WeakTypeTag[T]): c.Expr[Injection[T, Map[String, Any]]] = { | ||
import c.universe._ | ||
//TODO can make error handling better? | ||
val companion = T.tpe.typeSymbol.companionSymbol | ||
|
||
val getPutConv = T.tpe.declarations.collect { case m: MethodSymbol if m.isCaseAccessor => m }.zipWithIndex.map { | ||
case (m, idx) => | ||
val returnType = m.returnType | ||
val accStr = m.name.toTermName.toString | ||
returnType match { | ||
case tpe if recursivelyApply && IsCaseClassImpl.isCaseClassType(c)(tpe) => | ||
val conv = newTermName("c2m_" + idx) | ||
(q"""$conv.invert(m($accStr).asInstanceOf[_root_.scala.collection.immutable.Map[String, Any]]).get""", | ||
q"""($accStr, $conv(t.$m))""", | ||
Some(q"""val $conv = implicitly[_root_.com.twitter.bijection.Injection[$tpe, _root_.scala.collection.immutable.Map[String, Any]]]""")) //TODO cache these | ||
case tpe => | ||
(q"""m($accStr).asInstanceOf[$returnType]""", | ||
q"""($accStr, t.$m)""", | ||
None) | ||
} | ||
} | ||
|
||
val getters = getPutConv.map(_._1) | ||
val putters = getPutConv.map(_._2) | ||
val converters = getPutConv.flatMap(_._3) | ||
|
||
c.Expr[Injection[T, Map[String, Any]]](q""" | ||
new Injection[$T, _root_.scala.collection.immutable.Map[String, Any]] with MacroGenerated { | ||
override def apply(t: $T): _root_.scala.collection.immutable.Map[String, Any] = { | ||
..$converters | ||
_root_.scala.collection.immutable.Map[String, Any](..$putters) | ||
} | ||
override def invert(m: _root_.scala.collection.immutable.Map[String, Any]): _root_.scala.util.Try[ $T ] = { | ||
..$converters | ||
try { _root_.scala.util.Success($companion(..$getters)) } catch { case _root_.scala.util.control.NonFatal(e) => _root_.scala.util.Failure(e) } | ||
} | ||
} | ||
""") | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package com.twitter.bijection.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 | ||
|
||
import com.twitter.bijection._ | ||
import com.twitter.bijection.macros.{ IsCaseClass, MacroGenerated } | ||
|
||
private[bijection] object CaseClassToTuple { | ||
def caseClassToTupleImplWithOption[T, Tup](c: Context)(recursivelyApply: c.Expr[Boolean])(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T], Tup: c.WeakTypeTag[Tup]): c.Expr[Bijection[T, Tup]] = { | ||
import c.universe._ | ||
recursivelyApply match { | ||
case q"""true""" => caseClassToTupleNoProofImpl(c)(T, Tup) | ||
case q"""false""" => caseClassToTupleNoProofImplNonRecursive(c)(T, Tup) | ||
case _ => caseClassToTupleNoProofImpl(c)(T, Tup) | ||
} | ||
} | ||
|
||
// Entry point | ||
def caseClassToTupleImpl[T, Tup](c: Context)(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T], Tup: c.WeakTypeTag[Tup]): c.Expr[Bijection[T, Tup]] = | ||
caseClassToTupleNoProofImpl(c)(T, Tup) | ||
|
||
def caseClassToTupleImplNonRecursive[T, Tup](c: Context)(proof: c.Expr[IsCaseClass[T]])(implicit T: c.WeakTypeTag[T], Tup: c.WeakTypeTag[Tup]): c.Expr[Bijection[T, Tup]] = | ||
caseClassToTupleNoProofImplNonRecursive(c)(T, Tup) | ||
|
||
def caseClassToTupleNoProofImplNonRecursive[T, Tup](c: Context)(implicit T: c.WeakTypeTag[T], | ||
Tup: c.WeakTypeTag[Tup]): c.Expr[Bijection[T, Tup]] = | ||
caseClassToTupleNoProofImplCommon(c, false)(T, Tup) | ||
|
||
def caseClassToTupleNoProofImpl[T, Tup](c: Context)(implicit T: c.WeakTypeTag[T], | ||
Tup: c.WeakTypeTag[Tup]): c.Expr[Bijection[T, Tup]] = | ||
caseClassToTupleNoProofImplCommon(c, true)(T, Tup) | ||
|
||
def caseClassToTupleNoProofImplCommon[T, Tup](c: Context, | ||
recursivelyApply: Boolean)(implicit T: c.WeakTypeTag[T], | ||
Tup: c.WeakTypeTag[Tup]): c.Expr[Bijection[T, Tup]] = { | ||
import c.universe._ | ||
val tupUtils = new TupleUtils[c.type](c) | ||
val convCache = MMap.empty[Type, TermName] | ||
|
||
//TODO can make error handling better | ||
val companion = T.tpe.typeSymbol.companionSymbol | ||
val getPutConv = T.tpe | ||
.declarations | ||
.collect { case m: MethodSymbol if m.isCaseAccessor => m } | ||
.zip(tupUtils.tupleCaseClassEquivalent(T.tpe)) | ||
.zip(Tup.tpe.declarations.collect { case m: MethodSymbol if m.isCaseAccessor => m }) | ||
.zipWithIndex | ||
.map { | ||
case (((tM, treeEquiv), tupM), idx) => | ||
tM.returnType match { | ||
case tpe if recursivelyApply && IsCaseClassImpl.isCaseClassType(c)(tpe) => | ||
val needDeclaration = !convCache.contains(tpe) | ||
val conv = convCache.getOrElseUpdate(tpe, newTermName("c2t_" + idx)) | ||
(q"""$conv.invert(tup.$tupM)""", | ||
q"""$conv(t.$tM)""", | ||
if (needDeclaration) Some(q"""val $conv = implicitly[_root_.com.twitter.bijection.Bijection[${tM.returnType}, $treeEquiv]]""") else None) // cache these | ||
case tpe => | ||
(q"""tup.$tupM""", | ||
q"""t.$tM""", | ||
None) | ||
} | ||
} | ||
|
||
val getters = getPutConv.map(_._1) | ||
val putters = getPutConv.map(_._2) | ||
val converters = getPutConv.flatMap(_._3) | ||
|
||
c.Expr[Bijection[T, Tup]](q""" | ||
new Bijection[$T,$Tup] with MacroGenerated { | ||
override def apply(t: $T): $Tup = { | ||
..$converters | ||
(..$putters) | ||
} | ||
override def invert(tup: $Tup): $T = { | ||
..$converters | ||
$companion(..$getters) | ||
} | ||
} | ||
""") | ||
} | ||
} | ||
//TODO test serialization of them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is weird. We can't ever use these right? Should we mark them not implicit?
I think we should just not stack them. Have something like:
then you
import MacroImplicits.Recursive._
or something.The current stacking is confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use the recursive ones in the lower priority, the maps is what I was pinging you about the other day. That doesn't make sense. I think we can hit both for the case classes since only one of them will match. (Aborting in the macro just removes it from the search so we should still be able to search for the recursive one).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the comment about us using abort in the macro to stop the implicit search. That will be tricky if you just look at the types.
Can we remove the map one since it does not make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done