diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2012-07-07 09:24:50 +0200 |
---|---|---|
committer | Adriaan Moors <adriaan.moors@epfl.ch> | 2012-07-14 17:46:45 +0200 |
commit | a3bf34563d718f19ad02ff9ac5a2a1cec865aa24 (patch) | |
tree | 0725474dd1873e173a60508c420d031c7347d8c2 | |
parent | 32dc7e80698c83947bf4b74f6eadd385a06f5b09 (diff) | |
download | scala-a3bf34563d718f19ad02ff9ac5a2a1cec865aa24.tar.gz scala-a3bf34563d718f19ad02ff9ac5a2a1cec865aa24.tar.bz2 scala-a3bf34563d718f19ad02ff9ac5a2a1cec865aa24.zip |
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.
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/LambdaLift.scala | 85 | ||||
-rw-r--r-- | test/files/pos/t6028/t6028_1.scala | 3 | ||||
-rw-r--r-- | test/files/pos/t6028/t6028_2.scala | 4 | ||||
-rw-r--r-- | test/files/run/t6028.check | 18 | ||||
-rw-r--r-- | test/files/run/t6028.scala | 1 |
5 files changed, 71 insertions, 40 deletions
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 @@ -215,24 +231,26 @@ abstract class LambdaLift extends InfoTransform { 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 <empty> { }; private[this] val field: Int = 0; <stable> <accessor> 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(<empty>); - 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(<empty>); + 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 <synthetic> 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() } |