Merge pull request #2282 from dotty-staging/changle-elimbyname

Rework ElimByName and combine with new phase HoistSuperArgs

Author: odersky

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

@@ -55,7 +55,9 @@ 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,      // 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.
       List(new TryCatchPatterns,    // Compile cases in try/catch
            new PatternMatcher,      // Compile pattern matches
@@ -69,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 and arguments
+           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/ast/tpd.scala

@@ -629,7 +629,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
           case tree: DefTree =>
             val sym = tree.symbol
             val prevDenot = sym.denot(ctx.withPhase(trans))
-            if (prevDenot.owner == from) {
+            if (prevDenot.effectiveOwner == from.skipWeakOwner) {
               val d = sym.copySymDenotation(owner = to)
               d.installAfter(trans)
               d.transformAfter(trans, d => if (d.owner eq from) d.copySymDenotation(owner = to) else d)

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/Flags.scala

@@ -372,9 +372,6 @@ object Flags {
   /** Symbol always defines a fresh named type */
   final val Fresh = commonFlag(45, "<fresh>")
 
-  /** Symbol is defined in a super call */
-  final val InSuperCall = commonFlag(46, "<in supercall>")
-
   /** Denotation is in train of being loaded and completed, used to catch cyclic dependencies */
   final val Touched = commonFlag(48, "<touched>")
 
@@ -451,7 +448,7 @@ object Flags {
   /** Flags guaranteed to be set upon symbol creation */
   final val FromStartFlags =
     Module | Package | Deferred | MethodOrHKCommon | Param | ParamAccessor |
-    Scala2ExistentialCommon | Mutable.toCommonFlags | InSuperCall | Touched | JavaStatic |
+    Scala2ExistentialCommon | Mutable.toCommonFlags | Touched | JavaStatic |
     CovariantOrOuter | ContravariantOrLabel | CaseAccessorOrBaseTypeArg |
     Fresh | Frozen | Erroneous | ImplicitCommon | Permanent | Synthetic |
     SuperAccessorOrScala2x | Inline
@@ -511,8 +508,7 @@ object Flags {
     Accessor | AbsOverride | Stable | Captured | Synchronized
 
   /** Flags that can apply to a module class */
-  final val RetainedModuleClassFlags: FlagSet = RetainedModuleValAndClassFlags |
-    InSuperCall | ImplClass
+  final val RetainedModuleClassFlags: FlagSet = RetainedModuleValAndClassFlags | ImplClass
 
   /** Packages and package classes always have these flags set */
   final val PackageCreationFlags =

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

@@ -221,6 +221,7 @@ object NameKinds {
   val ExceptionBinderName     = new UniqueNameKind("ex")
   val SkolemName              = new UniqueNameKind("?")
   val LiftedTreeName          = new UniqueNameKind("liftedTree")
+  val SuperArgName            = new UniqueNameKind("$superArg$")
 
   val UniqueExtMethName = new UniqueNameKind("$extension") {
     override def unmangle(name: SimpleTermName): TermName = {

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

@@ -379,6 +379,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"
@@ -392,7 +393,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/core/SymDenotations.scala

@@ -851,7 +851,7 @@ object SymDenotations {
 
     /** The class containing this denotation.
      *  If this denotation is already a class, return itself
-     *  Definitions flagged with InSuperCall are treated specially.
+     *  Definitions flagged with JavaStatic are treated specially.
      *  Their enclosing class is not the lexically enclosing class,
      *  but in turn the enclosing class of the latter. This reflects
      *  the context created by `Context#superCallContext`, `Context#thisCallArgContext`
@@ -862,7 +862,7 @@ object SymDenotations {
      */
     final def enclosingClass(implicit ctx: Context): Symbol = {
       def enclClass(sym: Symbol, skip: Boolean): Symbol = {
-        def newSkip = sym.is(InSuperCall) || sym.is(JavaStaticTerm)
+        def newSkip = sym.is(JavaStaticTerm)
         if (!sym.exists)
           NoSymbol
         else if (sym.isClass)

compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala

@@ -186,7 +186,6 @@ Standard-Section: "ASTs" TopLevelStat*
                   CONTRAVARIANT             // type param marked “-”
                   SCALA2X                           // Imported from Scala2.x
                   DEFAULTparameterized  // Method with default params
-                  INSUPERCALL           // defined in the argument of a constructor supercall
                   STABLE                // Method that is assumed to be stable
                   Annotation
   Annotation    = ANNOTATION     Length tycon_Type fullAnnotation_Term
@@ -278,8 +277,7 @@ object TastyFormat {
   final val CONTRAVARIANT = 28
   final val SCALA2X = 29
   final val DEFAULTparameterized = 30
-  final val INSUPERCALL = 31
-  final val STABLE = 32
+  final val STABLE = 31
 
   final val SHARED = 64
   final val TERMREFdirect = 65
@@ -403,7 +401,6 @@ object TastyFormat {
        | CONTRAVARIANT
        | SCALA2X
        | DEFAULTparameterized
-       | INSUPERCALL
        | STABLE
        | ANNOTATION
        | PRIVATEqualified
@@ -469,7 +466,6 @@ object TastyFormat {
     case CONTRAVARIANT => "CONTRAVARIANT"
     case SCALA2X => "SCALA2X"
     case DEFAULTparameterized => "DEFAULTparameterized"
-    case INSUPERCALL => "INSUPERCALL"
     case STABLE => "STABLE"
 
     case SHARED => "SHARED"

compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

@@ -577,7 +577,6 @@ class TreePickler(pickler: TastyPickler) {
     if (flags is Synthetic) writeByte(SYNTHETIC)
     if (flags is Artifact) writeByte(ARTIFACT)
     if (flags is Scala2x) writeByte(SCALA2X)
-    if (flags is InSuperCall) writeByte(INSUPERCALL)
     if (sym.isTerm) {
       if (flags is Implicit) writeByte(IMPLICIT)
       if ((flags is Lazy) && !(sym is Module)) writeByte(LAZY)

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -510,7 +510,6 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi
           case CONTRAVARIANT => addFlag(Contravariant)
           case SCALA2X => addFlag(Scala2x)
           case DEFAULTparameterized => addFlag(DefaultParameterized)
-          case INSUPERCALL => addFlag(InSuperCall)
           case STABLE => addFlag(Stable)
           case PRIVATEqualified =>
             readByte()

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

@@ -0,0 +1,37 @@
+package dotty.tools.dotc
+package transform
+
+import TreeTransforms._
+import core._
+import Symbols._
+import SymDenotations._
+import Contexts._
+import Types._
+import Flags._
+import Decorators._
+import DenotTransformers.IdentityDenotTransformer
+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()   ==>    <cbn-arg>(e)       if e is pure
+ *      e           ==>    <cbn-arg>(() => e) for all other arguments
+ *
+ *  where
+ *
+ *     <cbn-arg>: [T](() => T): T
+ *
+ *  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 mkByNameClosure(arg: Tree, argType: Type)(implicit ctx: Context): Tree = {
+    val meth = ctx.newSymbol(
+      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/Constructors.scala

@@ -30,8 +30,7 @@ class Constructors extends MiniPhaseTransform with IdentityDenotTransformer { th
   import tpd._
 
   override def phaseName: String = "constructors"
-  override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Memoize])
-
+  override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Memoize], classOf[HoistSuperArgs])
 
   // Collect all private parameter accessors and value definitions that need
   // to be retained. There are several reasons why a parameter accessor or
@@ -103,7 +102,7 @@ class Constructors extends MiniPhaseTransform with IdentityDenotTransformer { th
    *                outer link, so no parameter accessors need to be rewired to parameters
    */
   private def noDirectRefsFrom(tree: Tree)(implicit ctx: Context) =
-    tree.isDef && tree.symbol.isClass && !tree.symbol.is(InSuperCall)
+    tree.isDef && tree.symbol.isClass
 
   /** Class members that can be eliminated if referenced only from their own
    *  constructor.

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._
@@ -15,32 +15,16 @@ 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:
  *
- *      => T        ==>    () => T
+ *         => T       ==>    () => T
  *
- *  Note that `=> T` types are not eliminated in MethodTypes. This is done later at erasure.
- *  Terms are rewritten as follows:
+ *  For cbn parameter values
  *
- *      x           ==>    x.apply()   if x is a parameter that had type => T
- *
- *  Arguments to call-by-name parameters are translated as follows. First, the argument is
- *  rewritten by the rules
- *
- *      e.apply()   ==>    e           if e.apply() is an argument to a call-by-name parameter
- *      expr        ==>    () => expr  if other expr is an argument to a call-by-name parameter
- *
- *  This makes the argument compatible with a parameter type of () => T, which will be the
- *  formal parameter type at erasure. But to be -Ycheckable until then, any argument
- *  ARG rewritten by the rules above is again wrapped in an application DummyApply(ARG)
- *  where
- *
- *     DummyApply: [T](() => T): T
- *
- *  is a synthetic method defined in Definitions. Erasure will later strip these DummyApply wrappers.
+ *         x          ==>    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
@@ -53,61 +37,18 @@ import ast.Trees._
  *  Option 2: Merge ElimByName with erasure, or have it run immediately before. This has not been
  *  tried yet.
  */
-class ElimByName extends MiniPhaseTransform with InfoTransformer { thisTransformer =>
+class ElimByName extends TransformByNameApply with InfoTransformer { thisTransformer =>
   import ast.tpd._
 
   override def phaseName: String = "elimByName"
 
   override def runsAfterGroupsOf = Set(classOf[Splitter])
-    // assumes idents and selects have symbols; interferes with splitter distribution
-    // that's why it's "after group".
-
-  /** 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))
-
-  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
-        val argFun = arg match {
-          case Apply(Select(qual, nme.apply), Nil)
-          if qual.tpe.derivesFrom(defn.FunctionClass(0)) && isPureExpr(qual) =>
-            qual
-          case _ =>
-            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, _ =>
-              atGroupEnd { implicit ctx: Context =>
-                arg.changeOwner(ctx.owner, meth)
-              }
-            )
-        }
-        ref(defn.dummyApply).appliedToType(argType).appliedTo(argFun)
-      case _ =>
-        arg
-    }
-
-    val mt @ MethodType(_) = tree.fun.tpe.widen
-    val args1 = tree.args.zipWithConserve(mt.paramInfos)(transformArg)
-    cpy.Apply(tree)(tree.fun, args1)
-  }
-
-  /** If denotation had an ExprType before, it now gets a function type */
-  private def exprBecomesFunction(symd: SymDenotation)(implicit ctx: Context) =
-    (symd is Param) || (symd is (ParamAccessor, butNot = Method))
+    // 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) = {
-    val origDenot = originalDenotation(ftree)
-    if (exprBecomesFunction(origDenot) && (origDenot.info.isInstanceOf[ExprType]))
-      tree.select(defn.Function0_apply).appliedToNone
+  private def applyIfFunction(tree: Tree, ftree: Tree)(implicit ctx: Context) =
+    if (isByNameRef(ftree)) tree.select(defn.Function0_apply).appliedToNone
     else tree
-  }
 
   override def transformIdent(tree: Ident)(implicit ctx: Context, info: TransformerInfo): Tree =
     applyIfFunction(tree, tree)

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/ExplicitOuter.scala

@@ -46,7 +46,7 @@ class ExplicitOuter extends MiniPhaseTransform with InfoTransformer { thisTransf
   /** List of names of phases that should have finished their processing of all compilation units
     * before this phase starts
     */
-  override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[PatternMatcher])
+  override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[PatternMatcher], classOf[HoistSuperArgs])
 
   /** Add outer accessors if a class always needs an outer pointer */
   override def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context) = tp match {