Move the inlining of value class methods before Erasure

VCInline is split into two phases:
- VCInlineMethods (before Erasure) replaces value class method calls
  by calls to extension methods
- VCElideAllocations (after Erasure) handles == and optimizing the
  unboxing of a boxed value class, as VCInline did before.

This should not affect anything currently, but in the future we will have
phases before Erasure that mangle names (like TypeSpecializer, see #630),
being able to put these phases after VCInlineMethods means that
VCInlineMethods does not need to know anything about how these phases
mangle names, this reduces the coupling between phases. The trade-off is
that VCInlineMethods needs to deal with type parameters and multiple
parameter lists whereas VCInline didn't.

3 files changed, 112 insertions, 25 deletions

diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala
index 827134e84..75b790861 100644
--- a/src/dotty/tools/dotc/Compiler.scala
+++ b/src/dotty/tools/dotc/Compiler.scala
@@ -52,7 +52,8 @@ class Compiler {
       List(new PatternMatcher,
            new ExplicitOuter,
            new Splitter),
-      List(new SeqLiterals,
+      List(new VCInlineMethods,
+           new SeqLiterals,
            new InterceptedMethods,
            new Literalize,
            new Getters,
@@ -62,7 +63,7 @@ class Compiler {
            new ResolveSuper),
       List(new Erasure),
       List(new ElimErasedValueType,
-           new VCInline,
+           new VCElideAllocations,
            new Mixin,
            new LazyVals,
            new Memoize,
diff --git a/src/dotty/tools/dotc/transform/VCInline.scala b/src/dotty/tools/dotc/transform/VCElideAllocations.scala
index e7b16f59e..1582158ac 100644
--- a/src/dotty/tools/dotc/transform/VCInline.scala
+++ b/src/dotty/tools/dotc/transform/VCElideAllocations.scala
@@ -7,20 +7,18 @@ import Contexts._, Trees._, StdNames._, Symbols._
 import DenotTransformers._, TreeTransforms._, Phases.Phase
 import ExtensionMethods._, TreeExtractors._, ValueClasses._
 
-/** This phase inlines calls to methods and fields of value classes.
+/** This phase elides unnecessary value class allocations
  *
  *  For a value class V defined as:
- *    case class V(val underlying: U) extends AnyVal
- *  We replace method calls by calls to the corresponding extension method:
- *    v.foo(args) => V.foo$extension(v.underlying(), args)
- *  And we avoid unnecessary allocations:
+ *    class V(val underlying: U) extends AnyVal
+ *  we avoid unnecessary allocations:
  *     new V(u1) == new V(u2) => u1 == u2
  *    (new V(u)).underlying() => u
  */
-class VCInline extends MiniPhaseTransform with IdentityDenotTransformer {
+class VCElideAllocations extends MiniPhaseTransform with IdentityDenotTransformer {
   import tpd._
 
-  override def phaseName: String = "vcInline"
+  override def phaseName: String = "vcElideAllocations"
 
   override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[ElimErasedValueType])
 
@@ -37,22 +35,6 @@ class VCInline extends MiniPhaseTransform with IdentityDenotTransformer {
       case ValueClassUnbox(NewWithArgs(_, List(u))) =>
         u
 
-      // (new V(u)).foo(args) => V.foo$extension(u, args)
-      //          v.foo(args) => V.foo$extension(v.underlying(), args)
-      case Apply(sel @ Select(receiver, _), args) =>
-        val classMeth = sel.symbol
-        if (isMethodWithExtension(classMeth)) {
-          val classSym = receiver.tpe.widenDealias.typeSymbol.asClass
-          val unboxedReceiver = receiver match {
-            case NewWithArgs(_, List(u)) =>
-              u
-            case _ =>
-              receiver.select(valueClassUnbox(classSym)).appliedToNone
-          }
-          val extensionMeth = extensionMethod(classMeth)
-          ref(extensionMeth).appliedToArgs(unboxedReceiver :: args)
-        } else tree
-
       case _ =>
         tree
     }
diff --git a/src/dotty/tools/dotc/transform/VCInlineMethods.scala b/src/dotty/tools/dotc/transform/VCInlineMethods.scala
new file mode 100644
index 000000000..1c2b015a1
--- /dev/null
+++ b/src/dotty/tools/dotc/transform/VCInlineMethods.scala
@@ -0,0 +1,104 @@
+package dotty.tools.dotc
+package transform
+
+import ast.{Trees, tpd}
+import core._, core.Decorators._
+import Contexts._, Trees._, Types._
+import DenotTransformers._, TreeTransforms._, Phases.Phase
+import ExtensionMethods._, ValueClasses._
+
+import collection.mutable.ListBuffer
+
+/** This phase inlines calls to methods of value classes.
+ *
+ *  A value class V after [[ExtensionMethods]] will look like:
+ *    class V[A, B, ...](val underlying: U) extends AnyVal {
+ *      def foo[T, S, ...](arg1: A1, arg2: A2, ...) =
+ *        V.foo$extension[T, S, ..., A, B, ...](this)(arg1, arg2, ...)
+ *
+ *       ...
+ *    }
+ *
+ *  Let e have type V, if e is a stable prefix or if V does not have any class
+ *  type parameter, then we can rewrite:
+ *    e.foo[X, Y, ...](args)
+ *  as:
+ *    V.foo$extension[X, Y, ..., e.A, e.B, ...](e)(args)
+ *  Otherwise, we need to evaluate e first:
+ *    {
+ *      val ev = e
+ *      V.foo$extension[X, Y, ..., ev.A, ev.B, ...](ev)(args)
+ *    }
+ *
+ *  This phase needs to be placed after phases which may introduce calls to
+ *  value class methods (like [[PatternMatcher]]). This phase uses name mangling
+ *  to find the correct extension method corresponding to a value class method
+ *  (see [[ExtensionMethods.extensionMethod]]), therefore we choose to place it
+ *  before phases which may perform their own name mangling on value class
+ *  methods (like [[TypeSpecializer]]), this way [[VCInlineMethods]] does not
+ *  need to have any knowledge of the name mangling done by other phases.
+ */
+class VCInlineMethods extends MiniPhaseTransform with IdentityDenotTransformer {
+  import tpd._
+
+  override def phaseName: String = "vcInlineMethods"
+
+  override def runsAfter: Set[Class[_ <: Phase]] =
+    Set(classOf[ExtensionMethods], classOf[PatternMatcher])
+
+  /** Replace a value class method call by a call to the corresponding extension method.
+   *
+   *  @param tree   The tree corresponding to the method call
+   *  @param mtArgs Type arguments for the method call not present in `tree`
+   *  @param mArgss Arguments for the method call not present in `tree`
+   *  @return       A tree for the extension method call
+   */
+  private def rewire(tree: Tree, mtArgs: List[Tree] = Nil, mArgss: List[List[Tree]] = Nil)
+    (implicit ctx: Context): Tree =
+    tree match {
+      case Apply(qual, mArgs) =>
+        rewire(qual, mtArgs, mArgs :: mArgss)
+      case TypeApply(qual, mtArgs2) =>
+        assert(mtArgs == Nil)
+        rewire(qual, mtArgs2, mArgss)
+      case sel @ Select(qual, _) =>
+        val origMeth = sel.symbol
+        val ctParams = origMeth.enclosingClass.typeParams
+        val extensionMeth = extensionMethod(origMeth)
+
+        if (!ctParams.isEmpty) {
+          evalOnce(qual) { ev =>
+            val ctArgs = ctParams map (ev.select(_))
+            ref(extensionMeth)
+              .appliedToTypeTrees(mtArgs ++ ctArgs)
+              .appliedTo(ev)
+              .appliedToArgss(mArgss)
+          }
+        } else {
+          ref(extensionMeth)
+            .appliedToTypeTrees(mtArgs)
+            .appliedTo(qual)
+            .appliedToArgss(mArgss)
+        }
+    }
+
+  /** If this tree corresponds to a fully-applied value class method call, replace it
+   *  by a call to the corresponding extension method, otherwise return it as is.
+   */
+  private def rewireIfNeeded(tree: Tree)(implicit ctx: Context) = tree.tpe.widen match {
+    case tp: MethodOrPoly =>
+      tree // The rewiring will be handled by a fully-applied parent node
+    case _ =>
+      if (isMethodWithExtension(tree.symbol))
+        rewire(tree)
+      else
+        tree
+  }
+
+  override def transformSelect(tree: Select)(implicit ctx: Context, info: TransformerInfo): Tree =
+    rewireIfNeeded(tree)
+  override def transformTypeApply(tree: TypeApply)(implicit ctx: Context, info: TransformerInfo): Tree =
+    rewireIfNeeded(tree)
+  override def transformApply(tree: Apply)(implicit ctx: Context, info: TransformerInfo): Tree =
+    rewireIfNeeded(tree)
+}