Merge pull request #3205 from dotty-staging/fix-#3130

Fix #3130: Various fixes to inline

Author: odersky

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

@@ -75,6 +75,7 @@ class Compiler {
            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 parameter references
+           new ElimOuterSelect,     // Expand outer selections
            new AugmentScala2Traits, // Expand traits defined in Scala 2.x to simulate old-style rewritings
            new ResolveSuper,        // Implement super accessors and add forwarders to trait methods
            new Simplify,            // Perform local optimizations, simplified versions of what linker does.

compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala

@@ -80,7 +80,7 @@ final class TreeTypeMap(
       val tmap = withMappedSyms(localSyms(impl :: self :: Nil))
       cpy.Template(impl)(
           constr = tmap.transformSub(constr),
-          parents = parents mapconserve transform,
+          parents = parents.mapconserve(transform),
           self = tmap.transformSub(self),
           body = impl.body mapconserve
             (tmap.transform(_)(ctx.withOwner(mapOwner(impl.symbol.owner))))

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -826,7 +826,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
      *  @param tp      The type of the destination of the outer path.
      */
     def outerSelect(levels: Int, tp: Type)(implicit ctx: Context): Tree =
-      untpd.Select(tree, OuterSelectName(EmptyTermName, levels)).withType(tp)
+      untpd.Select(tree, OuterSelectName(EmptyTermName, levels)).withType(SkolemType(tp))
 
     // --- Higher order traversal methods -------------------------------
 

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

@@ -3948,6 +3948,17 @@ object Types {
     protected def mapClassInfo(tp: ClassInfo): Type =
       derivedClassInfo(tp, this(tp.prefix))
 
+    /** A version of mapClassInfo which also maps parents and self type */
+    protected def mapFullClassInfo(tp: ClassInfo) =
+      tp.derivedClassInfo(
+        prefix = this(tp.prefix),
+        classParents = tp.classParents.mapConserve(this),
+        selfInfo = tp.selfInfo match {
+          case tp: Type => this(tp)
+          case sym => sym
+        }
+      )
+
     def andThen(f: Type => Type): TypeMap = new TypeMap {
       override def stopAtStatic = thisMap.stopAtStatic
       def apply(tp: Type) = f(thisMap(tp))

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

@@ -0,0 +1,34 @@
+package dotty.tools.dotc
+package transform
+
+import core._
+import TreeTransforms.{MiniPhaseTransform, TransformerInfo}
+import Contexts.Context
+import Types._
+import Decorators._
+import NameKinds.OuterSelectName
+import ast.Trees._
+
+/** This phase rewrites outer selects `E.n_<outer>` which were introduced by
+ *  inlining to outer paths.
+ */
+class ElimOuterSelect extends MiniPhaseTransform { thisTransform =>
+  import ast.tpd._
+
+  override def phaseName: String = "elimOuterSelect"
+
+  override def runsAfterGroupsOf = Set(classOf[ExplicitOuter])
+    // ExplicitOuter needs to have run to completion before so that all classes
+    // that need an outer accessor have one.
+
+  /** Convert a selection of the form `qual.n_<outer>` to an outer path from `qual` of
+   *  length `n`.
+   */
+  override def transformSelect(tree: Select)(implicit ctx: Context, info: TransformerInfo) =
+    tree.name match {
+      case OuterSelectName(_, nhops) =>
+        val SkolemType(tp) = tree.tpe
+        ExplicitOuter.outer.path(start = tree.qualifier, count = nhops).ensureConforms(tp)
+      case _ => tree
+    }
+}

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

@@ -11,7 +11,6 @@ import core.Decorators._
 import core.StdNames.nme
 import core.Names._
 import core.NameOps._
-import core.NameKinds.OuterSelectName
 import ast.Trees._
 import SymUtils._
 import dotty.tools.dotc.ast.tpd
@@ -62,14 +61,6 @@ class ExplicitOuter extends MiniPhaseTransform with InfoTransformer { thisTransf
 
   override def mayChange(sym: Symbol)(implicit ctx: Context): Boolean = sym.isClass
 
-  /** Convert a selection of the form `qual.C_<OUTER>` to an outer path from `qual` to `C` */
-  override def transformSelect(tree: Select)(implicit ctx: Context, info: TransformerInfo) =
-    tree.name match {
-      case OuterSelectName(_, nhops) =>
-        outer.path(start = tree.qualifier, count = nhops).ensureConforms(tree.tpe)
-      case _ => tree
-    }
-
   /** First, add outer accessors if a class does not have them yet and it references an outer this.
    *  If the class has outer accessors, implement them.
    *  Furthermore, if a parent trait might have an outer accessor,

compiler/src/dotty/tools/dotc/typer/Inliner.scala

@@ -39,16 +39,21 @@ object Inliner {
    */
   private def makeInlineable(tree: Tree)(implicit ctx: Context) = {
 
+    val inlineMethod = ctx.owner
+
     /** A tree map which inserts accessors for all non-public term members accessed
-     *  from inlined code. Accesors are collected in the `accessors` buffer.
+     *  from inlined code. Accessors are collected in the `accessors` buffer.
      */
     object addAccessors extends TreeMap {
-      val inlineMethod = ctx.owner
       val accessors = new mutable.ListBuffer[MemberDef]
 
-      /** A definition needs an accessor if it is private, protected, or qualified private */
+      /** A definition needs an accessor if it is private, protected, or qualified private
+       *  and it is not part of the tree that gets inlined. The latter test is implemented
+       *  by excluding all symbols properly contained in the inlined method.
+       */
       def needsAccessor(sym: Symbol)(implicit ctx: Context) =
-        sym.is(AccessFlags) || sym.privateWithin.exists
+        (sym.is(AccessFlags) || sym.privateWithin.exists) &&
+        !sym.owner.isContainedIn(inlineMethod)
 
       /** The name of the next accessor to be generated */
       def accessorName(implicit ctx: Context) = InlineAccessorName.fresh(inlineMethod.name.asTermName)
@@ -116,7 +121,8 @@ object Inliner {
 
             // The types that are local to the inlined method, and that therefore have
             // to be abstracted out in the accessor, which is external to the inlined method
-            val localRefs = qualType.namedPartsWith(_.symbol.isContainedIn(inlineMethod)).toList
+            val localRefs = qualType.namedPartsWith(ref =>
+              ref.isType && ref.symbol.isContainedIn(inlineMethod)).toList
 
             // Abstract accessed type over local refs
             def abstractQualType(mtpe: Type): Type =
@@ -175,8 +181,14 @@ object Inliner {
       }
     }
 
-    val tree1 = addAccessors.transform(tree)
-    flatTree(tree1 :: addAccessors.accessors.toList)
+    if (inlineMethod.owner.isTerm)
+      // Inline methods in local scopes can only be called in the scope they are defined,
+      // so no accessors are needed for them.
+      tree
+    else {
+      val tree1 = addAccessors.transform(tree)
+      flatTree(tree1 :: addAccessors.accessors.toList)
+    }
   }
 
   /** Register inline info for given inline method `sym`.
@@ -359,13 +371,15 @@ class Inliner(call: tpd.Tree, rhs: tpd.Tree)(implicit ctx: Context) {
 
   private def canElideThis(tpe: ThisType): Boolean =
     prefix.tpe == tpe && ctx.owner.isContainedIn(tpe.cls) ||
+    tpe.cls.isContainedIn(meth) ||
     tpe.cls.is(Package)
 
   /** Populate `thisProxy` and `paramProxy` as follows:
    *
    *  1a. If given type refers to a static this, thisProxy binds it to corresponding global reference,
-   *  1b. If given type refers to an instance this, create a proxy symbol and bind the thistype to
-   *      refer to the proxy. The proxy is not yet entered in `bindingsBuf` that will come later.
+   *  1b. If given type refers to an instance this to a class that is not contained in the
+   *      inlined method, create a proxy symbol and bind the thistype to refer to the proxy.
+   *      The proxy is not yet entered in `bindingsBuf`; that will come later.
    *  2.  If given type refers to a parameter, make `paramProxy` refer to the entry stored
    *      in `paramNames` under the parameter's name. This roundabout way to bind parameter
    *      references to proxies is done because  we not known a priori what the parameter
@@ -421,11 +435,12 @@ class Inliner(call: tpd.Tree, rhs: tpd.Tree)(implicit ctx: Context) {
       val rhs =
         if (lastSelf.exists)
           ref(lastSelf).outerSelect(lastLevel - level, selfSym.info)
-        else if (rhsClsSym.is(Module))
+        else if (rhsClsSym.is(Module) && rhsClsSym.isStatic)
           ref(rhsClsSym.sourceModule)
         else
           prefix
       bindingsBuf += ValDef(selfSym.asTerm, rhs)
+      inlining.println(i"proxy at $level: $selfSym = ${bindingsBuf.last}")
       lastSelf = selfSym
       lastLevel = level
     }
@@ -439,6 +454,7 @@ class Inliner(call: tpd.Tree, rhs: tpd.Tree)(implicit ctx: Context) {
         case t: SingletonType => paramProxy.getOrElse(t, mapOver(t))
         case t => mapOver(t)
       }
+      override def mapClassInfo(tp: ClassInfo) = mapFullClassInfo(tp)
     }
 
     // The tree map to apply to the inlined tree. This maps references to this-types

tests/pos/i3082.scala

@@ -0,0 +1,7 @@
+object Test {
+  private def foo(arg1: Int): Int = {
+    inline def bar: Int = foo(0)
+    if (arg1 == 0) 0 else bar
+  }
+  assert(foo(11) == 0)
+}

tests/pos/i3129.scala

@@ -0,0 +1,22 @@
+object companions2 {
+  inline def foo() = {
+    class C {
+      println(C.p)
+    }
+
+    object C {
+      private val p = 1
+    }
+  }
+}
+
+class A {
+  val b = new B
+
+  class B {
+    private def getAncestor2(p: A): A = p
+    private inline def getAncestor(p: A): A = {
+      p.b.getAncestor(p)
+    }
+  }
+}

tests/pos/i3130a.scala

@@ -0,0 +1,10 @@
+object O {
+  new D(2).DD.apply()
+}
+
+ class D(val x: Int) {
+  class DD()
+  object DD {
+   inline def apply() = x // new DD()
+  }
+}

tests/pos/i3130b.scala

@@ -0,0 +1,15 @@
+class Outer {
+  trait F { def f(): Int }
+  inline def inner: F = {
+    class InnerClass(x: Int) extends F {
+      def this() = this(3)
+      def f() = x
+    }
+    new InnerClass(3)
+  }
+}
+
+object Test extends App {
+  val o = new Outer
+  assert(o.inner.f() == 3)
+}

tests/pos/i3130c.scala

@@ -0,0 +1,23 @@
+trait Test {
+  trait Global {
+    type Tree
+    def get: Tree
+  }
+
+  trait TreeBuilder {
+    val global: Global
+    inline def set(tree: global.Tree) = {}
+  }
+
+  val nsc: Global
+
+  trait FileImpl {
+    object treeBuilder extends TreeBuilder {
+      val global: nsc.type = nsc
+    }
+    treeBuilder.set(nsc.get)
+  }
+  def file: FileImpl
+
+  file.treeBuilder.set(nsc.get)
+}

tests/pos/i3130d.scala

@@ -0,0 +1,9 @@
+class D(x: Int) {
+  class DD {
+    inline def apply() = new DD()
+  }
+  val inner = new DD
+}
+object Test {
+  new D(2).inner.apply()
+}