Move ByNameClosure earlier in the pipleline

 - Make it use changeOwnerAfter
 - Factor out TransformByNameApply as a common base trait
   between it and ElimByName

Author: odersky

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -55,6 +55,7 @@ class Compiler {
            new ExtensionMethods,    // Expand methods of value classes with extension methods
            new ExpandSAMs,          // Expand single abstract method closures to anonymous classes
            new TailRec,             // Rewrite tail recursion to loops
+           new ByNameClosures), List(      // Expand by-name arguments
            new LiftTry,             // Put try expressions that might execute on non-empty stacks into their own methods
            new ClassOf),            // Expand `Predef.classOf` calls.
       List(new TryCatchPatterns,    // Compile cases in try/catch
@@ -69,7 +70,7 @@ class Compiler {
            new SeqLiterals,         // Express vararg arguments as arrays
            new InterceptedMethods,  // Special handling of `==`, `|=`, `getClass` methods
            new Getters,             // Replace non-private vals and vars with getter defs (fields are added later)
-           new ElimByName,          // Expand by-name parameters and arguments
+           new ElimByName,          // Expand by-name parameters
            new AugmentScala2Traits, // Expand traits defined in Scala 2.11 to simulate old-style rewritings
            new ResolveSuper,        // Implement super accessors and add forwarders to trait methods
            new PrimitiveForwarders, // Add forwarders to trait methods that have a mismatch between generic and primitives

compiler/src/dotty/tools/dotc/transform/ByNameClosures.scala

@@ -9,8 +9,8 @@ import Contexts._
 import Types._
 import Flags._
 import Decorators._
+import DenotTransformers.IdentityDenotTransformer
 import core.StdNames.nme
-import ast.Trees._
 
 /** This phase translates arguments to call-by-name parameters, using the rules
  *
@@ -24,55 +24,15 @@ import ast.Trees._
  *
  *  is a synthetic method defined in Definitions. Erasure will later strip these DummyApply wrappers.
  */
-class ByNameClosures extends MiniPhaseTransform { thisTransformer =>
+class ByNameClosures extends TransformByNameApply with IdentityDenotTransformer { thisTransformer =>
   import ast.tpd._
 
   override def phaseName: String = "bynameClosures"
 
-  /** The info of the tree's symbol at phase Nullarify (i.e. before transformation) */
-  private def originalDenotation(tree: Tree)(implicit ctx: Context) =
-    tree.symbol.denot(ctx.withPhase(thisTransformer))
-
-  /** If denotation had an ExprType before, it now gets a function type */
-  protected def exprBecomesFunction(symd: SymDenotation)(implicit ctx: Context) =
-    (symd is Param) || (symd is (ParamAccessor, butNot = Method))
-
-  protected def isByNameRef(tree: Tree)(implicit ctx: Context) = {
-    val origDenot = originalDenotation(tree)
-    origDenot.info.isInstanceOf[ExprType] && exprBecomesFunction(origDenot)
-  }
-
-  override def transformApply(tree: Apply)(implicit ctx: Context, info: TransformerInfo): Tree =
-    ctx.traceIndented(s"transforming ${tree.show} at phase ${ctx.phase}", show = true) {
-
-    def transformArg(arg: Tree, formal: Type): Tree = formal.dealias match {
-      case formalExpr: ExprType =>
-        var argType = arg.tpe.widenIfUnstable
-        if (defn.isBottomType(argType)) argType = formal.widenExpr
-        def wrap(arg: Tree) = ref(defn.dummyApply).appliedToType(argType).appliedTo(arg)
-        arg match {
-          case Apply(Select(qual, nme.apply), Nil)
-          if qual.tpe.derivesFrom(defn.FunctionClass(0)) && isPureExpr(qual) =>
-            wrap(qual)
-          case _ =>
-            if (isByNameRef(arg) || arg.symbol == defn.dummyApply) arg
-            else {
-              val inSuper = if (ctx.mode.is(Mode.InSuperCall)) InSuperCall else EmptyFlags
-              val meth = ctx.newSymbol(
-                  ctx.owner, nme.ANON_FUN, Synthetic | Method | inSuper, MethodType(Nil, Nil, argType))
-              wrap(Closure(meth, _ =>
-                atGroupEnd { implicit ctx: Context =>
-                  arg.changeOwner(ctx.owner, meth)
-                }
-              ))
-          }
-        }
-      case _ =>
-        arg
-    }
-
-    val mt @ MethodType(_) = tree.fun.tpe.widen
-    val args1 = tree.args.zipWithConserve(mt.paramInfos)(transformArg)
-    cpy.Apply(tree)(tree.fun, args1)
+  override def mkClosure(arg: Tree, argType: Type)(implicit ctx: Context): Tree = {
+    val inSuper = if (ctx.mode.is(Mode.InSuperCall)) InSuperCall else EmptyFlags
+    val meth = ctx.newSymbol(
+      ctx.owner, nme.ANON_FUN, Synthetic | Method | inSuper, MethodType(Nil, Nil, argType))
+    Closure(meth, _ => arg.changeOwnerAfter(ctx.owner, meth, thisTransformer))
   }
 }

compiler/src/dotty/tools/dotc/transform/ElimByName.scala

@@ -3,7 +3,7 @@ package transform
 
 import TreeTransforms._
 import core._
-import DenotTransformers._
+import DenotTransformers.InfoTransformer
 import Symbols._
 import SymDenotations._
 import Contexts._
@@ -38,7 +38,7 @@ import ast.Trees._
  *  Option 2: Merge ElimByName with erasure, or have it run immediately before. This has not been
  *  tried yet.
  */
-class ElimByName extends ByNameClosures with InfoTransformer { thisTransformer =>
+class ElimByName extends TransformByNameApply with InfoTransformer { thisTransformer =>
   import ast.tpd._
 
   override def phaseName: String = "elimByName"

compiler/src/dotty/tools/dotc/transform/TransformByNameApply.scala

@@ -0,0 +1,61 @@
+package dotty.tools.dotc
+package transform
+
+import TreeTransforms._
+import core._
+import Symbols._
+import SymDenotations._
+import Contexts._
+import Types._
+import Flags._
+import Decorators._
+import DenotTransformers._
+import core.StdNames.nme
+import ast.Trees._
+
+/** Abstract base class of ByNameClosures and ELimByName, factoring out the
+ *  common functionality to transform arguments of by-name parameters.
+ */
+abstract class TransformByNameApply extends MiniPhaseTransform { thisTransformer: DenotTransformer =>
+  import ast.tpd._
+
+  /** The info of the tree's symbol at phase Nullarify (i.e. before transformation) */
+  private def originalDenotation(tree: Tree)(implicit ctx: Context) =
+    tree.symbol.denot(ctx.withPhase(thisTransformer))
+
+  /** If denotation had an ExprType before, it now gets a function type */
+  protected def exprBecomesFunction(symd: SymDenotation)(implicit ctx: Context) =
+    (symd is Param) || (symd is (ParamAccessor, butNot = Method))
+
+  protected def isByNameRef(tree: Tree)(implicit ctx: Context) = {
+    val origDenot = originalDenotation(tree)
+    origDenot.info.isInstanceOf[ExprType] && exprBecomesFunction(origDenot)
+  }
+
+  def mkClosure(arg: Tree, argType: Type)(implicit ctx: Context): Tree = unsupported("mkClosure")
+
+  override def transformApply(tree: Apply)(implicit ctx: Context, info: TransformerInfo): Tree =
+    ctx.traceIndented(s"transforming ${tree.show} at phase ${ctx.phase}", show = true) {
+
+    def transformArg(arg: Tree, formal: Type): Tree = formal.dealias match {
+      case formalExpr: ExprType =>
+        var argType = arg.tpe.widenIfUnstable
+        if (defn.isBottomType(argType)) argType = formal.widenExpr
+        def wrap(arg: Tree) = ref(defn.dummyApply).appliedToType(argType).appliedTo(arg)
+        arg match {
+          case Apply(Select(qual, nme.apply), Nil)
+          if qual.tpe.derivesFrom(defn.FunctionClass(0)) && isPureExpr(qual) =>
+            wrap(qual)
+          case _ =>
+            if (isByNameRef(arg) || arg.symbol == defn.dummyApply) arg
+            else wrap(mkClosure(arg, argType))
+        }
+      case _ =>
+        arg
+    }
+
+    val mt @ MethodType(_) = tree.fun.tpe.widen
+    val args1 = tree.args.zipWithConserve(mt.paramInfos)(transformArg)
+    cpy.Apply(tree)(tree.fun, args1)
+  }
+}

tests/neg/tailcall/t1672b.scala

@@ -46,7 +46,7 @@ object Test1772B {
     else 1 + (try {
       throw new RuntimeException
     } catch {
-      case _: Throwable => bar(i - 1) // old-error
+      case _: Throwable => bar(i - 1) // error
     })
   }
 }