Skip to content

Commit 3bd88e4

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

File tree

6 files changed

+132
-14
lines changed

6 files changed

+132
-14
lines changed

‎community-build/community-projects/scalatest

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

Lines changed: 32 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,34 @@ 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+
tree match {
52+
case Apply(Apply(appMeth, Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil) :: Nil), (ct: Apply) :: Nil)
53+
if appMeth.symbol.name == nme.apply && appMeth.symbol.owner == defn.ArrayModule &&
54+
defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) &&
55+
ct.symbol.maybeOwner.companionModule == defn.ClassTagModule =>
56+
seqLit
57+
case Apply(appMeth, elem0 :: Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil) :: Nil)
58+
if appMeth.symbol.name == nme.apply && appMeth.symbol.owner == defn.ArrayModule &&
59+
defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) =>
60+
tpd.JavaSeqLiteral(elem0 :: seqLit.elems, seqLit.elemtpt)
61+
case _ =>
62+
tree
63+
}
4564
}
4665

4766
} else tree

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
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+
}
24+
25+
@Test def testArrayGenericApply= {
26+
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)))
27+
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)))
28+
}
29+
30+
@Test def testArrayApplyBoolean =
31+
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)))
32+
33+
@Test def testArrayApplyByte =
34+
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)))
35+
36+
@Test def testArrayApplyShort =
37+
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)))
38+
39+
@Test def testArrayApplyInt =
40+
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)))
41+
42+
@Test def testArrayApplyLong =
43+
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)))
44+
45+
@Test def testArrayApplyFloat =
46+
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)))
47+
48+
@Test def testArrayApplyDouble =
49+
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)))
50+
51+
@Test def testArrayApplyChar =
52+
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)))
53+
54+
@Test def testArrayApplyUnit =
55+
test("Array[Unit]((), ())", List(Op(ICONST_2), TypeOp(ANEWARRAY, "scala/runtime/BoxedUnit"), Op(DUP),
56+
Op(ICONST_0), Field(GETSTATIC, "scala/runtime/BoxedUnit", "UNIT", "Lscala/runtime/BoxedUnit;"), Op(AASTORE), Op(DUP),
57+
Op(ICONST_1), Field(GETSTATIC, "scala/runtime/BoxedUnit", "UNIT", "Lscala/runtime/BoxedUnit;"), Op(AASTORE), Op(POP), Op(RETURN)))
58+
59+
private def test(code: String, expectedInstructions: List[Any])= {
60+
val source =
61+
s"""class Foo {
62+
| def test: Unit = $code
63+
|}
64+
""".stripMargin
65+
66+
checkBCode(source) { dir =>
67+
val clsIn = dir.lookupName("Foo.class", directory = false).input
68+
val clsNode = loadClassNode(clsIn)
69+
val meth = getMethod(clsNode, "test")
70+
71+
val instructions = instructionsFromMethod(meth)
72+
73+
assertEquals(expectedInstructions, instructions)
74+
}
75+
}
76+
77+
}

‎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)