Merge pull request scala#1568 from retronym/ticket/6611

SI-6611 Tighten up an unsafe array optimization

Author: paulp

src/compiler/scala/tools/nsc/transform/CleanUp.scala

@@ -15,6 +15,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
   import global._
   import definitions._
   import CODE._
+  import treeInfo.StripCast
 
   /** the following two members override abstract members in Transform */
   val phaseName: String = "cleanup"
@@ -618,14 +619,16 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
         }
         transformApply
 
-      // This transform replaces Array(Predef.wrapArray(Array(...)), <tag>)
-      // with just Array(...)
-      case Apply(appMeth, List(Apply(wrapRefArrayMeth, List(array)), _))
-      if (wrapRefArrayMeth.symbol == Predef_wrapRefArray &&
-          appMeth.symbol == ArrayModule_overloadedApply.suchThat {
-            _.tpe.resultType.dealias.typeSymbol == ObjectClass
-          }) =>
-        super.transform(array)
+      // Replaces `Array(Predef.wrapArray(ArrayValue(...).$asInstanceOf[...]), <tag>)`
+      // with just `ArrayValue(...).$asInstanceOf[...]`
+      //
+      // See SI-6611; we must *only* do this for literal vararg arrays.
+      case Apply(appMeth, List(Apply(wrapRefArrayMeth, List(arg @ StripCast(ArrayValue(_, _)))), _))
+      if wrapRefArrayMeth.symbol == Predef_wrapRefArray && appMeth.symbol == ArrayModule_genericApply =>
+        super.transform(arg)
+      case Apply(appMeth, List(elem0, Apply(wrapArrayMeth, List(rest @ ArrayValue(elemtpt, _)))))
+      if wrapArrayMeth.symbol == Predef_wrapArray(elemtpt.tpe) && appMeth.symbol == ArrayModule_apply(elemtpt.tpe) =>
+        super.transform(rest.copy(elems = elem0 :: rest.elems))
 
       case _ =>
         super.transform(tree)

src/library/scala/Array.scala

