Skip to content

Commit 4136421

Browse files
committed
Fix #1167: Remove the magic from Arrays.newRefArray.
Previously, the method `Arrays.newRefArray` was one of the only 3 methods that are kept generic after erasure. This commit removes this magic, by making it take an actual `j.l.Class[T]` as parameter. Moreover, the methods `newXArray` all receive an actual body, implemented on top of Java reflection, which means that a back-end does not *have to* special-case those methods for correctness. It might still be required for performance, though, depending on the back-end. The JVM back-end is made non-optimal in this commit, precisely because it does not specialize that method anymore. Doing so requires modifying the fork of scalac that we use, which should be done separately. The JS back-end is adapted simply by doing nothing at all on any of the newXArray methods. It will normally call the user-space implementations which use reflection. The Scala.js optimizer will inline and intrinsify the reflective calls, producing optimal code, at the end of the day.
1 parent d47ab77 commit 4136421

File tree

8 files changed

+36
-67
lines changed

8 files changed

+36
-67
lines changed

src/dotty/runtime/Arrays.scala

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package dotty.runtime
22

33
import scala.reflect.ClassTag
44

5+
import java.lang.{reflect => jlr}
6+
57
/** All but the first two operations should be short-circuited and implemented specially by
68
* the backend.
79
*/
@@ -22,35 +24,44 @@ object Arrays {
2224
arr
2325
}
2426

25-
/** Create an array of type T. T must be of form Array[E], with
26-
* E being a reference type.
27+
/** Create an array of a reference type T.
2728
*/
28-
def newRefArray[T](length: Int): T = ???
29+
def newRefArray[T](componentType: Class[T])(length: Int): Array[T] =
30+
jlr.Array.newInstance(componentType, length).asInstanceOf[Array[T]]
2931

3032
/** Create a Byte[] array */
31-
def newByteArray(length: Int): Array[Byte] = ???
33+
def newByteArray(length: Int): Array[Byte] =
34+
jlr.Array.newInstance(classOf[Byte], length).asInstanceOf[Array[Byte]]
3235

3336
/** Create a Short[] array */
34-
def newShortArray(length: Int): Array[Short] = ???
37+
def newShortArray(length: Int): Array[Short] =
38+
jlr.Array.newInstance(classOf[Short], length).asInstanceOf[Array[Short]]
3539

3640
/** Create a Char[] array */
37-
def newCharArray(length: Int): Array[Char] = ???
41+
def newCharArray(length: Int): Array[Char] =
42+
jlr.Array.newInstance(classOf[Char], length).asInstanceOf[Array[Char]]
3843

3944
/** Create an Int[] array */
40-
def newIntArray(length: Int): Array[Int] = ???
45+
def newIntArray(length: Int): Array[Int] =
46+
jlr.Array.newInstance(classOf[Int], length).asInstanceOf[Array[Int]]
4147

4248
/** Create a Long[] array */
43-
def newLongArray(length: Int): Array[Long] = ???
49+
def newLongArray(length: Int): Array[Long] =
50+
jlr.Array.newInstance(classOf[Long], length).asInstanceOf[Array[Long]]
4451

4552
/** Create a Float[] array */
46-
def newFloatArray(length: Int): Array[Float] = ???
53+
def newFloatArray(length: Int): Array[Float] =
54+
jlr.Array.newInstance(classOf[Float], length).asInstanceOf[Array[Float]]
4755

4856
/** Create a Double[] array */
49-
def newDoubleArray(length: Int): Array[Double] = ???
57+
def newDoubleArray(length: Int): Array[Double] =
58+
jlr.Array.newInstance(classOf[Double], length).asInstanceOf[Array[Double]]
5059

5160
/** Create a Boolean[] array */
52-
def newBooleanArray(length: Int): Array[Boolean] = ???
61+
def newBooleanArray(length: Int): Array[Boolean] =
62+
jlr.Array.newInstance(classOf[Boolean], length).asInstanceOf[Array[Boolean]]
5363

5464
/** Create a scala.runtime.BoxedUnit[] array */
55-
def newUnitArray(length: Int): Array[Unit] = ???
65+
def newUnitArray(length: Int): Array[Unit] =
66+
jlr.Array.newInstance(classOf[scala.runtime.BoxedUnit], length).asInstanceOf[Array[Unit]]
5667
}

