Integration with HoistSuperArgs

Do the necessary changes to combine HoistSuperArgs and ByNameClosures.
Also, polishings.

Author: odersky

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

@@ -55,7 +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 ByNameClosures,      // Expand arguments to by-name parameters to closures
            new LiftTry,             // Put try expressions that might execute on non-empty stacks into their own methods
            new HoistSuperArgs,      // Hoist complex arguments of supercalls to enclosing scope
            new ClassOf),            // Expand `Predef.classOf` calls.
@@ -71,7 +71,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
+           new ElimByName,          // Expand by-name parameter references
            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/core/Definitions.scala

@@ -280,9 +280,11 @@ class Definitions {
     def ObjectMethods = List(Object_eq, Object_ne, Object_synchronized, Object_clone,
         Object_finalize, Object_notify, Object_notifyAll, Object_wait, Object_waitL, Object_waitLI)
 
-  /** Dummy method needed by elimByName */
-  lazy val dummyApply = enterPolyMethod(
-      OpsPackageClass, nme.dummyApply, 1,
+  /** Marker method to indicate an argument to a call-by-name parameter.
+   *  Created by byNameClosures and elimByName, eliminated by Erasure,
+   */
+  lazy val cbnArg = enterPolyMethod(
+      OpsPackageClass, nme.cbnArg, 1,
       pt => MethodType(List(FunctionOf(Nil, TypeParamRef(pt, 0))), TypeParamRef(pt, 0)))
 
   /** Method representing a throw */

compiler/src/dotty/tools/dotc/core/StdNames.scala

@@ -378,6 +378,7 @@ object StdNames {
     val build : N               = "build"
     val bytes: N                = "bytes"
     val canEqual_ : N           = "canEqual"
+    val cbnArg: N               = "<cbn-arg>"
     val checkInitialized: N     = "checkInitialized"
     val ClassManifestFactory: N = "ClassManifestFactory"
     val classOf: N              = "classOf"
@@ -391,7 +392,6 @@ object StdNames {
     val delayedInitArg: N       = "delayedInit$body"
     val drop: N                 = "drop"
     val dynamics: N             = "dynamics"
-    val dummyApply: N           = "<dummy-apply>"
     val elem: N                 = "elem"
     val emptyValDef: N          = "emptyValDef"
     val ensureAccessible : N    = "ensureAccessible"

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

@@ -15,24 +15,23 @@ import core.StdNames.nme
 /** This phase translates arguments to call-by-name parameters, using the rules
  *
  *      x           ==>    x                   if x is a => parameter
- *      e.apply()   ==>    DummyApply(e)       if e is pure
- *      e           ==>    DummyApply(() => e) for all other arguments
+ *      e.apply()   ==>    <cbn-arg>(e)       if e is pure
+ *      e           ==>    <cbn-arg>(() => e) for all other arguments
  *
  *  where
  *
- *     DummyApply: [T](() => T): T
+ *     <cbn-arg>: [T](() => T): T
  *
- *  is a synthetic method defined in Definitions. Erasure will later strip these DummyApply wrappers.
+ *  is a synthetic method defined in Definitions. Erasure will later strip the <cbn-arg> wrappers.
  */
 class ByNameClosures extends TransformByNameApply with IdentityDenotTransformer { thisTransformer =>
   import ast.tpd._
 
-  override def phaseName: String = "bynameClosures"
+  override def phaseName: String = "byNameClosures"
 
   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))
+      ctx.owner, nme.ANON_FUN, Synthetic | Method, MethodType(Nil, Nil, argType))
     Closure(meth, _ => arg.changeOwnerAfter(ctx.owner, meth, thisTransformer))
   }
 }

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

@@ -15,7 +15,7 @@ import util.Attachment
 import core.StdNames.nme
 import ast.Trees._
 
-/** This phase eliminates ExprTypes `=> T` as types of function parameters, and replaces them by
+/** This phase eliminates ExprTypes `=> T` as types of method parameter references, and replaces them b
  *  nullary function types.  More precisely:
  *
  *  For the types of parameter symbols:
@@ -25,7 +25,6 @@ import ast.Trees._
  *  For cbn parameter values
  *
  *         x          ==>    x()
- *         CbnArg(x)  ==>    DummyApply(x)
  *
  *  Note: This scheme to have inconsistent types between method types (whose formal types are still
  *  ExprTypes and parameter valdefs (which are now FunctionTypes) is not pretty. There are two
@@ -43,10 +42,8 @@ class ElimByName extends TransformByNameApply with InfoTransformer { thisTransfo
 
   override def phaseName: String = "elimByName"
 
-  override def runsAfter = Set(classOf[HoistSuperArgs])
   override def runsAfterGroupsOf = Set(classOf[Splitter])
-    // assumes idents and selects have symbols; interferes with splitter distribution
-    // that's why it's "after group".
+    // I got errors running this phase in an earlier group, but I did not track them down.
 
   /** Map `tree` to `tree.apply()` is `ftree` was of ExprType and becomes now a function */
   private def applyIfFunction(tree: Tree, ftree: Tree)(implicit ctx: Context) =

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

