Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

awakens default getter synthesis from the untyper nightmare

Our happy little macro paradise is regularly invaded by resetAllAttrs,
the bane of all macros and typers. It’s so ruthless and devastating that
we’ve been long scheming to hack something really cool and to one day
defeat it.

Today we make the first step towards the happy future. Today we overthrow
the UnTyper, resetAllAttrs’s elder brother that rules in the kingdoms of
GetterLand and CaseClassia, and banish him from the land of getters.
In the name of what’s good and meta, let’s band together and completely
drive him away in a subsequent pull request!

To put it in a nutshell, instead of using untyper on a DefDef to do
default getter synthesis, the commit duplicates the DefDef and does
resetLocalAttrs on it.

As default getter synthesis proceeds with figuring out type and value
parameters for the getter, then its tpt and finally its rhs, the commit
destructures the duplicated DefDef and then assembles the default getter
from the destructured parts instead of doing copyUntyped/copyUntypedInvariant
on the original DefDef.

I would say the test coverage is pretty good, as I had to figure out
3 or 4 test failures before I got to the stage when everything worked.
Iirc it tests pretty exotic stuff like polymorphic default parameters
in both second and third parameter lists, so it looks like we're pretty
good in this department.
  • Loading branch information...
commit 59cdd50fa8d2466b9778dca4b354afc03f17be67 1 parent 9f0594c
Eugene Burmako xeno-by authored
82 src/compiler/scala/tools/nsc/typechecker/Namers.scala
View
@@ -1132,7 +1132,7 @@ trait Namers extends MethodSynthesis {
}
}
- addDefaultGetters(meth, vparamss, tparams, overriddenSymbol(methResTp))
+ addDefaultGetters(meth, ddef, vparamss, tparams, overriddenSymbol(methResTp))
// fast track macros, i.e. macros defined inside the compiler, are hardcoded
// hence we make use of that and let them have whatever right-hand side they need
@@ -1174,7 +1174,12 @@ trait Namers extends MethodSynthesis {
* typechecked, the corresponding param would not yet have the "defaultparam"
* flag.
*/
- private def addDefaultGetters(meth: Symbol, vparamss: List[List[ValDef]], tparams: List[TypeDef], overriddenSymbol: => Symbol) {
+ private def addDefaultGetters(meth: Symbol, ddef: DefDef, vparamss: List[List[ValDef]], tparams: List[TypeDef], overriddenSymbol: => Symbol) {
+ val DefDef(_, _, rtparams0, rvparamss0, _, _) = resetLocalAttrs(ddef.duplicate)
+ // having defs here is important to make sure that there's no sneaky tree sharing
+ // in methods with multiple default parameters
+ def rtparams = rtparams0.map(_.duplicate)
+ def rvparamss = rvparamss0.map(_.map(_.duplicate))
val methOwner = meth.owner
val isConstr = meth.isConstructor
val overridden = if (isConstr || !methOwner.isClass) NoSymbol else overriddenSymbol
@@ -1206,23 +1211,36 @@ trait Namers extends MethodSynthesis {
//
vparamss.foldLeft(Nil: List[List[ValDef]]) { (previous, vparams) =>
assert(!overrides || vparams.length == baseParamss.head.length, ""+ meth.fullName + ", "+ overridden.fullName)
+ val rvparams = rvparamss(previous.length)
var baseParams = if (overrides) baseParamss.head else Nil
- for (vparam <- vparams) {
+ map2(vparams, rvparams)((vparam, rvparam) => {
val sym = vparam.symbol
// true if the corresponding parameter of the base class has a default argument
val baseHasDefault = overrides && baseParams.head.hasDefault
if (sym.hasDefault) {
- // generate a default getter for that argument
+ // Create a "default getter", i.e. a DefDef that will calculate vparam.rhs
+ // for those who are going to call meth without providing an argument corresponding to vparam.
+ // After the getter is created, a corresponding synthetic symbol is created and entered into the parent namer.
+ //
+ // In the ideal world, this DefDef would be a simple one-liner that just returns vparam.rhs,
+ // but in scalac things are complicated in two different ways.
+ //
+ // 1) Because the underlying language is quite sophisticated, we must allow for those sophistications in our getter.
+ // Namely: a) our getter has to copy type parameters from the associated method (or the associated class
+ // if meth is a constructor), because vparam.rhs might refer to one of them, b) our getter has to copy
+ // preceding value parameter lists from the associated method, because again vparam.rhs might refer to one of them.
+ //
+ // 2) Because we have already assigned symbols to type and value parameters that we have to copy, we must jump through
+ // hoops in order to destroy them and allow subsequent naming create new symbols for our getter. Previously this
+ // was done in an overly brutal way akin to resetAllAttrs, but now we utilize a resetLocalAttrs-based approach.
+ // Still far from ideal, but at least enables things like run/macro-default-params that were previously impossible.
+
val oflag = if (baseHasDefault) OVERRIDE else 0
val name = nme.defaultGetterName(meth.name, posCounter)
- // Create trees for the defaultGetter. Uses tools from Unapplies.scala
- var deftParams = tparams map copyUntyped[TypeDef]
- val defvParamss = mmap(previous) { p =>
- // in the default getter, remove the default parameter
- val p1 = atPos(p.pos.focus) { ValDef(p.mods &~ DEFAULTPARAM, p.name, p.tpt.duplicate, EmptyTree) }
- UnTyper.traverse(p1)
- p1
+ var defTparams = rtparams
+ val defVparamss = mmap(rvparamss.take(previous.length)){ rvp =>
+ copyValDef(rvp)(mods = rvp.mods &~ DEFAULTPARAM, rhs = EmptyTree)
}
val parentNamer = if (isConstr) {
@@ -1244,7 +1262,8 @@ trait Namers extends MethodSynthesis {
return // fix #3649 (prevent crash in erroneous source code)
}
}
- deftParams = cdef.tparams map copyUntypedInvariant
+ val ClassDef(_, _, rtparams, _) = resetLocalAttrs(cdef.duplicate)
+ defTparams = rtparams.map(rt => copyTypeDef(rt)(mods = rt.mods &~ (COVARIANT | CONTRAVARIANT)))
nmr
}
else ownerNamer getOrElse {
@@ -1255,23 +1274,30 @@ trait Namers extends MethodSynthesis {
nmr
}
- // If the parameter type mentions any type parameter of the method, let the compiler infer the
- // return type of the default getter => allow "def foo[T](x: T = 1)" to compile.
- // This is better than always using Wildcard for inferring the result type, for example in
- // def f(i: Int, m: Int => Int = identity _) = m(i)
- // if we use Wildcard as expected, we get "Nothing => Nothing", and the default is not usable.
- val names = deftParams map { case TypeDef(_, name, _, _) => name }
- val subst = new TypeTreeSubstituter(names contains _)
-
- val defTpt = subst(copyUntyped(vparam.tpt match {
- // default getter for by-name params
- case AppliedTypeTree(_, List(arg)) if sym.hasFlag(BYNAMEPARAM) => arg
- case t => t
- }))
- val defRhs = copyUntyped(vparam.rhs)
+ val defTpt =
+ // don't mess with tpt's of case copy default getters, because assigning something other than TypeTree()
+ // will break the carefully orchestrated naming/typing logic that involves enterCopyMethod and caseClassCopyMeth
+ if (meth.isCaseCopy) TypeTree()
+ else {
+ // If the parameter type mentions any type parameter of the method, let the compiler infer the
+ // return type of the default getter => allow "def foo[T](x: T = 1)" to compile.
+ // This is better than always using Wildcard for inferring the result type, for example in
+ // def f(i: Int, m: Int => Int = identity _) = m(i)
+ // if we use Wildcard as expected, we get "Nothing => Nothing", and the default is not usable.
+ // TODO: this is a very brittle approach; I sincerely hope that Denys's research into hygiene
+ // will open the doors to a much better way of doing this kind of stuff
+ val tparamNames = defTparams map { case TypeDef(_, name, _, _) => name }
+ val eraseAllMentionsOfTparams = new TypeTreeSubstituter(tparamNames contains _)
+ eraseAllMentionsOfTparams(rvparam.tpt match {
+ // default getter for by-name params
+ case AppliedTypeTree(_, List(arg)) if sym.hasFlag(BYNAMEPARAM) => arg
+ case t => t
+ })
+ }
+ val defRhs = rvparam.rhs
val defaultTree = atPos(vparam.pos.focus) {
- DefDef(Modifiers(paramFlagsToDefaultGetter(meth.flags)) | oflag, name, deftParams, defvParamss, defTpt, defRhs)
+ DefDef(Modifiers(paramFlagsToDefaultGetter(meth.flags)) | oflag, name, defTparams, defVparamss, defTpt, defRhs)
}
if (!isConstr)
methOwner.resetFlag(INTERFACE) // there's a concrete member now
@@ -1286,7 +1312,7 @@ trait Namers extends MethodSynthesis {
}
posCounter += 1
if (overrides) baseParams = baseParams.tail
- }
+ })
if (overrides) baseParamss = baseParamss.tail
previous :+ vparams
}
10 src/compiler/scala/tools/nsc/typechecker/Typers.scala
View
@@ -56,16 +56,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
resetDocComments()
}
- object UnTyper extends Traverser {
- override def traverse(tree: Tree) = {
- if (tree.canHaveAttrs) {
- tree.clearType()
- if (tree.hasSymbolField) tree.symbol = NoSymbol
- }
- super.traverse(tree)
- }
- }
-
sealed abstract class SilentResult[+T] {
@inline final def fold[U](none: => U)(f: T => U): U = this match {
case SilentResultValue(value) => f(value)
29 src/compiler/scala/tools/nsc/typechecker/Unapplies.scala
View
@@ -43,10 +43,35 @@ trait Unapplies extends ast.TreeDSL {
def unapply(tp: Type): Option[Symbol] = unapplyMember(tp).toOption
}
- def copyUntyped[T <: Tree](tree: T): T =
+ // NOTE: Do not unprivate this method! Even if you think you need it, you most likely don't need it.
+ //
+ // resetAllAttrs, copyUntyped and likes irreparably break certain important tree shapes (see SI-5464 for details),
+ // including a very common case of TypeTree's without originals, which means that every call to resetAllAttrs
+ // breaks code that emits such trees.
+ //
+ // However there are situations when we want to reuse already attributed trees somewhere else
+ // (e.g. for case class synthesis, for auxiliary codegen that desugars default parameters, etc), and then the only way to go
+ // in the current architecture is to erase attributes. That's a sad lose-lose situation.
+ //
+ // But not all hope is lost! First of all, resetLocalAttrs is oftentimes good enough (e.g. see addDefaultGetters),
+ // and since it's much less destructive that resetAllAttrs, the breakages that it causes are going to manifest themselves
+ // much less frequently. Secondly, research into hygiene promises to give us a better way of doing bindings, so let's cross our fingers.
+ private def copyUntyped[T <: Tree](tree: T): T = {
+ object UnTyper extends Traverser {
+ override def traverse(tree: Tree) = {
+ if (tree.canHaveAttrs) {
+ tree.clearType()
+ if (tree.hasSymbolField) tree.symbol = NoSymbol
+ }
+ super.traverse(tree)
+ }
+ }
returning[T](tree.duplicate)(UnTyper traverse _)
+ }
- def copyUntypedInvariant(td: TypeDef): TypeDef =
+ // NOTE: do not unprivate this method
+ // see comments for copyUntyped for more information
+ private def copyUntypedInvariant(td: TypeDef): TypeDef =
copyTypeDef(copyUntyped(td))(mods = td.mods &~ (COVARIANT | CONTRAVARIANT))
private def toIdent(x: DefTree) = Ident(x.name) setPos x.pos.focus
5 src/reflect/scala/reflect/internal/Symbols.scala
View
@@ -801,9 +801,14 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
isConstructor && !isPrimaryConstructor
/** Is this symbol a synthetic apply or unapply method in a companion object of a case class? */
+ // xeno-by: why this obscure use of the CASE flag? why not simply compare name with nme.apply and nme.unapply?
final def isCaseApplyOrUnapply =
isMethod && isCase && isSynthetic
+ /** Is this symbol a synthetic copy method in a case class? */
+ final def isCaseCopy =
+ isMethod && owner.isCase && isSynthetic && name == nme.copy
+
/** Is this symbol a trait which needs an implementation class? */
final def needsImplClass = (
isTrait
1  test/files/run/macro-default-params.check
View
@@ -0,0 +1 @@
+0
27 test/files/run/macro-default-params/Macros_1.scala
View
@@ -0,0 +1,27 @@
+import scala.language.experimental.macros
+import scala.reflect.macros.WhiteboxContext
+
+object Macros {
+ def id[A]: A = null.asInstanceOf[A]
+
+ def foo: Any = macro impl
+ def impl(c: WhiteboxContext): c.Tree = {
+ import c.universe._
+ import Flag._
+
+ lazy val tpe = TypeTree(typeOf[Int])
+
+ /* If we used this line instead, it would work! */
+ // lazy val tpe = tq"Int"
+
+ lazy val param: ValDef = {
+ val p1 = q"val a: ${tpe.duplicate} = Macros.id[${tpe.duplicate}]"
+ ValDef(Modifiers(DEFAULTPARAM), p1.name, p1.tpt, p1.rhs)
+ }
+
+ q"""
+ class C { def f($param) = a }
+ println(new C().f())
+ """
+ }
+}
3  test/files/run/macro-default-params/Test_2.scala
View
@@ -0,0 +1,3 @@
+object Test extends App {
+ Macros.foo
+}
Please sign in to comment.
Something went wrong with that request. Please try again.