From 3393616658fbd9946a9cc0520be42907b756964f Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 28 Jan 2021 16:16:11 +1000 Subject: [PATCH 1/2] Fix regression in specialization of genericWrapArray(Array(..)) The extension to the optimization in Cleanup of Array.apply assumed that the argument array was non-primitive. This broke compilation of spire (code minimized to test/files/run/sd760a.scala) I also found a path to the same bug without specialization, by using a generic type parmater bounded by a primitive. This is in test/files/run/sd760b.scala The fix here is straight forward -- disable this optimization when the argument array is primitive. --- src/compiler/scala/tools/nsc/transform/CleanUp.scala | 2 +- .../run/array-cleanup-optimation-specialized.scala | 12 ++++++++++++ test/files/run/sd760a.scala | 12 ++++++++++++ test/files/run/sd760b.scala | 11 +++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 test/files/run/array-cleanup-optimation-specialized.scala create mode 100644 test/files/run/sd760a.scala create mode 100644 test/files/run/sd760b.scala diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala index 7689cc652923..017446d4708a 100644 --- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala +++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala @@ -488,7 +488,7 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL { // See scala/bug#6611; we must *only* do this for literal vararg arrays. case Apply(appMeth, Apply(wrapRefArrayMeth, (arg @ StripCast(ArrayValue(elemtpt, elems))) :: Nil) :: classTagEvidence :: Nil) if (wrapRefArrayMeth.symbol == currentRun.runDefinitions.Predef_genericWrapRefArray || wrapRefArrayMeth.symbol == currentRun.runDefinitions.Predef_wrapRefArray) && appMeth.symbol == ArrayModule_genericApply && - !elemtpt.tpe.typeSymbol.isBottomClass => + !elemtpt.tpe.typeSymbol.isBottomClass && !elemtpt.tpe.typeSymbol.isPrimitiveValueClass /* can happen via specialization.*/ => classTagEvidence.attachments.get[analyzer.MacroExpansionAttachment] match { case Some(att) if att.expandee.symbol.name == nme.materializeClassTag && tree.isInstanceOf[ApplyToImplicitArgs] => super.transform(arg) diff --git a/test/files/run/array-cleanup-optimation-specialized.scala b/test/files/run/array-cleanup-optimation-specialized.scala new file mode 100644 index 000000000000..5db397752df3 --- /dev/null +++ b/test/files/run/array-cleanup-optimation-specialized.scala @@ -0,0 +1,12 @@ +import scala.reflect.ClassTag + +object Test { + def main(args: Array[String]): Unit = { + assert(apply[String]("") == classOf[Array[String]]) + assert(apply[Double](1d) == classOf[Array[Double]]) + } + + def apply[@specialized(Double) C: ClassTag](c: C): Class[_] = { + Array(c).getClass + } +} diff --git a/test/files/run/sd760a.scala b/test/files/run/sd760a.scala new file mode 100644 index 000000000000..5db397752df3 --- /dev/null +++ b/test/files/run/sd760a.scala @@ -0,0 +1,12 @@ +import scala.reflect.ClassTag + +object Test { + def main(args: Array[String]): Unit = { + assert(apply[String]("") == classOf[Array[String]]) + assert(apply[Double](1d) == classOf[Array[Double]]) + } + + def apply[@specialized(Double) C: ClassTag](c: C): Class[_] = { + Array(c).getClass + } +} diff --git a/test/files/run/sd760b.scala b/test/files/run/sd760b.scala new file mode 100644 index 000000000000..fae0e9cf8a6d --- /dev/null +++ b/test/files/run/sd760b.scala @@ -0,0 +1,11 @@ +import scala.reflect.ClassTag + +object Test { + def main(args: Array[String]): Unit = { + assert(apply[Double](1d) == classOf[Array[Double]]) + } + + def apply[D <: Double: ClassTag](d: D): Class[_] = { + Array.apply[D](d).getClass + } +} From c9692130aef49deec6db637bb5a740345d69c427 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 28 Jan 2021 16:50:05 +1000 Subject: [PATCH 2/2] Faster `Array[T](t)` for @specialialized T. Avoid boxing the elements in favour of bulk clone of the varargs array into the result array, when the runtime type of the class-tag-generated result array is the same as the varargs array. --- src/library/scala/Array.scala | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/library/scala/Array.scala b/src/library/scala/Array.scala index b41a0d9c1349..b2390a41804e 100644 --- a/src/library/scala/Array.scala +++ b/src/library/scala/Array.scala @@ -13,10 +13,11 @@ package scala import scala.collection.generic._ -import scala.collection.{ mutable, immutable } -import mutable.{ ArrayBuilder, ArraySeq } -import scala.reflect.ClassTag -import scala.runtime.ScalaRunTime.{ array_apply, array_update } +import scala.collection.{immutable, mutable} +import mutable.{ArrayBuilder, ArraySeq} +import scala.reflect.{ClassTag, classTag} +import scala.runtime.ScalaRunTime +import scala.runtime.ScalaRunTime.{array_apply, array_update} /** Contains a fallback builder for arrays when the element type * does not have a class tag. In that case a generic array is built. @@ -194,10 +195,20 @@ object Array extends FallbackArrayBuilding { // 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 - for (x <- xs.iterator) { array(i) = x; i += 1 } - array + val len = xs.length + xs match { + case wa: mutable.WrappedArray[_] if wa.elemTag == classTag[T] => + // We get here in test/files/run/sd760a.scala, `Array[T](t)` for + // a specialized type parameter `T`. While we still pay for two + // copies of the array it is better than before when we also boxed + // each element when populating the result. + ScalaRunTime.array_clone(wa.array).asInstanceOf[Array[T]] + case _ => + val array = new Array[T](len) + var i = 0 + for (x <- xs.iterator) { array(i) = x; i += 1 } + array + } } /** Creates an array of `Boolean` objects */