Skip to content

Commit feec230

Browse files
nicolasstuckinicolasstucki
committed
Fix #502: Optimize Array.apply([...]) to [...] 
1 parent dfa1a91 commit feec230

File tree

6 files changed

+147
-13
lines changed

6 files changed

+147
-13
lines changed

‎compiler/src/dotty/tools/dotc/core/Definitions.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,8 @@ class Definitions {
755755
@threadUnsafe lazy val ClassTagType: TypeRef = ctx.requiredClassRef("scala.reflect.ClassTag")
756756
def ClassTagClass(implicit ctx: Context): ClassSymbol = ClassTagType.symbol.asClass
757757
def ClassTagModule(implicit ctx: Context): Symbol = ClassTagClass.companionModule
758+
@threadUnsafe lazy val ClassTagModule_applyR: TermRef = ClassTagModule.requiredMethodRef("apply")
759+
def ClassTagModule_apply(implicit ctx: Context): Symbol = ClassTagModule_applyR.symbol
758760

759761
@threadUnsafe lazy val QuotedExprType: TypeRef = ctx.requiredClassRef("scala.quoted.Expr")
760762
def QuotedExprClass(implicit ctx: Context): ClassSymbol = QuotedExprType.symbol.asClass

‎compiler/src/dotty/tools/dotc/transform/ArrayConstructors.scala

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import scala.collection.immutable.::
1616

1717
/** This phase rewrites calls to array constructors to newArray method in Dotty.runtime.Arrays module.
1818
*
19-
* It assummes that generic arrays have already been handled by typer(see Applications.convertNewGenericArray).
20-
* Additionally it optimizes calls to scala.Array.ofDim functions by replacing them with calls to newArray with specific dimensions
19+
* It assumes that generic arrays have already been handled by typer(see Applications.convertNewGenericArray).
20+
* Additionally:
21+
* - it optimizes calls to scala.Array.ofDim functions by replacing them with calls to newArray with specific dimensions*
22+
* - it optimizes `scala.Array.apply([....])` and `scala.Array.apply(..., [....])` into `[...]`
2123
*/
2224
class ArrayConstructors extends MiniPhase {
2325
import ast.tpd._
@@ -31,17 +33,40 @@ class ArrayConstructors extends MiniPhase {
3133
if (tree.fun.symbol eq defn.ArrayConstructor) {
3234
val TypeApply(tycon, targ :: Nil) = tree.fun
3335
expand(targ.tpe, tree.args)
34-
} else if ((tree.fun.symbol.maybeOwner eq defn.ArrayModule) && (tree.fun.symbol.name eq nme.ofDim) && !tree.tpe.isInstanceOf[MethodicType]) {
35-
val Apply(Apply(TypeApply(_, List(tp)), _), _) = tree
36-
val cs = tp.tpe.widen.classSymbol
37-
tree.fun match {
38-
case Apply(TypeApply(t: Ident, targ), dims)
39-
if !TypeErasure.isGeneric(targ.head.tpe) && !ValueClasses.isDerivedValueClass(cs) =>
40-
expand(targ.head.tpe, dims)
41-
case Apply(TypeApply(t: Select, targ), dims)
42-
if !TypeErasure.isGeneric(targ.head.tpe) && !ValueClasses.isDerivedValueClass(cs) =>
43-
Block(t.qualifier :: Nil, expand(targ.head.tpe, dims))
44-
case _ => tree
36+
} else if (tree.fun.symbol.maybeOwner eq defn.ArrayModule) {
37+
if ((tree.fun.symbol.name eq nme.ofDim) && !tree.tpe.isInstanceOf[MethodicType]) {
38+
val Apply(Apply(TypeApply(_, List(tp)), _), _) = tree
39+
val cs = tp.tpe.widen.classSymbol
40+
tree.fun match {
41+
case Apply(TypeApply(t: Ident, targ), dims)
42+
if !TypeErasure.isGeneric(targ.head.tpe) && !ValueClasses.isDerivedValueClass(cs) =>
43+
expand(targ.head.tpe, dims)
44+
case Apply(TypeApply(t: Select, targ), dims)
45+
if !TypeErasure.isGeneric(targ.head.tpe) && !ValueClasses.isDerivedValueClass(cs) =>
46+
Block(t.qualifier :: Nil, expand(targ.head.tpe, dims))
47+
case _ => tree
48+
}
49+
} else {
50+
// Optimize `Array.apply([....])` and `Array.apply(..., [....])` into `[...]`
51+
52+
def isElidibleIsClassTag(ct: Tree): Boolean = {
53+
ct.symbol == defn.ClassTagModule_apply || // Elide `ClassTag.apply`
54+
isPureRef(ct) // Elide `ClassTag.{Byte, Boolean, ...}` or other pure references
55+
}
56+
57+
tree match {
58+
case Apply(Apply(appMeth, Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil) :: Nil), ct :: Nil)
59+
if appMeth.symbol.name == nme.apply && appMeth.symbol.owner == defn.ArrayModule && // Is `Array.apply`
60+
defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) && // Is `wrap{X, Ref}Array`
61+
isElidibleIsClassTag(ct) =>
62+
seqLit
63+
case Apply(appMeth, elem0 :: Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil) :: Nil)
64+
if appMeth.symbol.name == nme.apply && appMeth.symbol.owner == defn.ArrayModule && // Is Array.apply
65+
defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) => // Is `wrap{X, Ref}Array`
66+
tpd.JavaSeqLiteral(elem0 :: seqLit.elems, seqLit.elemtpt)
67+
case _ =>
68+
tree
69+
}
4570
}
4671

4772
} else tree

