From d7d63e93f35c692b26e95db1b02d758723dde18d Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 10 Nov 2013 13:40:26 +0100 Subject: Tidy up the Uncurry component of delambdafy - Use tree factories that accept symbols and encapsulate ValDef creation - Use `gen.mkForwarder` to handle the conditional addition of `: _*` for varargs functions. We don't need to predicate this on `etaExpandKeepsStar`; the only place that need to do that is EtaExpansion. --- .../scala/tools/nsc/transform/UnCurry.scala | 62 +++++++--------------- src/reflect/scala/reflect/internal/Trees.scala | 6 +++ 2 files changed, 26 insertions(+), 42 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index 3d648ccbac..d1d4759ea5 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -217,14 +217,14 @@ abstract class UnCurry extends InfoTransform // nullary or parameterless case fun1 if fun1 ne fun => fun1 case _ => - val parents = addSerializable(abstractFunctionForFunctionType(fun.tpe)) - val anonClass = fun.symbol.owner newAnonymousFunctionClass(fun.pos, inConstructorFlag) addAnnotation SerialVersionUIDAnnotation - anonClass setInfo ClassInfoType(parents, newScope, anonClass) + val formals :+ restpe = fun.tpe.typeArgs - val targs = fun.tpe.typeArgs - val (formals, restpe) = (targs.init, targs.last) + def typedFunPos(t: Tree) = localTyper.typedPos(fun.pos)(t) if (inlineFunctionExpansion) { + val parents = addSerializable(abstractFunctionForFunctionType(fun.tpe)) + val anonClass = fun.symbol.owner newAnonymousFunctionClass(fun.pos, inConstructorFlag) addAnnotation SerialVersionUIDAnnotation + anonClass setInfo ClassInfoType(parents, newScope, anonClass) val applyMethodDef = { val methSym = anonClass.newMethod(nme.apply, fun.pos, FINAL) val paramSyms = map2(formals, fun.vparams) { @@ -235,7 +235,7 @@ abstract class UnCurry extends InfoTransform fun.vparams foreach (_.symbol.owner = methSym) fun.body changeOwner (fun.symbol -> methSym) - val body = localTyper.typedPos(fun.pos)(fun.body) + val body = typedFunPos(fun.body) val methDef = DefDef(methSym, List(fun.vparams), body) // Have to repack the type to avoid mismatches when existentials @@ -244,7 +244,7 @@ abstract class UnCurry extends InfoTransform methDef } - localTyper.typedPos(fun.pos) { + typedFunPos { Block( List(ClassDef(anonClass, NoMods, ListOfNil, List(applyMethodDef), fun.pos)), Typed(New(anonClass.tpe), TypeTree(fun.tpe))) @@ -270,23 +270,17 @@ abstract class UnCurry extends InfoTransform * @bodyF function that turns the method symbol and list of value params * into a body for the method */ - def createMethod(owner: Symbol, name: TermName, additionalFlags: Long)(bodyF: (Symbol, List[ValDef]) => Tree) = { + def createMethod(owner: Symbol, name: TermName, additionalFlags: Long)(bodyF: Symbol => Tree) = { val methSym = owner.newMethod(name, fun.pos, FINAL | additionalFlags) - val vparams = fun.vparams map (_.duplicate) - val paramSyms = map2(formals, vparams) { + val paramSyms = map2(formals, fun.vparams) { (tp, vparam) => methSym.newSyntheticValueParam(tp, vparam.name) } - foreach2(vparams, paramSyms){(valdef, sym) => valdef.symbol = sym} - vparams foreach (_.symbol.owner = methSym) - val methodType = MethodType(paramSyms, restpe.deconst) - methSym setInfo methodType + methSym setInfo MethodType(paramSyms, restpe.deconst) - // TODO this is probably cleaner if bodyF only works with symbols rather than parameter ValDefs - val tempBody = bodyF(methSym, vparams) - val body = localTyper.typedPos(fun.pos)(tempBody) - val methDef = DefDef(methSym, List(vparams), body) + val body = typedFunPos(bodyF(methSym)) + val methDef = DefDef(methSym, body) // Have to repack the type to avoid mismatches when existentials // appear in the result - see SI-4869. @@ -294,35 +288,19 @@ abstract class UnCurry extends InfoTransform methDef } - val methodFlags = ARTIFACT + val funParams = fun.vparams map (_.symbol) // method definition with the same arguments, return type, and body as the original lambda - val liftedMethod = createMethod(fun.symbol.owner, tpnme.ANON_FUN_NAME.toTermName, methodFlags){ - case(methSym, vparams) => - fun.body.substituteSymbols(fun.vparams map (_.symbol), vparams map (_.symbol)) + val liftedMethod = createMethod(fun.symbol.owner, nme.ANON_FUN_NAME, additionalFlags = ARTIFACT) { + methSym => + fun.body.substituteSymbols(funParams, methSym.paramss.head) fun.body changeOwner (fun.symbol -> methSym) } - // callsite for the lifted method - val args = fun.vparams map { vparam => - val ident = Ident(vparam.symbol) - // if -Yeta-expand-keeps-star is turned on then T* types can get through. In order - // to forward them we need to forward x: T* ascribed as "x:_*" - if (settings.etaExpandKeepsStar && definitions.isRepeatedParamType(vparam.tpt.tpe)) - gen.wildcardStar(ident) - else - ident - } - - val funTyper = localTyper.typedPos(fun.pos) _ - - val liftedMethodCall = funTyper(Apply(liftedMethod.symbol, args:_*)) - // new function whose body is just a call to the lifted method - val newFun = treeCopy.Function(fun, fun.vparams, liftedMethodCall) - funTyper(Block( - List(funTyper(liftedMethod)), - super.transform(newFun) - )) + val newFun = deriveFunction(fun)(_ => typedFunPos( + gen.mkForwarder(gen.mkAttributedRef(liftedMethod.symbol), funParams :: Nil) + )) + typedFunPos(Block(liftedMethod, super.transform(newFun))) } } } diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala index af0af8afe8..b0abc0be7b 100644 --- a/src/reflect/scala/reflect/internal/Trees.scala +++ b/src/reflect/scala/reflect/internal/Trees.scala @@ -1804,6 +1804,12 @@ trait Trees extends api.Trees { case t => sys.error("Not a LabelDef: " + t + "/" + t.getClass) } + def deriveFunction(func: Tree)(applyToRhs: Tree => Tree): Function = func match { + case Function(params0, rhs0) => + treeCopy.Function(func, params0, applyToRhs(rhs0)) + case t => + sys.error("Not a Function: " + t + "/" + t.getClass) + } // -------------- Classtags -------------------------------------------------------- -- cgit v1.2.3 From cb37548ef85d471951867b9f8a97cb9b9820fc66 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 23 Nov 2013 09:41:07 +0100 Subject: Symbol substutition must consider ClassInfoType#parents An upcoming change to uncurry, in which I plan to make substitution of `lambda params -> apply method params` requires that I first to fix a problem in symbol substition. This situation can arise when the body of this definition: def owner1(a: A) = { class C extends M[a.B] } is transplanted into a new owner that has a different symbol for `a`. I speculated that value classes might also be prone to the fact that symbol substitution neglected `ClassInfoType#parents`. We can test change with Value Classes: Partial Functions that are dependent on value class param type used to fail with a spurious overriding error, now they work correctly. --- .../scala/reflect/internal/tpe/TypeMaps.scala | 6 ++++++ .../run/value-class-partial-func-depmet.scala | 24 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 test/files/run/value-class-partial-func-depmet.scala diff --git a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala index 9a54ad8217..f5aa048e6a 100644 --- a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala +++ b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala @@ -717,6 +717,12 @@ private[internal] trait TypeMaps { else appliedType(tcon.typeConstructor, args) case SingleType(NoPrefix, sym) => substFor(sym) + case ClassInfoType(parents, decls, sym) => + val parents1 = parents mapConserve this + // We don't touch decls here; they will be touched when an enclosing TreeSubstitutor + // transforms the tree that defines them. + if (parents1 eq parents) tp + else ClassInfoType(parents1, decls, sym) case _ => tp } diff --git a/test/files/run/value-class-partial-func-depmet.scala b/test/files/run/value-class-partial-func-depmet.scala new file mode 100644 index 0000000000..12ff64ed36 --- /dev/null +++ b/test/files/run/value-class-partial-func-depmet.scala @@ -0,0 +1,24 @@ +class C +class A { class C } + +object Test { + def main(args: Array[String]) { + val a = new A + + new VC("").foo(a) + } +} + +class VC(val a: Any) extends AnyVal { + def foo(a: A) = { + val pf: PartialFunction[a.C, Any] = { case x => x } + (pf: PartialFunction[Null, Any]).isDefinedAt(null) + } +} + +// 2.11.0-M6 +// test/files/run/value-class-partial-func-depmet.scala:14: error: overriding method applyOrElse in trait PartialFunction of type [A1 <: a.C, B1 >: Any](x: A1, default: A1 => B1)B1; +// method applyOrElse has incompatible type +// val pf: PartialFunction[a.C, Any] = { case x => x } +// ^ +// one error found -- cgit v1.2.3 From 736613ea8a2fb3eede9bc01346a743c70b44eeaa Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 23 Nov 2013 09:45:19 +0100 Subject: Substitute new parameter symbols into lambda body Previously, new synthetic parameter symbols were created for the apply method parameters, but only used in its `MethodType`. The `ValDef` trees of the function, along with their symbols, were used directly as the parameter trees of the apply method. % cat sandbox/test.scala object Test { (x: Int) => { (y: Int) => ??? } } % scalac-hash v2.10.3 -Xprint:typer,uncurry -uniqid ../scala2/sandbox/test.scala | egrep 'syntax|\bx' [info] v2.10.3 => /Users/jason/usr/scala-v2.10.3-0-g88b5d19 [[syntax trees at end of typer]] // test.scala ((x#12152: Int#351) => x#12152)#12151 [[syntax trees at end of uncurry]] // test.scala final def apply#15952(x#12152: Int#351): Int#351 = x#12152 This approach dates back a long way: c64152bc3. @odersky tells me it was most likely due to insufficent substitution/cloning machinery at the time. % qbin/scalac -Xprint:typer,uncurry -uniqid ../scala2/sandbox/test.scala | egrep 'syntax|\bx' [[syntax trees at end of typer]] // test.scala ((x#13685: Int#1760) => x#13685)#13671 [[syntax trees at end of uncurry]] // test.scala final def apply#13959(x#13960: Int#1760): Int#1760 = x#13960 To make this work, I had to fix a problem in symbol substition; this was commited in the previous commit. In the enclosed test, at Uncurry, the symbol of the nested class `N` had a `ClassInfoType`, which listed as a parent `M[a.C]`. When we substituted the new method parameter symbol `a` for the eponymous function parameter symbol, this was not touched. --- src/compiler/scala/tools/nsc/transform/UnCurry.scala | 6 +++--- .../run/delambdafy-dependent-on-param-subst-2.scala | 20 ++++++++++++++++++++ .../run/delambdafy-dependent-on-param-subst.flags | 1 + .../run/delambdafy-dependent-on-param-subst.scala | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 test/files/run/delambdafy-dependent-on-param-subst-2.scala create mode 100644 test/files/run/delambdafy-dependent-on-param-subst.flags create mode 100644 test/files/run/delambdafy-dependent-on-param-subst.scala diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index d1d4759ea5..a851585442 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -232,11 +232,11 @@ abstract class UnCurry extends InfoTransform } methSym setInfoAndEnter MethodType(paramSyms, restpe) - fun.vparams foreach (_.symbol.owner = methSym) - fun.body changeOwner (fun.symbol -> methSym) + fun.body changeOwner (fun.symbol -> methSym) + fun.body substituteSymbols (fun.vparams.map(_.symbol), paramSyms) val body = typedFunPos(fun.body) - val methDef = DefDef(methSym, List(fun.vparams), body) + val methDef = DefDef(methSym, body) // Have to repack the type to avoid mismatches when existentials // appear in the result - see SI-4869. diff --git a/test/files/run/delambdafy-dependent-on-param-subst-2.scala b/test/files/run/delambdafy-dependent-on-param-subst-2.scala new file mode 100644 index 0000000000..7b6fc597e8 --- /dev/null +++ b/test/files/run/delambdafy-dependent-on-param-subst-2.scala @@ -0,0 +1,20 @@ +trait M[-X] { + def m(x: X): Boolean +} + +class C +class A { class C } + +object Test { + def main(args: Array[String]) { + val a = new A + + // class O extends M[a.C] { def m(x: a.C) = true } + // (new O: M[Null]).m(null) // Okay + + ((a: A) => { + class N extends M[a.C] { def m(x: a.C) = true } + new N: M[Null] + }).apply(a).m(null) // NPE, missing bridge + } +} diff --git a/test/files/run/delambdafy-dependent-on-param-subst.flags b/test/files/run/delambdafy-dependent-on-param-subst.flags new file mode 100644 index 0000000000..2b27e19830 --- /dev/null +++ b/test/files/run/delambdafy-dependent-on-param-subst.flags @@ -0,0 +1 @@ +-Ydelambdafy:method \ No newline at end of file diff --git a/test/files/run/delambdafy-dependent-on-param-subst.scala b/test/files/run/delambdafy-dependent-on-param-subst.scala new file mode 100644 index 0000000000..7b6fc597e8 --- /dev/null +++ b/test/files/run/delambdafy-dependent-on-param-subst.scala @@ -0,0 +1,20 @@ +trait M[-X] { + def m(x: X): Boolean +} + +class C +class A { class C } + +object Test { + def main(args: Array[String]) { + val a = new A + + // class O extends M[a.C] { def m(x: a.C) = true } + // (new O: M[Null]).m(null) // Okay + + ((a: A) => { + class N extends M[a.C] { def m(x: a.C) = true } + new N: M[Null] + }).apply(a).m(null) // NPE, missing bridge + } +} -- cgit v1.2.3 From b5be392967d84591d45153408960d0e5b68d82e9 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 10 Nov 2013 15:00:06 +0100 Subject: Refactor away duplication between -Ydelambdafy:{inline,method} --- src/compiler/scala/tools/nsc/ast/TreeGen.scala | 40 ++++++++++++ .../scala/tools/nsc/transform/UnCurry.scala | 71 +++------------------- 2 files changed, 47 insertions(+), 64 deletions(-) diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala index d4ac21a6b8..4ac6672727 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala @@ -256,4 +256,44 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL { val stats1 = if (stats.isEmpty) List(Literal(Constant(()))) else stats mkNew(Nil, noSelfType, stats1, NoPosition, NoPosition) } + + /** + * Create a method based on a Function + * + * Used both to under `-Ydelambdafy:method` create a lifted function and + * under `-Ydelamdafy:inline` to create the apply method on the anonymous + * class. + * + * It creates a method definition with value params cloned from the + * original lambda. Then it calls a supplied function to create + * the body and types the result. Finally + * everything is wrapped up in a DefDef + * + * @param owner The owner for the new method + * @param name name for the new method + * @param additionalFlags flags to be put on the method in addition to FINAL + */ + def mkMethodFromFunction(localTyper: analyzer.Typer) + (fun: Function, owner: Symbol, name: TermName, additionalFlags: FlagSet = NoFlags) = { + val funParams = fun.vparams map (_.symbol) + val formals :+ restpe = fun.tpe.typeArgs + + val methSym = owner.newMethod(name, fun.pos, FINAL | additionalFlags) + + val paramSyms = map2(formals, fun.vparams) { + (tp, vparam) => methSym.newSyntheticValueParam(tp, vparam.name) + } + + methSym setInfo MethodType(paramSyms, restpe.deconst) + + fun.body.substituteSymbols(funParams, paramSyms) + fun.body changeOwner (fun.symbol -> methSym) + + val methDef = DefDef(methSym, fun.body) + + // Have to repack the type to avoid mismatches when existentials + // appear in the result - see SI-4869. + methDef.tpt setType localTyper.packedType(fun.body, methSym).deconst + methDef + } } diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index a851585442..844774e75f 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -217,84 +217,27 @@ abstract class UnCurry extends InfoTransform // nullary or parameterless case fun1 if fun1 ne fun => fun1 case _ => - val formals :+ restpe = fun.tpe.typeArgs - def typedFunPos(t: Tree) = localTyper.typedPos(fun.pos)(t) + val funParams = fun.vparams map (_.symbol) + def mkMethod(owner: Symbol, name: TermName, additionalFlags: FlagSet = NoFlags): DefDef = + gen.mkMethodFromFunction(localTyper)(fun, owner, name, additionalFlags) if (inlineFunctionExpansion) { val parents = addSerializable(abstractFunctionForFunctionType(fun.tpe)) val anonClass = fun.symbol.owner newAnonymousFunctionClass(fun.pos, inConstructorFlag) addAnnotation SerialVersionUIDAnnotation anonClass setInfo ClassInfoType(parents, newScope, anonClass) - val applyMethodDef = { - val methSym = anonClass.newMethod(nme.apply, fun.pos, FINAL) - val paramSyms = map2(formals, fun.vparams) { - (tp, param) => methSym.newSyntheticValueParam(tp, param.name) - } - methSym setInfoAndEnter MethodType(paramSyms, restpe) - - fun.body changeOwner (fun.symbol -> methSym) - fun.body substituteSymbols (fun.vparams.map(_.symbol), paramSyms) - val body = typedFunPos(fun.body) - val methDef = DefDef(methSym, body) - - // Have to repack the type to avoid mismatches when existentials - // appear in the result - see SI-4869. - methDef.tpt setType localTyper.packedType(body, methSym) - methDef - } + val applyMethodDef = mkMethod(anonClass, nme.apply) + anonClass.info.decls enter applyMethodDef.symbol typedFunPos { Block( - List(ClassDef(anonClass, NoMods, ListOfNil, List(applyMethodDef), fun.pos)), + ClassDef(anonClass, NoMods, ListOfNil, List(applyMethodDef), fun.pos), Typed(New(anonClass.tpe), TypeTree(fun.tpe))) } } else { - /** - * Abstracts away the common functionality required to create both - * the lifted function and the apply method on the anonymous class - * It creates a method definition with value params cloned from the - * original lambda. Then it calls a supplied function to create - * the body and types the result. Finally - * everything is wrapped up in a MethodDef - * - * TODO it is intended that this common functionality be used - * whether inlineFunctionExpansion is true or not. However, it - * seems to introduce subtle ownwership changes that produce - * binary incompatible changes and so it is completely - * hidden behind the inlineFunctionExpansion for now. - * - * @param owner The owner for the new method - * @param name name for the new method - * @param additionalFlags flags to be put on the method in addition to FINAL - * @bodyF function that turns the method symbol and list of value params - * into a body for the method - */ - def createMethod(owner: Symbol, name: TermName, additionalFlags: Long)(bodyF: Symbol => Tree) = { - val methSym = owner.newMethod(name, fun.pos, FINAL | additionalFlags) - - val paramSyms = map2(formals, fun.vparams) { - (tp, vparam) => methSym.newSyntheticValueParam(tp, vparam.name) - } - - methSym setInfo MethodType(paramSyms, restpe.deconst) - - val body = typedFunPos(bodyF(methSym)) - val methDef = DefDef(methSym, body) - - // Have to repack the type to avoid mismatches when existentials - // appear in the result - see SI-4869. - methDef.tpt setType localTyper.packedType(body, methSym).deconst - methDef - } - - val funParams = fun.vparams map (_.symbol) // method definition with the same arguments, return type, and body as the original lambda - val liftedMethod = createMethod(fun.symbol.owner, nme.ANON_FUN_NAME, additionalFlags = ARTIFACT) { - methSym => - fun.body.substituteSymbols(funParams, methSym.paramss.head) - fun.body changeOwner (fun.symbol -> methSym) - } + val liftedMethod = mkMethod(fun.symbol.owner, nme.ANON_FUN_NAME, additionalFlags = ARTIFACT) // new function whose body is just a call to the lifted method val newFun = deriveFunction(fun)(_ => typedFunPos( -- cgit v1.2.3 From 5d5596bb07c0b5985fe9a6ba5433a3d463918b28 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 20 Nov 2013 10:26:32 +0100 Subject: Special treatment for local symbols in TypeTreeMemberType Avoids calling `thisType` on the owner if it is a term symbol, which doesn't make much sense. This method is used internally in tree factory methods that create, e.g, a `DefDef` based on the info of a `Symbol`. --- src/reflect/scala/reflect/internal/Trees.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala index b0abc0be7b..d191fbd38f 100644 --- a/src/reflect/scala/reflect/internal/Trees.scala +++ b/src/reflect/scala/reflect/internal/Trees.scala @@ -590,7 +590,7 @@ trait Trees extends api.Trees { def TypeTree(tp: Type): TypeTree = TypeTree() setType tp private def TypeTreeMemberType(sym: Symbol): TypeTree = { // Needed for pos/t4970*.scala. See SI-7853 - val resType = (sym.owner.thisType memberType sym).finalResultType + val resType = (if (sym.isLocal) sym.tpe else (sym.owner.thisType memberType sym)).finalResultType atPos(sym.pos.focus)(TypeTree(resType)) } -- cgit v1.2.3