@@ -452,7 +452,7 @@ object Erasure extends TypeTestsCasts{
 	 */
     override def typedApply(tree: untpd.Apply, pt: Type)(implicit ctx: Context): Tree = {
       val Apply(fun, args) = tree
-      if (fun.symbol == defn.dummyApply)
+      if (fun.symbol == defn.cbnArg)
         typedUnadapted(args.head, pt)
       else typedExpr(fun, FunProto(args, pt, this)) match {
         case fun1: Apply => // arguments passed in prototype were already passed

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

@@ -42,6 +42,11 @@ class HoistSuperArgs extends MiniPhaseTransform with IdentityDenotTransformer {
 
   def phaseName = "hoistSuperArgs"
 
+  override def runsAfter = Set(classOf[ByNameClosures])
+    // By name closures need to be introduced first in order to be hoisted out here.
+    // There's an interaction with by name closures in that the <cbn-arg> marker
+    // application should not be hoisted, but be left at the point of call.
+
   /** Hoist complex arguments in super call `parent` out of the class.
    *  @return A pair consisting of the transformed super call and a list of super argument
    *          defininitions.
@@ -81,16 +86,18 @@ class HoistSuperArgs extends MiniPhaseTransform with IdentityDenotTransformer {
       if (methOwner.isClass) meth.enteredAfter(thisTransform) else meth
     }
 
+    def refNeedsHoist(tp: Type): Boolean = tp match {
+      case tp: ThisType => !tp.cls.isStaticOwner && tp.cls != cls
+      case tp: TermRef => refNeedsHoist(tp.prefix)
+      case _ => false
+    }
+
     /** Super call argument is complex, needs to be hoisted */
     def needsHoist(tree: Tree) = tree match {
       case _: DefDef => true
       case _: Template => true
-      case _: This => !tree.symbol.isStaticOwner
       case _: New => !tree.tpe.typeSymbol.isStatic
-      case _: RefTree =>
-        var owner = tree.symbol.effectiveOwner
-        if (owner.isLocalDummy) owner = owner.owner
-        tree.isTerm && !owner.isStaticOwner && owner != cls
+      case _: RefTree | _: This => refNeedsHoist(tree.tpe)
       case _ => false
     }
 
@@ -99,8 +106,10 @@ class HoistSuperArgs extends MiniPhaseTransform with IdentityDenotTransformer {
      *  @return  The argument after possible hoisting
      *  Might append a method definition to `superArgs` as a side effect.
      */
-    def hoistSuperArg(arg: Tree) =
-      if (arg.existsSubTree(needsHoist)) {
+    def hoistSuperArg(arg: Tree): Tree = arg match {
+      case Apply(fn, arg1 :: Nil) if fn.symbol == defn.cbnArg =>
+        cpy.Apply(arg)(fn, hoistSuperArg(arg1) :: Nil)
+      case _ if (arg.existsSubTree(needsHoist)) =>
         val superMeth = newSuperArgMethod(arg.tpe)
         val superArgDef = polyDefDef(superMeth, trefs => vrefss => {
           val paramSyms = trefs.map(_.typeSymbol) ::: vrefss.flatten.map(_.symbol)
@@ -139,8 +148,8 @@ class HoistSuperArgs extends MiniPhaseTransform with IdentityDenotTransformer {
           .appliedToArgss(termParamRefs(constr.info))
         ctx.log(i"hoist $arg, cls = $cls = $res")
         res
-      }
-      else arg
+      case _ => arg
+    }
 
     def recur(tree: Tree): Tree = tree match {
       case Apply(fn, args) => cpy.Apply(tree)(recur(fn), args.mapconserve(hoistSuperArg))

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

@@ -13,7 +13,7 @@ import DenotTransformers._
 import core.StdNames.nme
 import ast.Trees._
 
-/** Abstract base class of ByNameClosures and ELimByName, factoring out the
+/** 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 =>
@@ -32,7 +32,7 @@ abstract class TransformByNameApply extends MiniPhaseTransform { thisTransformer
     origDenot.info.isInstanceOf[ExprType] && exprBecomesFunction(origDenot)
   }
 
-  def mkClosure(arg: Tree, argType: Type)(implicit ctx: Context): Tree = unsupported("mkClosure")
+  def mkClosure(arg: Tree, argType: Type)(implicit ctx: Context): Tree = unsupported(i"mkClosure($arg)")
 
   override def transformApply(tree: Apply)(implicit ctx: Context, info: TransformerInfo): Tree =
     ctx.traceIndented(s"transforming ${tree.show} at phase ${ctx.phase}", show = true) {
@@ -41,13 +41,13 @@ abstract class TransformByNameApply extends MiniPhaseTransform { thisTransformer
       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)
+        def wrap(arg: Tree) = ref(defn.cbnArg).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
+            if (isByNameRef(arg) || arg.symbol == defn.cbnArg) arg
             else wrap(mkClosure(arg, argType))
         }
       case _ =>

tests/neg/tailcall/t1672b.scala

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