‎compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package dotty.tools.backend.jvm
2+
3+
import org.junit.Test
4+
import org.junit.Assert._
5+
6+
import scala.tools.asm.Opcodes._
7+
8+
class ArrayApplyOptTest extends DottyBytecodeTest {
9+
import ASMConverters._
10+
11+
@Test def testArrayEmptyGenericApply= {
12+
test("Array[String]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "java/lang/String"), Op(POP), Op(RETURN)))
13+
test("Array[Unit]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "scala/runtime/BoxedUnit"), Op(POP), Op(RETURN)))
14+
test("Array[Object]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "java/lang/Object"), Op(POP), Op(RETURN)))
15+
test("Array[Boolean]()", List(Op(ICONST_0), IntOp(NEWARRAY, 4), Op(POP), Op(RETURN)))
16+
test("Array[Char]()", List(Op(ICONST_0), IntOp(NEWARRAY, 5), Op(POP), Op(RETURN)))
17+
test("Array[Float]()", List(Op(ICONST_0), IntOp(NEWARRAY, 6), Op(POP), Op(RETURN)))
18+
test("Array[Double]()", List(Op(ICONST_0), IntOp(NEWARRAY, 7), Op(POP), Op(RETURN)))
19+
test("Array[Byte]()", List(Op(ICONST_0), IntOp(NEWARRAY, 8), Op(POP), Op(RETURN)))
20+
test("Array[Short]()", List(Op(ICONST_0), IntOp(NEWARRAY, 9), Op(POP), Op(RETURN)))
21+
test("Array[Int]()", List(Op(ICONST_0), IntOp(NEWARRAY, 10), Op(POP), Op(RETURN)))
22+
test("Array[Long]()", List(Op(ICONST_0), IntOp(NEWARRAY, 11), Op(POP), Op(RETURN)))
23+
test("Array[T]()", List(Op(ICONST_0), IntOp(NEWARRAY, 10), Op(POP), Op(RETURN)))
24+
}
25+
26+
@Test def testArrayGenericApply= {
27+
test("""Array("a", "b")""", List(Op(ICONST_2), TypeOp(ANEWARRAY, "java/lang/String"), Op(DUP), Op(ICONST_0), Ldc(LDC, "a"), Op(AASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, "b"), Op(AASTORE), Op(POP), Op(RETURN)))
28+
test("""Array[Object]("a", "b")""", List(Op(ICONST_2), TypeOp(ANEWARRAY, "java/lang/Object"), Op(DUP), Op(ICONST_0), Ldc(LDC, "a"), Op(AASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, "b"), Op(AASTORE), Op(POP), Op(RETURN)))
29+
}
30+
31+
@Test def testArrayApplyBoolean =
32+
test("Array(true, false)", List(Op(ICONST_2), IntOp(NEWARRAY, 4), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_0), Op(BASTORE), Op(POP), Op(RETURN)))
33+
34+
@Test def testArrayApplyByte =
35+
test("Array[Byte](1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 8), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(BASTORE), Op(POP), Op(RETURN)))
36+
37+
@Test def testArrayApplyShort =
38+
test("Array[Short](1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 9), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(SASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(SASTORE), Op(POP), Op(RETURN)))
39+
40+
@Test def testArrayApplyInt = {
41+
test("Array(1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), Op(POP), Op(RETURN)))
42+
test("""Array[T](t, t)""", List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE), Op(DUP), Op(ICONST_1), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE), Op(POP), Op(RETURN)))
43+
}
44+
45+
@Test def testArrayApplyLong =
46+
test("Array(2L, 3L)", List(Op(ICONST_2), IntOp(NEWARRAY, 11), Op(DUP), Op(ICONST_0), Ldc(LDC, 2), Op(LASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3), Op(LASTORE), Op(POP), Op(RETURN)))
47+
48+
@Test def testArrayApplyFloat =
49+
test("Array(2.1f, 3.1f)", List(Op(ICONST_2), IntOp(NEWARRAY, 6), Op(DUP), Op(ICONST_0), Ldc(LDC, 2.1f), Op(FASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.1f), Op(FASTORE), Op(POP), Op(RETURN)))
50+
51+
@Test def testArrayApplyDouble =
52+
test("Array(2.2d, 3.2d)", List(Op(ICONST_2), IntOp(NEWARRAY, 7), Op(DUP), Op(ICONST_0), Ldc(LDC, 2.2d), Op(DASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.2d), Op(DASTORE), Op(POP), Op(RETURN)))
53+
54+
@Test def testArrayApplyChar =
55+
test("Array('x', 'y')", List(Op(ICONST_2), IntOp(NEWARRAY, 5), Op(DUP), Op(ICONST_0), IntOp(BIPUSH, 120), Op(CASTORE), Op(DUP), Op(ICONST_1), IntOp(BIPUSH, 121), Op(CASTORE), Op(POP), Op(RETURN)))
56+
57+
@Test def testArrayApplyUnit =
58+
test("Array[Unit]((), ())", List(Op(ICONST_2), TypeOp(ANEWARRAY, "scala/runtime/BoxedUnit"), Op(DUP),
59+
Op(ICONST_0), Field(GETSTATIC, "scala/runtime/BoxedUnit", "UNIT", "Lscala/runtime/BoxedUnit;"), Op(AASTORE), Op(DUP),
60+
Op(ICONST_1), Field(GETSTATIC, "scala/runtime/BoxedUnit", "UNIT", "Lscala/runtime/BoxedUnit;"), Op(AASTORE), Op(POP), Op(RETURN)))
61+
62+
private def test(code: String, expectedInstructions: List[Any])= {
63+
val source =
64+
s"""class Foo {
65+
| import Foo._
66+
| def test: Unit = $code
67+
|}
68+
|object Foo {
69+
| opaque type T = Int
70+
| def t: T = 1
71+
|}
72+
""".stripMargin
73+
74+
checkBCode(source) { dir =>
75+
val clsIn = dir.lookupName("Foo.class", directory = false).input
76+
val clsNode = loadClassNode(clsIn)
77+
val meth = getMethod(clsNode, "test")
78+
79+
val instructions = instructionsFromMethod(meth)
80+
81+
assertEquals(expectedInstructions, instructions)
82+
}
83+
}
84+
85+
}

‎tests/run/i502.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Ok
2+
foo

‎tests/run/i502.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import scala.reflect.ClassTag
2+
3+
object Test extends App {
4+
Array[Int](1, 2)
5+
6+
try {
7+
Array[Int](1, 2)(null)
8+
???
9+
} catch {
10+
case _: NullPointerException => println("Ok")
11+
}
12+
13+
Array[Int](1, 2)({println("foo"); the[ClassTag[Int]]})
14+
}

‎tests/run/t6611b.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
object Test extends App {
2+
val a = Array("1")
3+
val a2 = Array(a: _*)
4+
a2(0) = "2"
5+
assert(a(0) == "1")
6+
}

0 commit comments

Comments
 (0)