Boxing cleanup: erasure, post-erasure, value classes.

Introduces extractors for value class trees. Puts them to
work to make the value class tree manipulations believable.
Eliminated some boxing code in erasure which had been
marked "maybe subsumed by posterasure?" after deciding
that it had been subsumed by posterasure.

Added some same-bytecode tests involving value class boxing
(actually the lack thereof.)

6 files changed, 138 insertions, 99 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeInfo.scala b/src/compiler/scala/tools/nsc/ast/TreeInfo.scala
index f53f99a279..6a0f4407fc 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeInfo.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeInfo.scala
@@ -14,6 +14,65 @@ package ast
 abstract class TreeInfo extends scala.reflect.internal.TreeInfo {
   val global: Global
   import global._
+  import definitions._
+
+  // arg1.op(arg2) returns (arg1, op.symbol, arg2)
+  object BinaryOp {
+    def unapply(t: Tree): Option[(Tree, Symbol, Tree)] = t match {
+      case Apply(sel @ Select(arg1, _), arg2 :: Nil) => Some((arg1, sel.symbol, arg2))
+      case _                                         => None
+    }
+  }
+  // recv.op[T1, ...] returns (recv, op.symbol, type argument types)
+  object TypeApplyOp {
+    def unapply(t: Tree): Option[(Tree, Symbol, List[Type])] = t match {
+      case TypeApply(sel @ Select(recv, _), targs) => Some((recv, sel.symbol, targs map (_.tpe)))
+      case _                                       => None
+    }
+  }
+
+  // x.asInstanceOf[T] returns (x, typeOf[T])
+  object AsInstanceOf {
+    def unapply(t: Tree): Option[(Tree, Type)] = t match {
+      case Apply(TypeApplyOp(recv, Object_asInstanceOf, tpe :: Nil), Nil) => Some((recv, tpe))
+      case _                                                              => None
+    }
+  }
+
+  // Extractors for value classes.
+  object ValueClass {
+    def isValueClass(tpe: Type)                  = enteringErasure(tpe.typeSymbol.isDerivedValueClass)
+    def valueUnbox(tpe: Type)                    = enteringErasure(tpe.typeSymbol.derivedValueClassUnbox)
+
+    // B.unbox. Returns B.
+    object Unbox {
+      def unapply(t: Tree): Option[Tree] = t match {
+        case Apply(sel @ Select(ref, _), Nil) if valueUnbox(ref.tpe) == sel.symbol => Some(ref)
+        case _                                                                     => None
+      }
+    }
+    // new B(v). Returns B and v.
+    object Box {
+      def unapply(t: Tree): Option[(Tree, Type)] = t match {
+        case Apply(sel @ Select(New(tpt), nme.CONSTRUCTOR), v :: Nil) => Some((v, tpt.tpe.finalResultType))
+        case _                                                        => None
+      }
+    }
+    // (new B(v)).unbox. returns v.
+    object BoxAndUnbox {
+      def unapply(t: Tree): Option[Tree] = t match {
+        case Unbox(Box(v, tpe)) if isValueClass(tpe) => Some(v)
+        case _                                       => None
+      }
+    }
+    // new B(v1) op new B(v2) where op is == or !=. Returns v1, op, v2.
+    object BoxAndCompare {
+      def unapply(t: Tree): Option[(Tree, Symbol, Tree)] = t match {
+        case BinaryOp(Box(v1, tpe1), op @ (Object_== | Object_!=), Box(v2, tpe2)) if isValueClass(tpe1) && tpe1 =:= tpe2 => Some((v1, op, v2))
+        case _                                                                                                           => None
+      }
+    }
+  }
 
   /** Is tree legal as a member definition of an interface?
    */
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index f380b9d04f..8287c1f631 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -21,6 +21,7 @@ abstract class Erasure extends AddInterfaces
   import global._
   import definitions._
   import CODE._
+  import treeInfo._
 
   val phaseName: String = "erasure"
 
@@ -357,41 +358,10 @@ abstract class Erasure extends AddInterfaces
 
   override def newTyper(context: Context) = new Eraser(context)
 
-  private def safeToRemoveUnbox(cls: Symbol): Boolean =
-    (cls == definitions.NullClass) || isBoxedValueClass(cls)
-
-  /** An extractor object for unboxed expressions (maybe subsumed by posterasure?) */
-  object Unboxed {
-    def unapply(tree: Tree): Option[Tree] = tree match {
-      case Apply(fn, List(arg)) if isUnbox(fn.symbol) && safeToRemoveUnbox(arg.tpe.typeSymbol) =>
-        Some(arg)
-      case Apply(
-        TypeApply(
-          cast @ Select(
-            Apply(
-              sel @ Select(arg, acc),
-              List()),
-            asinstanceof),
-          List(tpt)),
-        List())
-      if cast.symbol == Object_asInstanceOf &&
-        tpt.tpe.typeSymbol.isDerivedValueClass &&
-        sel.symbol == tpt.tpe.typeSymbol.derivedValueClassUnbox =>
-        Some(arg)
-      case _ =>
-        None
-    }
-  }
-
-  /** An extractor object for boxed expressions (maybe subsumed by posterasure?) */
-  object Boxed {
-    def unapply(tree: Tree): Option[Tree] = tree match {
-      case Apply(Select(New(tpt), nme.CONSTRUCTOR), List(arg)) if (tpt.tpe.typeSymbol.isDerivedValueClass) =>
-        Some(arg)
-      case LabelDef(name, params, Boxed(rhs)) =>
-        Some(treeCopy.LabelDef(tree, name, params, rhs) setType rhs.tpe)
-      case _ =>
-        None
+  private def isSafelyRemovableUnbox(fn: Tree, arg: Tree): Boolean = {
+    isUnbox(fn.symbol) && {
+      val cls = arg.tpe.typeSymbol
+      (cls == definitions.NullClass) || isBoxedValueClass(cls)
     }
   }
 
@@ -578,12 +548,7 @@ abstract class Erasure extends AddInterfaces
         val tree1 = tree.tpe match {
           case ErasedValueType(tref) =>
             val clazz = tref.sym
-            tree match {
-              case Unboxed(arg) if arg.tpe.typeSymbol == clazz =>
-                log("shortcircuiting unbox -> box "+arg); arg
-              case _ =>
-                New(clazz, cast(tree, underlyingOfValueClass(clazz)))
-            }
+            New(clazz, cast(tree, underlyingOfValueClass(clazz)))
           case _ =>
             tree.tpe.typeSymbol match {
           case UnitClass  =>
@@ -599,7 +564,7 @@ abstract class Erasure extends AddInterfaces
                *  This is important for specialization: calls to the super constructor should not box/unbox specialized
                *  fields (see TupleX). (ID)
                */
-              case Apply(boxFun, List(arg)) if isUnbox(tree.symbol) && safeToRemoveUnbox(arg.tpe.typeSymbol) =>
+              case Apply(boxFun, List(arg)) if isSafelyRemovableUnbox(tree, arg) =>
                 log(s"boxing an unbox: ${tree.symbol} -> ${arg.tpe}")
                 arg
               case _ =>
@@ -634,24 +599,18 @@ abstract class Erasure extends AddInterfaces
       case _ =>
         val tree1 = pt match {
           case ErasedValueType(tref) =>
-            tree match {
-              case Boxed(arg) if arg.tpe.isInstanceOf[ErasedValueType] =>
-                log("shortcircuiting box -> unbox "+arg)
-                arg
-              case _ =>
-                val clazz = tref.sym
-                log("not boxed: "+tree)
-                lazy val underlying = underlyingOfValueClass(clazz)
-                val tree0 =
-                  if (tree.tpe.typeSymbol == NullClass &&
-                      isPrimitiveValueClass(underlying.typeSymbol)) {
-                    // convert `null` directly to underlying type, as going
-                    // via the unboxed type would yield a NPE (see SI-5866)
-                    unbox1(tree, underlying)
-                  } else
-                    Apply(Select(adaptToType(tree, clazz.tpe), clazz.derivedValueClassUnbox), List())
-                cast(tree0, pt)
-            }
+            val clazz = tref.sym
+            log("not boxed: "+tree)
+            lazy val underlying = underlyingOfValueClass(clazz)
+            val tree0 =
+              if (tree.tpe.typeSymbol == NullClass &&
+                  isPrimitiveValueClass(underlying.typeSymbol)) {
+                // convert `null` directly to underlying type, as going
+                // via the unboxed type would yield a NPE (see SI-5866)
+                unbox1(tree, underlying)
+              } else
+                Apply(Select(adaptToType(tree, clazz.tpe), clazz.derivedValueClassUnbox), List())
+            cast(tree0, pt)
           case _ =>
             pt.typeSymbol match {
               case UnitClass  =>
diff --git a/src/compiler/scala/tools/nsc/transform/PostErasure.scala b/src/compiler/scala/tools/nsc/transform/PostErasure.scala
index a8dc47046b..2a86d711f1 100644
--- a/src/compiler/scala/tools/nsc/transform/PostErasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/PostErasure.scala
@@ -9,10 +9,10 @@ package transform
  *  performs peephole optimizations.
  */
 trait PostErasure extends InfoTransform with TypingTransformers {
-
   val global: Global
+
   import global._
-  import definitions._
+  import treeInfo._
 
   val phaseName: String = "posterasure"
 
@@ -21,51 +21,33 @@ trait PostErasure extends InfoTransform with TypingTransformers {
 
   object elimErasedValueType extends TypeMap {
     def apply(tp: Type) = tp match {
-      case ConstantType(Constant(tp: Type)) =>
-        ConstantType(Constant(apply(tp)))
-      case ErasedValueType(tref) =>
-        enteringPhase(currentRun.erasurePhase)(erasure.erasedValueClassArg(tref))
-      case _ => mapOver(tp)
+      case ConstantType(Constant(tp: Type)) => ConstantType(Constant(apply(tp)))
+      case ErasedValueType(tref)            => enteringErasure(erasure.erasedValueClassArg(tref))
+      case _                                => mapOver(tp)
     }
   }
 
   def transformInfo(sym: Symbol, tp: Type) = elimErasedValueType(tp)
 
   class PostErasureTransformer(unit: CompilationUnit) extends TypingTransformer(unit) {
+    override def transform(tree: Tree) = {
+      def finish(res: Tree) = logResult(s"Posterasure reduction\n  Old: $tree\n  New")(res)
+
+      /** We use the name of the operation being performed and not the symbol
+       *  itself because the symbol hails from the boxed class, and this transformation
+       *  exists to operate directly on the values. So we are for instance looking
+       *  up == on an lhs of type Int, whereas the symbol which has been passed in
+       *  is from java.lang.Integer.
+       */
+      def binop(lhs: Tree, op: Symbol, rhs: Tree) =
+        finish(localTyper typed (Apply(Select(lhs, op.name) setPos tree.pos, rhs :: Nil) setPos tree.pos))
 
-    override def transform(tree: Tree) =
       super.transform(tree) setType elimErasedValueType(tree.tpe) match {
-        case // new C(arg).underlying  ==>  arg
-          Apply(sel @ Select(
-            Apply(Select(New(tpt), nme.CONSTRUCTOR), List(arg)),
-            acc), List())
-        if enteringPhase(currentRun.erasurePhase) {
-          tpt.tpe.typeSymbol.isDerivedValueClass &&
-          sel.symbol == tpt.tpe.typeSymbol.derivedValueClassUnbox
-        } =>
-          if (settings.debug.value) log("Removing "+tree+" -> "+arg)
-          arg
-        case // new C(arg1) == new C(arg2)  ==>  arg1 == arg2
-          Apply(sel @ Select(
-            Apply(Select(New(tpt1), nme.CONSTRUCTOR), List(arg1)),
-            cmp),
-            List(Apply(Select(New(tpt2), nme.CONSTRUCTOR), List(arg2))))
-        if enteringPhase(currentRun.erasurePhase) {
-          tpt1.tpe.typeSymbol.isDerivedValueClass &&
-          (sel.symbol == Object_== || sel.symbol == Object_!=) &&
-          tpt2.tpe.typeSymbol == tpt1.tpe.typeSymbol
-        } =>
-          val result = Apply(Select(arg1, cmp) setPos sel.pos, List(arg2)) setPos tree.pos
-          log("shortcircuiting equality "+tree+" -> "+result)
-          localTyper.typed(result)
-
-        case // arg.asInstanceOf[T]  ==>  arg      if arg.tpe == T
-          Apply(TypeApply(cast @ Select(arg, asinstanceof), List(tpt)), List())
-        if cast.symbol == Object_asInstanceOf && arg.tpe =:= tpt.tpe => // !!! <:< ?
-          if (settings.debug.value) log("Shortening "+tree+" -> "+arg)
-          arg
-        case tree1 =>
-          tree1
+        case AsInstanceOf(v, tpe) if v.tpe <:< tpe => finish(v)          // x.asInstanceOf[X]       ==> x
+        case ValueClass.BoxAndUnbox(v)             => finish(v)          // (new B(v)).unbox        ==> v
+        case ValueClass.BoxAndCompare(v1, op, v2)  => binop(v1, op, v2)  // new B(v1) == new B(v2)  ==> v1 == v2
+        case tree                                  => tree
       }
+    }
   }
 }
diff --git a/test/files/jvm/value-class-boxing.check b/test/files/jvm/value-class-boxing.check
new file mode 100644
index 0000000000..20a9fe2ba8
--- /dev/null
+++ b/test/files/jvm/value-class-boxing.check
@@ -0,0 +1,7 @@
+a2 and a1: bytecode identical
+a3 and a1: bytecode identical
+a4 and a1: bytecode identical
+b2 and b1: bytecode identical
+b3 and b1: bytecode identical
+b4 and b1: bytecode identical
+b5 and b1: bytecode identical
diff --git a/test/files/jvm/value-class-boxing/Analyzed_1.scala b/test/files/jvm/value-class-boxing/Analyzed_1.scala
new file mode 100644
index 0000000000..dec8565351
--- /dev/null
+++ b/test/files/jvm/value-class-boxing/Analyzed_1.scala
@@ -0,0 +1,17 @@
+class Wrap(val x: Int) extends AnyVal {
+  def ***(other: Bip): Wrap = new Wrap(x * other.x)
+}
+class Bip(val x: Int) extends AnyVal
+
+class SameBytecode {
+  def a1(x: Int, y: Int): Int = x + y
+  def a2(x: Wrap, y: Wrap): Wrap = new Wrap(x.x + y.x)
+  def a3(x: Int, y: Wrap): Wrap = new Wrap(x + y.x)
+  def a4(x: Int, y: Wrap): Int = x + y.x
+
+  def b1(x: Wrap, y: Int): Int = (x *** new Bip(y)).x
+  def b2(x: Wrap, y: Bip): Wrap = x *** y
+  def b3(x: Wrap, y: Int): Wrap = x *** new Bip(y)
+  def b4(x: Wrap, y: Bip): Bip = new Bip((x *** y).x)
+  def b5(x: Wrap, y: Int): Bip = new Bip((x *** new Bip(y)).x)
+}
diff --git a/test/files/jvm/value-class-boxing/test.scala b/test/files/jvm/value-class-boxing/test.scala
new file mode 100644
index 0000000000..cf331832de
--- /dev/null
+++ b/test/files/jvm/value-class-boxing/test.scala
@@ -0,0 +1,15 @@
+import scala.tools.partest.BytecodeTest
+
+object Test extends BytecodeTest {
+  def show: Unit = {
+    val classNode = loadClassNode("SameBytecode")
+    List("a2", "a3", "a4") foreach { m =>
+      print(m + " and a1: ")
+      sameBytecode(getMethod(classNode, "a1"), getMethod(classNode, m))
+    }
+    List("b2", "b3", "b4", "b5") foreach { m =>
+      print(m + " and b1: ")
+      sameBytecode(getMethod(classNode, "b1"), getMethod(classNode, m))
+    }
+  }
+}