From 32dc7e80698c83947bf4b74f6eadd385a06f5b09 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 7 Jul 2012 13:18:44 +0200 Subject: A test case that scrutinises lambdalifter's output. No doubt there are plenty of additional variations that could be added to exercise more code paths. This is the unflattering "before" photo; the next commit will make over the name mangling and reveal the simple beauty of unmangled names. References SI-6028 --- test/files/run/t6028.check | 84 ++++++++++++++++++++++++++++++++++++++++++++++ test/files/run/t6028.scala | 22 ++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 test/files/run/t6028.check create mode 100644 test/files/run/t6028.scala (limited to 'test') diff --git a/test/files/run/t6028.check b/test/files/run/t6028.check new file mode 100644 index 0000000000..6c9930b503 --- /dev/null +++ b/test/files/run/t6028.check @@ -0,0 +1,84 @@ +[[syntax trees at end of lambdalift]] // newSource1 +package { + class T extends Object { + val T$$classParam: Int = _; + def (classParam: Int): T = { + T.super.(); + () + }; + private[this] val field: Int = 0; + def field(): Int = T.this.field; + def foo(methodParam$1: Int): Function0 = { + val methodLocal$1: Int = 0; + { + (new anonymous class $anonfun$foo$1(T.this, methodParam$1, methodLocal$1): Function0) + } + }; + def bar(barParam$1: Int): Object = { + @volatile var MethodLocalObject$module$1: scala.runtime.VolatileObjectRef = new scala.runtime.VolatileObjectRef(); + T.this.MethodLocalObject$1(barParam$1, MethodLocalObject$module$1) + }; + def tryy(tryyParam$1: Int): Function0 = { + var tryyLocal$1: scala.runtime.IntRef = new scala.runtime.IntRef(0); + { + (new anonymous class $anonfun$tryy$1(T.this, tryyParam$1, tryyLocal$1): Function0) + } + }; + @SerialVersionUID(0) final class $anonfun$foo$1 extends scala.runtime.AbstractFunction0$mcI$sp with Serializable { + def ($outer: T, methodParam$1: Int, methodLocal$1: Int): anonymous class $anonfun$foo$1 = { + $anonfun$foo$1.super.(); + () + }; + final def apply(): Int = $anonfun$foo$1.this.apply$mcI$sp(); + def apply$mcI$sp(): Int = $anonfun$foo$1.this.$outer.T$$classParam.+($anonfun$foo$1.this.$outer.field()).+($anonfun$foo$1.this.methodParam$1).+($anonfun$foo$1.this.methodLocal$1); + private[this] val $outer: T = _; + def T$$anonfun$$$outer(): T = $anonfun$foo$1.this.$outer; + final def apply(): Object = scala.Int.box($anonfun$foo$1.this.apply()); + private[this] val methodParam$1: Int = _; + private[this] val methodLocal$1: Int = _ + }; + abstract trait MethodLocalTrait$1 extends Object { + def T$MethodLocalTrait$$$outer(): T + }; + object MethodLocalObject$2 extends Object with T#MethodLocalTrait$1 { + def ($outer: T, barParam$1: Int): ... = { + MethodLocalObject$2.super.(); + MethodLocalObject$2.this.$asInstanceOf[T#MethodLocalTrait$1$class]()./*MethodLocalTrait$1$class*/$init$(barParam$1); + () + }; + private[this] val $outer: T = _; + def T$MethodLocalObject$$$outer(): T = MethodLocalObject$2.this.$outer; + def T$MethodLocalTrait$$$outer(): T = MethodLocalObject$2.this.$outer + }; + final private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: scala.runtime.VolatileObjectRef): ... = { + MethodLocalObject$module$1.elem = new ...(T.this, barParam$1); + MethodLocalObject$module$1.elem.$asInstanceOf[...]() + }; + abstract trait MethodLocalTrait$1$class extends Object with T#MethodLocalTrait$1 { + def /*MethodLocalTrait$1$class*/$init$(barParam$1: Int): Unit = { + () + }; + scala.this.Predef.print(scala.Int.box(barParam$1)) + }; + @SerialVersionUID(0) final class $anonfun$tryy$1 extends scala.runtime.AbstractFunction0$mcV$sp with Serializable { + def ($outer: T, tryyParam$1: Int, tryyLocal$1: scala.runtime.IntRef): anonymous class $anonfun$tryy$1 = { + $anonfun$tryy$1.super.(); + () + }; + final def apply(): Unit = $anonfun$tryy$1.this.apply$mcV$sp(); + def apply$mcV$sp(): Unit = try { + $anonfun$tryy$1.this.tryyLocal$1.elem = $anonfun$tryy$1.this.tryyParam$1 + } finally (); + private[this] val $outer: T = _; + def T$$anonfun$$$outer(): T = $anonfun$tryy$1.this.$outer; + final def apply(): Object = { + $anonfun$tryy$1.this.apply(); + scala.runtime.BoxedUnit.UNIT + }; + private[this] val tryyParam$1: Int = _; + private[this] val tryyLocal$1: scala.runtime.IntRef = _ + } + } +} + +warning: there were 1 feature warnings; re-run with -feature for details diff --git a/test/files/run/t6028.scala b/test/files/run/t6028.scala new file mode 100644 index 0000000000..7611aee0fc --- /dev/null +++ b/test/files/run/t6028.scala @@ -0,0 +1,22 @@ +import scala.tools.partest._ +import java.io.{Console => _, _} + +object Test extends DirectTest { + + override def extraSettings: String = "-usejavacp -Xprint:lambdalift -d " + testOutput.path + + override def code = """class T(classParam: Int) { + | val field: Int = 0 + | def foo(methodParam: Int) = {val methodLocal = 0 ; () => classParam + field + methodParam + methodLocal } + | def bar(barParam: Int) = { trait MethodLocalTrait { print(barParam) }; object MethodLocalObject extends MethodLocalTrait; MethodLocalObject } + | def tryy(tryyParam: Int) = { var tryyLocal = 0; () => try { tryyLocal = tryyParam } finally () } + |} + |""".stripMargin.trim + + override def show(): Unit = { + // redirect err to out, for logging + Console.withErr(System.out) { + compile() + } + } +} -- cgit v1.2.3 From a3bf34563d718f19ad02ff9ac5a2a1cec865aa24 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 7 Jul 2012 09:24:50 +0200 Subject: SI-6028 Avoid needless symbol renaming in lambdalift. Preserve names of all referenced free vars. Only the proxy symbols have the fresh names. The resulting natural beauty is evident in the diff of t6028.check. This subsumes the treatment in 0e170e4b that ensured named parameter calls cannot see mangled names; pos/t6028 confirms as much. --- .../scala/tools/nsc/transform/LambdaLift.scala | 85 ++++++++++++++-------- test/files/pos/t6028/t6028_1.scala | 3 + test/files/pos/t6028/t6028_2.scala | 4 + test/files/run/t6028.check | 18 ++--- test/files/run/t6028.scala | 1 - 5 files changed, 71 insertions(+), 40 deletions(-) create mode 100644 test/files/pos/t6028/t6028_1.scala create mode 100644 test/files/pos/t6028/t6028_2.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index fad41ae98d..bee5aa5f4f 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -56,6 +56,31 @@ abstract class LambdaLift extends InfoTransform { /** The set of symbols that need to be renamed. */ private val renamable = newSymSet + /** + * The new names for free variables proxies. If we simply renamed the + * free variables, we would transform: + * {{{ + * def closure(x: Int) = { () => x } + * }}} + * + * To: + * {{{ + * def closure(x$1: Int) = new anonFun$1(this, x$1) + * class anonFun$1(outer$: Outer, x$1: Int) { def apply() => x$1 } + * }}} + * + * This is fatally bad for named arguments (0e170e4b), extremely impolite to tools + * reflecting on the method parameter names in the generated bytecode (SI-6028), + * and needlessly bothersome to anyone using a debugger. + * + * Instead, we transform to: + * {{{ + * def closure(x: Int) = new anonFun$1(this, x) + * class anonFun$1(outer$: Outer, x$1: Int) { def apply() => x$1 } + * }}} + */ + private val proxyNames = mutable.HashMap[Symbol, Name]() + // (trait, name) -> owner private val localTraits = mutable.HashMap[(Symbol, Name), Symbol]() // (owner, name) -> implClass @@ -117,15 +142,6 @@ abstract class LambdaLift extends InfoTransform { if (!ss(sym)) { ss addEntry sym renamable addEntry sym - beforePickler { - // The param symbol in the MethodType should not be renamed, only the symbol in scope. This way, - // parameter names for named arguments are not changed. Example: without cloning the MethodType, - // def closure(x: Int) = { () => x } - // would have the signature - // closure: (x$1: Int)() => Int - if (sym.isParameter && sym.owner.info.paramss.exists(_ contains sym)) - sym.owner modifyInfo (_ cloneInfo sym.owner) - } changedFreeVars = true debuglog("" + sym + " is free in " + enclosure); if (sym.isVariable) sym setFlag CAPTURED @@ -214,25 +230,27 @@ abstract class LambdaLift extends InfoTransform { } while (changedFreeVars) def renameSym(sym: Symbol) { + val originalName = sym.name + sym setName newName(sym) + debuglog("renaming in %s: %s => %s".format(sym.owner.fullLocationString, originalName, sym.name)) + } + + def newName(sym: Symbol): Name = { val originalName = sym.name def freshen(prefix: String): Name = if (originalName.isTypeName) unit.freshTypeName(prefix) else unit.freshTermName(prefix) - val newName: Name = ( - if (sym.isAnonymousFunction && sym.owner.isMethod) { - freshen(sym.name + nme.NAME_JOIN_STRING + sym.owner.name + nme.NAME_JOIN_STRING) - } else { - // SI-5652 If the lifted symbol is accessed from an inner class, it will be made public. (where?) - // Generating a a unique name, mangled with the enclosing class name, avoids a VerifyError - // in the case that a sub-class happens to lifts out a method with the *same* name. - val name = freshen(sym.name + nme.NAME_JOIN_STRING) - if (originalName.isTermName && !sym.enclClass.isImplClass && calledFromInner(sym)) nme.expandedName(name, sym.enclClass) - else name - } - ) - sym setName newName - debuglog("renaming in %s: %s => %s".format(sym.owner.fullLocationString, originalName, sym.name)) + if (sym.isAnonymousFunction && sym.owner.isMethod) { + freshen(sym.name + nme.NAME_JOIN_STRING + sym.owner.name + nme.NAME_JOIN_STRING) + } else { + // SI-5652 If the lifted symbol is accessed from an inner class, it will be made public. (where?) + // Generating a a unique name, mangled with the enclosing class name, avoids a VerifyError + // in the case that a sub-class happens to lifts out a method with the *same* name. + val name = freshen(sym.name + nme.NAME_JOIN_STRING) + if (originalName.isTermName && !sym.enclClass.isImplClass && calledFromInner(sym)) nme.expandedName(name, sym.enclClass) + else name + } } /** Rename a trait's interface and implementation class in coordinated fashion. @@ -245,6 +263,8 @@ abstract class LambdaLift extends InfoTransform { debuglog("renaming impl class in step with %s: %s => %s".format(traitSym, originalImplName, implSym.name)) } + val allFree: Set[Symbol] = free.values.flatMap(_.iterator).toSet + for (sym <- renamable) { // If we renamed a trait from Foo to Foo$1, we must rename the implementation // class from Foo$class to Foo$1$class. (Without special consideration it would @@ -252,7 +272,9 @@ abstract class LambdaLift extends InfoTransform { // under us, and there's no reliable link between trait symbol and impl symbol, // we have maps from ((trait, name)) -> owner and ((owner, name)) -> impl. localTraits remove ((sym, sym.name)) match { - case None => renameSym(sym) + case None => + if (allFree(sym)) proxyNames(sym) = newName(sym) + else renameSym(sym) case Some(owner) => localImplClasses remove ((owner, sym.name)) match { case Some(implSym) => renameTrait(sym, implSym) @@ -267,7 +289,8 @@ abstract class LambdaLift extends InfoTransform { debuglog("free var proxy: %s, %s".format(owner.fullLocationString, freeValues.toList.mkString(", "))) proxies(owner) = for (fv <- freeValues.toList) yield { - val proxy = owner.newValue(fv.name, owner.pos, newFlags) setInfo fv.info + val proxyName = proxyNames.getOrElse(fv, fv.name) + val proxy = owner.newValue(proxyName, owner.pos, newFlags) setInfo fv.info if (owner.isClass) owner.info.decls enter proxy proxy } @@ -280,9 +303,9 @@ abstract class LambdaLift extends InfoTransform { if (enclosure eq NoSymbol) throw new IllegalArgumentException("Could not find proxy for "+ sym.defString +" in "+ sym.ownerChain +" (currentOwner= "+ currentOwner +" )") debuglog("searching for " + sym + "(" + sym.owner + ") in " + enclosure + " " + enclosure.logicallyEnclosingMember) - val ps = (proxies get enclosure.logicallyEnclosingMember).toList.flatten filter (_.name == sym.name) - if (ps.isEmpty) searchIn(enclosure.skipConstructor.owner) - else ps.head + val proxyName = proxyNames.getOrElse(sym, sym.name) + val ps = (proxies get enclosure.logicallyEnclosingMember).toList.flatten find (_.name == proxyName) + ps getOrElse searchIn(enclosure.skipConstructor.owner) } debuglog("proxy %s from %s has logical enclosure %s".format( sym.debugLocationString, @@ -501,8 +524,10 @@ abstract class LambdaLift extends InfoTransform { } override def transformUnit(unit: CompilationUnit) { - computeFreeVars - afterOwnPhase(super.transformUnit(unit)) + computeFreeVars() + afterOwnPhase { + super.transformUnit(unit) + } assert(liftedDefs.isEmpty, liftedDefs.keys mkString ", ") } } // class LambdaLifter diff --git a/test/files/pos/t6028/t6028_1.scala b/test/files/pos/t6028/t6028_1.scala new file mode 100644 index 0000000000..6edb76069e --- /dev/null +++ b/test/files/pos/t6028/t6028_1.scala @@ -0,0 +1,3 @@ +class C { + def foo(a: Int): Unit = () => a +} diff --git a/test/files/pos/t6028/t6028_2.scala b/test/files/pos/t6028/t6028_2.scala new file mode 100644 index 0000000000..f44048c0ab --- /dev/null +++ b/test/files/pos/t6028/t6028_2.scala @@ -0,0 +1,4 @@ +object Test { + // ensure that parameter names are untouched by lambdalift + new C().foo(a = 0) +} diff --git a/test/files/run/t6028.check b/test/files/run/t6028.check index 6c9930b503..dca61115ad 100644 --- a/test/files/run/t6028.check +++ b/test/files/run/t6028.check @@ -8,20 +8,20 @@ package { }; private[this] val field: Int = 0; def field(): Int = T.this.field; - def foo(methodParam$1: Int): Function0 = { - val methodLocal$1: Int = 0; + def foo(methodParam: Int): Function0 = { + val methodLocal: Int = 0; { - (new anonymous class $anonfun$foo$1(T.this, methodParam$1, methodLocal$1): Function0) + (new anonymous class $anonfun$foo$1(T.this, methodParam, methodLocal): Function0) } }; - def bar(barParam$1: Int): Object = { - @volatile var MethodLocalObject$module$1: scala.runtime.VolatileObjectRef = new scala.runtime.VolatileObjectRef(); - T.this.MethodLocalObject$1(barParam$1, MethodLocalObject$module$1) + def bar(barParam: Int): Object = { + @volatile var MethodLocalObject$module: scala.runtime.VolatileObjectRef = new scala.runtime.VolatileObjectRef(); + T.this.MethodLocalObject$1(barParam, MethodLocalObject$module) }; - def tryy(tryyParam$1: Int): Function0 = { - var tryyLocal$1: scala.runtime.IntRef = new scala.runtime.IntRef(0); + def tryy(tryyParam: Int): Function0 = { + var tryyLocal: scala.runtime.IntRef = new scala.runtime.IntRef(0); { - (new anonymous class $anonfun$tryy$1(T.this, tryyParam$1, tryyLocal$1): Function0) + (new anonymous class $anonfun$tryy$1(T.this, tryyParam, tryyLocal): Function0) } }; @SerialVersionUID(0) final class $anonfun$foo$1 extends scala.runtime.AbstractFunction0$mcI$sp with Serializable { diff --git a/test/files/run/t6028.scala b/test/files/run/t6028.scala index 7611aee0fc..cab17535fc 100644 --- a/test/files/run/t6028.scala +++ b/test/files/run/t6028.scala @@ -14,7 +14,6 @@ object Test extends DirectTest { |""".stripMargin.trim override def show(): Unit = { - // redirect err to out, for logging Console.withErr(System.out) { compile() } -- cgit v1.2.3