src/dotty/tools/backend/jvm/DottyBackendInterface.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile)(implicit ctx: Context
153153
}.toMap
154154
def unboxMethods: Map[Symbol, Symbol] = defn.ScalaValueClasses().map(x => (x, Erasure.Boxing.unboxMethod(x.asClass))).toMap
155155

156-
private val mkArrayNames: Set[Name] = Set("Byte", "Float", "Char", "Double", "Boolean", "Unit", "Long", "Int", "Short", "Ref").map{ x=>
156+
private val mkArrayNames: Set[Name] = Set("Byte", "Float", "Char", "Double", "Boolean", "Unit", "Long", "Int", "Short"/*, "Ref"*/).map{ x=>
157157
("new" + x + "Array").toTermName
158158
}
159159

src/dotty/tools/backend/sjs/JSCodeGen.scala

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,8 +1036,6 @@ class JSCodeGen()(implicit ctx: Context) {
10361036
genStringConcat(tree, receiver, args)
10371037
else if (code == HASH)
10381038
genScalaHash(tree, receiver)
1039-
else if (isArrayNew(code))
1040-
genArrayNew(tree, code)
10411039
else if (isArrayOp(code))
10421040
genArrayOp(tree, code)
10431041
else if (code == SYNCHRONIZED)
@@ -1409,24 +1407,6 @@ class JSCodeGen()(implicit ctx: Context) {
14091407
List(genExpr(receiver)))
14101408
}
14111409

1412-
/** Gen JS code for a new array operation. */
1413-
private def genArrayNew(tree: Tree, code: Int): js.Tree = {
1414-
import scala.tools.nsc.backend.ScalaPrimitives._
1415-
1416-
implicit val pos: Position = tree.pos
1417-
1418-
val Apply(fun, args) = tree
1419-
val genLength = genExpr(args.head)
1420-
1421-
toIRType(tree.tpe) match {
1422-
case arrayType: jstpe.ArrayType =>
1423-
js.NewArray(arrayType, List(genLength))
1424-
1425-
case irTpe =>
1426-
throw new FatalError(s"ArrayNew $tree must have an array type but was $irTpe")
1427-
}
1428-
}
1429-
14301410
/** Gen JS code for an array operation (get, set or length) */
14311411
private def genArrayOp(tree: Tree, code: Int): js.Tree = {
14321412
import scala.tools.nsc.backend.ScalaPrimitives._

src/dotty/tools/backend/sjs/JSPrimitives.scala

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,6 @@ class JSPrimitives(ctx: Context) extends DottyPrimitives(ctx) {
8080

8181
val jsdefn = JSDefinitions.jsdefn
8282

83-
// For some reason, the JVM primitive set does not register those
84-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newBooleanArray")), NEW_ZARRAY)
85-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newByteArray")), NEW_BARRAY)
86-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newShortArray")), NEW_SARRAY)
87-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newCharArray")), NEW_CARRAY)
88-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newIntArray")), NEW_IARRAY)
89-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newLongArray")), NEW_LARRAY)
90-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newFloatArray")), NEW_FARRAY)
91-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newDoubleArray")), NEW_DARRAY)
92-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newRefArray")), NEW_OARRAY)
93-
addPrimitive(defn.DottyArraysModule.requiredMethod(Names.termName("newUnitArray")), NEW_OARRAY)
94-
9583
addPrimitive(defn.Any_getClass, GETCLASS)
9684

9785
for (i <- 0 to 22)

src/dotty/tools/dotc/ast/tpd.scala

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,9 +372,12 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
372372
newArr("Generic").appliedToTypeTrees(typeArg :: Nil)
373373
else if (elemClass.isPrimitiveValueClass)
374374
newArr(elemClass.name.toString)
375-
else
376-
newArr("Ref").appliedToTypeTrees(
377-
TypeTree(defn.ArrayOf(elemType)).withPos(typeArg.pos) :: Nil)
375+
else {
376+
val typeApplied = newArr("Ref").appliedToTypeTrees(typeArg :: Nil)
377+
val classOfArg =
378+
ref(defn.Predef_classOf).appliedToTypeTrees(typeArg :: Nil)
379+
Apply(typeApplied, classOfArg :: Nil).withPos(typeArg.pos)
380+
}
378381
}
379382

380383
// ------ Creating typed equivalents of trees that exist only in untyped form -------

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,6 @@ class Definitions {
247247
lazy val DottyArraysModuleRef = ctx.requiredModuleRef("dotty.runtime.Arrays")
248248
def DottyArraysModule(implicit ctx: Context) = DottyArraysModuleRef.symbol
249249

250-
def newRefArrayMethod(implicit ctx: Context) = DottyArraysModule.requiredMethod("newRefArray")
251-
252250
lazy val NilModuleRef = ctx.requiredModuleRef("scala.collection.immutable.Nil")
253251
def NilModule(implicit ctx: Context) = NilModuleRef.symbol
254252

@@ -620,7 +618,7 @@ class Definitions {
620618
lazy val PhantomClasses = Set[Symbol](AnyClass, AnyValClass, NullClass, NothingClass)
621619

622620
def isPolymorphicAfterErasure(sym: Symbol) =
623-
(sym eq Any_isInstanceOf) || (sym eq Any_asInstanceOf) || (sym eq newRefArrayMethod)
621+
(sym eq Any_isInstanceOf) || (sym eq Any_asInstanceOf)
624622

625623
def isTupleType(tp: Type)(implicit ctx: Context) = {
626624
val arity = tp.dealias.argInfos.length

src/dotty/tools/dotc/transform/Erasure.scala

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -467,28 +467,18 @@ object Erasure extends TypeTestsCasts{
467467
tpt = untpd.TypedSplice(TypeTree(sym.info).withPos(vdef.tpt.pos))), sym)
468468

469469
override def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(implicit ctx: Context) = {
470-
var effectiveSym = sym
471-
if (sym == defn.newRefArrayMethod) {
472-
// newRefArray is treated specially: It's the only source-defined method
473-
// that has a polymorphic type after erasure. But treating its (dummy) definition
474-
// with a polymorphic type at and after erasure is an awkward special case.
475-
// We therefore rewrite the method definition with a new Symbol of type
476-
// (length: Int)Object
477-
val MethodType(pnames, ptypes) = sym.info.resultType
478-
effectiveSym = sym.copy(info = MethodType(pnames, ptypes, defn.ObjectType))
479-
}
480470
val restpe =
481-
if (effectiveSym.isConstructor) defn.UnitType
482-
else effectiveSym.info.resultType
471+
if (sym.isConstructor) defn.UnitType
472+
else sym.info.resultType
483473
val ddef1 = untpd.cpy.DefDef(ddef)(
484474
tparams = Nil,
485-
vparamss = (outer.paramDefs(effectiveSym) ::: ddef.vparamss.flatten) :: Nil,
475+
vparamss = (outer.paramDefs(sym) ::: ddef.vparamss.flatten) :: Nil,
486476
tpt = untpd.TypedSplice(TypeTree(restpe).withPos(ddef.tpt.pos)),
487477
rhs = ddef.rhs match {
488478
case id @ Ident(nme.WILDCARD) => untpd.TypedSplice(id.withType(restpe))
489479
case _ => ddef.rhs
490480
})
491-
super.typedDefDef(ddef1, effectiveSym)
481+
super.typedDefDef(ddef1, sym)
492482
}
493483

494484
/** After erasure, we may have to replace the closure method by a bridge.

src/dotty/tools/dotc/transform/TreeChecker.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,7 @@ class TreeChecker extends Phase with SymTransformer {
277277
def isNonMagicalMethod(x: Symbol) =
278278
x.is(Method) &&
279279
!x.isCompanionMethod &&
280-
!x.isValueClassConvertMethod &&
281-
x != defn.newRefArrayMethod
280+
!x.isValueClassConvertMethod
282281

283282
val symbolsNotDefined = cls.classInfo.decls.toSet.filter(isNonMagicalMethod) -- impl.body.map(_.symbol) - constr.symbol
284283

0 commit comments

Comments
 (0)