@@ -115,6 +115,8 @@ object Array extends FallbackArrayBuilding {
    *  @param xs the elements to put in the array
    *  @return an array containing all elements from xs.
    */
+  // Subject to a compiler optimization in Cleanup.
+  // Array(e0, ..., en) is translated to { val a = new Array(3); a(i) = ei; a }
   def apply[T: ClassTag](xs: T*): Array[T] = {
     val array = new Array[T](xs.length)
     var i = 0
@@ -123,6 +125,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Boolean` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Boolean, xs: Boolean*): Array[Boolean] = {
     val array = new Array[Boolean](xs.length + 1)
     array(0) = x
@@ -132,6 +135,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Byte` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Byte, xs: Byte*): Array[Byte] = {
     val array = new Array[Byte](xs.length + 1)
     array(0) = x
@@ -141,6 +145,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Short` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Short, xs: Short*): Array[Short] = {
     val array = new Array[Short](xs.length + 1)
     array(0) = x
@@ -150,6 +155,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Char` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Char, xs: Char*): Array[Char] = {
     val array = new Array[Char](xs.length + 1)
     array(0) = x
@@ -159,6 +165,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Int` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Int, xs: Int*): Array[Int] = {
     val array = new Array[Int](xs.length + 1)
     array(0) = x
@@ -168,6 +175,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Long` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Long, xs: Long*): Array[Long] = {
     val array = new Array[Long](xs.length + 1)
     array(0) = x
@@ -177,6 +185,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Float` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Float, xs: Float*): Array[Float] = {
     val array = new Array[Float](xs.length + 1)
     array(0) = x
@@ -186,6 +195,7 @@ object Array extends FallbackArrayBuilding {
   }
 
   /** Creates an array of `Double` objects */
+  // Subject to a compiler optimization in Cleanup, see above.
   def apply(x: Double, xs: Double*): Array[Double] = {
     val array = new Array[Double](xs.length + 1)
     array(0) = x

src/reflect/scala/reflect/internal/Definitions.scala

@@ -340,12 +340,13 @@ trait Definitions extends api.StandardDefinitions {
     lazy val PredefModule      = requiredModule[scala.Predef.type]
     lazy val PredefModuleClass = PredefModule.moduleClass
 
-      def Predef_classOf      = getMemberMethod(PredefModule, nme.classOf)
-      def Predef_identity     = getMemberMethod(PredefModule, nme.identity)
-      def Predef_conforms     = getMemberMethod(PredefModule, nme.conforms)
-      def Predef_wrapRefArray = getMemberMethod(PredefModule, nme.wrapRefArray)
-      def Predef_???          = getMemberMethod(PredefModule, nme.???)
-      def Predef_implicitly   = getMemberMethod(PredefModule, nme.implicitly)
+      def Predef_classOf             = getMemberMethod(PredefModule, nme.classOf)
+      def Predef_identity            = getMemberMethod(PredefModule, nme.identity)
+      def Predef_conforms            = getMemberMethod(PredefModule, nme.conforms)
+    def Predef_wrapRefArray        = getMemberMethod(PredefModule, nme.wrapRefArray)
+    def Predef_wrapArray(tp: Type) = getMemberMethod(PredefModule, wrapArrayMethodName(tp))
+    def Predef_???                 = getMemberMethod(PredefModule, nme.???)
+    def Predef_implicitly          = getMemberMethod(PredefModule, nme.implicitly)
 
     /** Is `sym` a member of Predef with the given name?
      *  Note: DON't replace this by sym == Predef_conforms/etc, as Predef_conforms is a `def`
@@ -470,6 +471,8 @@ trait Definitions extends api.StandardDefinitions {
     // arrays and their members
     lazy val ArrayModule                   = requiredModule[scala.Array.type]
       lazy val ArrayModule_overloadedApply = getMemberMethod(ArrayModule, nme.apply)
+           def ArrayModule_genericApply    = ArrayModule_overloadedApply.suchThat(_.paramss.flatten.last.tpe.typeSymbol == ClassTagClass) // [T: ClassTag](xs: T*): Array[T]
+           def ArrayModule_apply(tp: Type) = ArrayModule_overloadedApply.suchThat(_.tpe.resultType =:= arrayType(tp)) // (p1: AnyVal1, ps: AnyVal1*): Array[AnyVal1]
     lazy val ArrayClass                    = getRequiredClass("scala.Array") // requiredClass[scala.Array[_]]
       lazy val Array_apply                 = getMemberMethod(ArrayClass, nme.apply)
       lazy val Array_update                = getMemberMethod(ArrayClass, nme.update)

src/reflect/scala/reflect/internal/TreeInfo.scala

@@ -265,6 +265,20 @@ abstract class TreeInfo {
       tree
   }
 
+  /** Strips layers of `.asInstanceOf[T]` / `_.$asInstanceOf[T]()` from an expression */
+  def stripCast(tree: Tree): Tree = tree match {
+    case TypeApply(sel @ Select(inner, _), _) if isCastSymbol(sel.symbol) =>
+      stripCast(inner)
+    case Apply(TypeApply(sel @ Select(inner, _), _), Nil) if isCastSymbol(sel.symbol) =>
+      stripCast(inner)
+    case t =>
+      t
+  }
+
+  object StripCast {
+    def unapply(tree: Tree): Some[Tree] = Some(stripCast(tree))
+  }
+
   /** Is tree a self or super constructor call? */
   def isSelfOrSuperConstrCall(tree: Tree) = {
     // stripNamedApply for SI-3584: adaptToImplicitMethod in Typers creates a special context

test/files/instrumented/t6611.check

@@ -0,0 +1 @@
+Method call statistics:

test/files/instrumented/t6611.scala

@@ -0,0 +1,35 @@
+import scala.tools.partest.instrumented.Instrumentation._
+
+object Test {
+  def main(args: Array[String]) {
+    startProfiling()
+
+    // tests optimization in Cleanup for varargs reference arrays
+    Array("")
+
+
+    Array(true)
+    Array(true, false)
+    Array(1: Byte)
+    Array(1: Byte, 2: Byte)
+    Array(1: Short)
+    Array(1: Short, 2: Short)
+    Array(1)
+    Array(1, 2)
+    Array(1L)
+    Array(1L, 2L)
+    Array(1d)
+    Array(1d, 2d)
+    Array(1f)
+    Array(1f, 2f)
+
+    /* Not currently optimized:
+    Array[Int](1, 2) etc
+    Array(())
+    Array((), ())
+    */
+
+    stopProfiling()
+    printStatistics()
+  }
+}

test/files/run/t6611.scala

@@ -0,0 +1,61 @@
+object Test extends App {
+  locally {
+    val a = Array("1")
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array("1": Object)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(true)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(1: Short)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(1: Byte)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(1)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(1L)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(1f)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(1d)
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+
+  locally {
+    val a = Array(())
+    val a2 = Array(a: _*)
+    assert(a ne a2)
+  }
+}