From a8b6e361de37705036c4e8307ad75e279fafe9f5 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 19 Jan 2015 21:40:20 +0100 Subject: [PATCH 1/3] SI-9097 Remove spurious warning about conflicting filenames When using delambdafy:method, closure classes are generated late. The class is added to a map and integrated into the PackageDef in transformStats. When declaring a package object, there are potentially multiple PackageDefs for the same package. In this case, the closure class was added to all of them. As a result, GenASM / GenBCode would run multiple times on the closure class. In GenBCode this would trigger a warning about conflicting filenames. --- .../tools/nsc/transform/Delambdafy.scala | 4 ++- test/files/pos/t9097.flags | 1 + test/files/pos/t9097.scala | 9 +++++++ test/files/run/t9097.scala | 26 +++++++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/t9097.flags create mode 100644 test/files/pos/t9097.scala create mode 100644 test/files/run/t9097.scala diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala index d2c511a2d1fd..1f832ba81eab 100644 --- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala +++ b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala @@ -113,7 +113,9 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre // after working on the entire compilation until we'll have a set of // new class definitions to add to the top level override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = { - super.transformStats(stats, exprOwner) ++ lambdaClassDefs(exprOwner) + // Need to remove from the lambdaClassDefs map: there may be multiple PackageDef for the same + // package when defining a package object. We only add the lambda class to one. See SI-9097. + super.transformStats(stats, exprOwner) ++ lambdaClassDefs.remove(exprOwner).getOrElse(Nil) } private def optionSymbol(sym: Symbol): Option[Symbol] = if (sym.exists) Some(sym) else None diff --git a/test/files/pos/t9097.flags b/test/files/pos/t9097.flags new file mode 100644 index 000000000000..0f8175b88bf6 --- /dev/null +++ b/test/files/pos/t9097.flags @@ -0,0 +1 @@ +-Ydelambdafy:method -Ybackend:GenBCode -Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t9097.scala b/test/files/pos/t9097.scala new file mode 100644 index 000000000000..5e0e9212715a --- /dev/null +++ b/test/files/pos/t9097.scala @@ -0,0 +1,9 @@ +package o +package a { + class C { + def hihi = List(1,2).map(_ * 2) + } +} +package object a { + def f = 1 +} diff --git a/test/files/run/t9097.scala b/test/files/run/t9097.scala new file mode 100644 index 000000000000..0f148c3b9deb --- /dev/null +++ b/test/files/run/t9097.scala @@ -0,0 +1,26 @@ +import scala.tools.partest._ +import java.io.{Console => _, _} + +object Test extends DirectTest { + + override def extraSettings: String = "-usejavacp -Ydelambdafy:method -Xprint:delambdafy -d " + testOutput.path + + override def code = """package o + |package a { + | class C { + | def hihi = List(1,2).map(_ * 2) + | } + |} + |package object a { + | def f = 1 + |} + |""".stripMargin.trim + + override def show(): Unit = { + val baos = new java.io.ByteArrayOutputStream() + Console.withOut(baos)(Console.withErr(baos)(compile())) + val out = baos.toString("UTF-8") + // was 2 before the fix, the two PackageDefs for a would both contain the ClassDef for the closure + assert(out.lines.count(_ contains "class hihi$1") == 1, out) + } +} From a8bfa82dcb470f31953f43e0db46570e7a020855 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 20 Jan 2015 07:56:17 -0800 Subject: [PATCH 2/3] SI-9097 Consolidate test `pos` test is subsumed by `run`. --- test/files/pos/t9097.flags | 1 - test/files/pos/t9097.scala | 9 --------- test/files/run/t9097.scala | 12 ++++++++++-- 3 files changed, 10 insertions(+), 12 deletions(-) delete mode 100644 test/files/pos/t9097.flags delete mode 100644 test/files/pos/t9097.scala diff --git a/test/files/pos/t9097.flags b/test/files/pos/t9097.flags deleted file mode 100644 index 0f8175b88bf6..000000000000 --- a/test/files/pos/t9097.flags +++ /dev/null @@ -1 +0,0 @@ --Ydelambdafy:method -Ybackend:GenBCode -Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t9097.scala b/test/files/pos/t9097.scala deleted file mode 100644 index 5e0e9212715a..000000000000 --- a/test/files/pos/t9097.scala +++ /dev/null @@ -1,9 +0,0 @@ -package o -package a { - class C { - def hihi = List(1,2).map(_ * 2) - } -} -package object a { - def f = 1 -} diff --git a/test/files/run/t9097.scala b/test/files/run/t9097.scala index 0f148c3b9deb..d2bf55fc4481 100644 --- a/test/files/run/t9097.scala +++ b/test/files/run/t9097.scala @@ -1,9 +1,16 @@ import scala.tools.partest._ import java.io.{Console => _, _} -object Test extends DirectTest { +object Test extends StoreReporterDirectTest { - override def extraSettings: String = "-usejavacp -Ydelambdafy:method -Xprint:delambdafy -d " + testOutput.path + override def extraSettings: String = List( + "-usejavacp", + "-Xfatal-warnings", + "-Ybackend:GenBCode", + "-Ydelambdafy:method", + "-Xprint:delambdafy", + s"-d ${testOutput.path}" + ) mkString " " override def code = """package o |package a { @@ -19,6 +26,7 @@ object Test extends DirectTest { override def show(): Unit = { val baos = new java.io.ByteArrayOutputStream() Console.withOut(baos)(Console.withErr(baos)(compile())) + assert(!storeReporter.hasErrors, message = filteredInfos map (_.msg) mkString "; ") val out = baos.toString("UTF-8") // was 2 before the fix, the two PackageDefs for a would both contain the ClassDef for the closure assert(out.lines.count(_ contains "class hihi$1") == 1, out) From 486f92c5ddd6f02cdcd6e32329ce92f90a9fa1c9 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 3 Feb 2015 15:15:51 +1000 Subject: [PATCH 3/3] Refactor transformStats impls for consistency Uses the same idiom in `Flatten` and `LambdaLift` as is established in `Extender` and (recently) `Delambdafy`. This avoids any possibility of adding a member to a package twice, as used to happen in SI-9097. --- src/compiler/scala/tools/nsc/transform/Flatten.scala | 2 +- src/compiler/scala/tools/nsc/transform/LambdaLift.scala | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala index 6149e40fa7f7..fbb030777341 100644 --- a/src/compiler/scala/tools/nsc/transform/Flatten.scala +++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala @@ -166,7 +166,7 @@ abstract class Flatten extends InfoTransform { override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = { val stats1 = super.transformStats(stats, exprOwner) if (currentOwner.isPackageClass) { - val lifted = liftedDefs(currentOwner).toList + val lifted = liftedDefs.remove(currentOwner).toList.flatten stats1 ::: lifted } else stats1 diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index fa0c1f797bc3..a703542587e7 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -539,12 +539,11 @@ abstract class LambdaLift extends InfoTransform { override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = { def addLifted(stat: Tree): Tree = stat match { case ClassDef(_, _, _, _) => - val lifted = liftedDefs get stat.symbol match { + val lifted = liftedDefs remove stat.symbol match { case Some(xs) => xs reverseMap addLifted case _ => log("unexpectedly no lifted defs for " + stat.symbol) ; Nil } - try deriveClassDef(stat)(impl => deriveTemplate(impl)(_ ::: lifted)) - finally liftedDefs -= stat.symbol + deriveClassDef(stat)(impl => deriveTemplate(impl)(_ ::: lifted)) case DefDef(_, _, _, _, _, Block(Nil, expr)) if !stat.symbol.isConstructor => deriveDefDef(stat)(_ => expr)