SI-6306 Remove incorrect eta-expansion optimization in Uncurry

Fix SI-6306 by removing the offending code. Moreover, clarify comment
and add testcase.

This pattern match only matches on testcases triggering SI-6306; the
transformation it performs in that case is unsafe.

The intended optimization is to undo eta-expansion of nullary functions,
that is, transform `() => foo()` to `foo`. But that's only valid when
`foo` is an instance of `Function0`, so the optimization is unsafe.
Moreover, the pattern match will fail because at the end of typer that
code has become `() => foo.apply()`, and `isExprSafeToInline(foo.apply)`
always (correctly) fails the isExprSafeToInline test. The pattern match
should thus be different - this code was dead even when it was
introduced (45bcd02f6ba099277bedbf83ec2bda07435c7797), since it was not
invoked either when building the compiler or when compiling function
attempt2() in the included testcase.

Thanks to all commenters on SI-6306 and
https://github.com/scala/scala/pull/1255, in particular to Jason Zaugg
for diagnosing the underlying fault and Lukas Rytz for understanding the
goal of the code.

4 files changed, 46 insertions, 6 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
index 181463657b..f961685dd4 100644
--- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala
+++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -212,11 +212,6 @@ abstract class UnCurry extends InfoTransform
 
     /** Undo eta expansion for parameterless and nullary methods */
     def deEta(fun: Function): Tree = fun match {
-      case Function(List(), Apply(expr, List())) if treeInfo.isExprSafeToInline(expr) =>
-        if (expr hasSymbolWhich (_.isLazy))
-          fun
-        else
-          expr
       case Function(List(), expr) if isByNameRef(expr) =>
         noApply += expr
         expr
diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala
index 3a930a195b..6ef4c3f660 100644
--- a/src/reflect/scala/reflect/internal/TreeInfo.scala
+++ b/src/reflect/scala/reflect/internal/TreeInfo.scala
@@ -67,7 +67,7 @@ abstract class TreeInfo {
 
   /** Is tree an expression which can be inlined without affecting program semantics?
    *
-   *  Note that this is not called "isExprSafeToInline" since purity (lack of side-effects)
+   *  Note that this is not called "isExprPure" since purity (lack of side-effects)
    *  is not the litmus test.  References to modules and lazy vals are side-effecting,
    *  both because side-effecting code may be executed and because the first reference
    *  takes a different code path than all to follow; but they are safe to inline
diff --git a/test/files/run/pure-args-byname-noinline.check b/test/files/run/pure-args-byname-noinline.check
new file mode 100644
index 0000000000..a39c61eb64
--- /dev/null
+++ b/test/files/run/pure-args-byname-noinline.check
@@ -0,0 +1,12 @@
+2
+2
+2
+2
+List(1)
+List()
+
+1
+1
+1
+1
+1
diff --git a/test/files/run/pure-args-byname-noinline.scala b/test/files/run/pure-args-byname-noinline.scala
new file mode 100644
index 0000000000..5c5c8a7eb6
--- /dev/null
+++ b/test/files/run/pure-args-byname-noinline.scala
@@ -0,0 +1,33 @@
+object Test {
+  //Were affected by SI-6306
+  def f[A](a: =>A) = println(a.toString)
+  def f1[A <: AnyVal](a: =>A) = println(a.toString)
+  def f1a[A <: AnyVal](a: =>A) = println(a.##)
+  def f2[A <: AnyRef](a: =>A) = println(a.toString)
+  def f2a[A <: String](a: =>A) = println(a.toString)
+  //Works
+  def f3[A](a: =>Seq[A]) = println(a.toString)
+
+  def foo() = println(2)
+  def client(f: () => Unit) = {f(); f()}
+  def attempt2() {
+    val bar: () => Unit = foo _
+    //The code causing SI-6306 was supposed to optimize code like this:
+    client(() => bar ())
+    //to:
+    client(bar)
+  }
+  def main(args: Array[String]) {
+    attempt2()
+    f3(Seq(1))
+    f3(Seq())
+    f("")
+    f((1).toString)
+    f((1).##)
+    f1((1).##)
+    f2((1).toString)
+    f2a((1).toString)
+  }
+}
+
+// vim: set ts=8 sw=2 et: