-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #1167: Reduce the magic in Arrays.newRefArray. Implement multidimensional arrays #1188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
88baa04
27deb1e
0211e56
7e2352a
3f41bd6
57fcea6
3fc2b6e
2c370bd
9d7db0c
c74bb42
55e2ae6
0b1ca2d
5399fbe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ package dotty.runtime | |
|
||
import scala.reflect.ClassTag | ||
|
||
import java.lang.{reflect => jlr} | ||
|
||
/** All but the first two operations should be short-circuited and implemented specially by | ||
* the backend. | ||
*/ | ||
|
@@ -22,35 +24,8 @@ object Arrays { | |
arr | ||
} | ||
|
||
/** Create an array of type T. T must be of form Array[E], with | ||
* E being a reference type. | ||
/** Create an array of a reference type T. | ||
*/ | ||
def newRefArray[T](length: Int): T = ??? | ||
|
||
/** Create a Byte[] array */ | ||
def newByteArray(length: Int): Array[Byte] = ??? | ||
|
||
/** Create a Short[] array */ | ||
def newShortArray(length: Int): Array[Short] = ??? | ||
|
||
/** Create a Char[] array */ | ||
def newCharArray(length: Int): Array[Char] = ??? | ||
|
||
/** Create an Int[] array */ | ||
def newIntArray(length: Int): Array[Int] = ??? | ||
|
||
/** Create a Long[] array */ | ||
def newLongArray(length: Int): Array[Long] = ??? | ||
|
||
/** Create a Float[] array */ | ||
def newFloatArray(length: Int): Array[Float] = ??? | ||
|
||
/** Create a Double[] array */ | ||
def newDoubleArray(length: Int): Array[Double] = ??? | ||
|
||
/** Create a Boolean[] array */ | ||
def newBooleanArray(length: Int): Array[Boolean] = ??? | ||
|
||
/** Create a scala.runtime.BoxedUnit[] array */ | ||
def newUnitArray(length: Int): Array[Unit] = ??? | ||
def newArray[Arr](componentType: Class[_], returnType: Class[Arr], dimensions: Array[Int]): Arr = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Am I right that the backend will eliminate these calls? If not it would be a potential performance problem, say if someone creates lots of 3-element Float vectors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the intention! See example here: https://github.com/dotty-staging/dotty/blob/remove-newarray-magic/tests/run/MultiArr.scala |
||
jlr.Array.newInstance(componentType, dimensions: _*).asInstanceOf[Arr] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need Also, why no bound? Right now def newArray[T](componentType: Class[T], returnType: Class[Arrary[T]], dimensions: Array[Int]): Array[T] =
jlr.Array.newInstance(componentType, dimensions: _*).asInstanceOf[Arrary[T]] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I need to recreate the returned type at backend. And this is the easiest way I've found to do this.
It's NOT in sync before erasure. When you're creating a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ouch! Why do we need this? Why can't we keep a |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,8 +125,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { | |
def SeqLiteral(elems: List[Tree], elemtpt: Tree)(implicit ctx: Context): SeqLiteral = | ||
ta.assignType(untpd.SeqLiteral(elems, elemtpt), elems, elemtpt) | ||
|
||
def JavaSeqLiteral(elems: List[Tree], elemtpt: Tree)(implicit ctx: Context): SeqLiteral = | ||
ta.assignType(new untpd.JavaSeqLiteral(elems, elemtpt), elems, elemtpt) | ||
def JavaSeqLiteral(elems: List[Tree], elemtpt: Tree)(implicit ctx: Context): JavaSeqLiteral = | ||
ta.assignType(new untpd.JavaSeqLiteral(elems, elemtpt), elems, elemtpt).asInstanceOf[JavaSeqLiteral] | ||
|
||
def TypeTree(original: Tree)(implicit ctx: Context): TypeTree = | ||
TypeTree(original.tpe, original) | ||
|
@@ -362,18 +362,16 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { | |
* kind for the given element type in `typeArg`. No type arguments or | ||
* `length` arguments are given. | ||
*/ | ||
def newArray(typeArg: Tree, pos: Position)(implicit ctx: Context): Tree = { | ||
val elemType = typeArg.tpe | ||
val elemClass = elemType.classSymbol | ||
def newArr(kind: String) = | ||
ref(defn.DottyArraysModule).select(s"new${kind}Array".toTermName).withPos(pos) | ||
if (TypeErasure.isUnboundedGeneric(elemType)) | ||
newArr("Generic").appliedToTypeTrees(typeArg :: Nil) | ||
else if (elemClass.isPrimitiveValueClass) | ||
newArr(elemClass.name.toString) | ||
else | ||
newArr("Ref").appliedToTypeTrees( | ||
TypeTree(defn.ArrayOf(elemType)).withPos(typeArg.pos) :: Nil) | ||
def newArray(elemTpe: Type, returnTpe: Type, pos: Position, dims: JavaSeqLiteral)(implicit ctx: Context): Tree = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum, the signature of this method changes, but I see no call site change in this commit. That's suspicious ;) |
||
val elemClass = elemTpe.classSymbol | ||
def newArr = | ||
ref(defn.DottyArraysModule).select(defn.newArrayMethod).withPos(pos) | ||
|
||
if (!ctx.erasedTypes) { | ||
assert(!TypeErasure.isUnboundedGeneric(elemTpe)) //needs to be done during typer. See Applications.convertNewGenericArray | ||
newArr.appliedToTypeTrees(TypeTree(returnTpe) :: Nil).appliedToArgs(clsOf(elemTpe) :: clsOf(returnTpe) :: dims :: Nil).withPos(pos) | ||
} else // after erasure | ||
newArr.appliedToArgs(clsOf(elemTpe) :: clsOf(returnTpe) :: dims :: Nil).withPos(pos) | ||
} | ||
|
||
// ------ Creating typed equivalents of trees that exist only in untyped form ------- | ||
|
@@ -835,7 +833,10 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { | |
case tpnme.Float => TYPE(defn.BoxedFloatModule) | ||
case tpnme.Double => TYPE(defn.BoxedDoubleModule) | ||
case tpnme.Unit => TYPE(defn.BoxedUnitModule) | ||
case _ => Literal(Constant(TypeErasure.erasure(tp))) | ||
case _ => | ||
if(ctx.erasedTypes || !tp.derivesFrom(defn.ArrayClass)) | ||
Literal(Constant(TypeErasure.erasure(tp))) | ||
else Literal(Constant(tp)) | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package dotty.tools.dotc | ||
package transform | ||
|
||
import core._ | ||
import TreeTransforms._ | ||
import Contexts.Context | ||
import Flags._ | ||
import SymUtils._ | ||
import Symbols._ | ||
import SymDenotations._ | ||
import Types._ | ||
import Decorators._ | ||
import DenotTransformers._ | ||
import StdNames._ | ||
import NameOps._ | ||
import ast.Trees._ | ||
import dotty.tools.dotc.ast.tpd | ||
import util.Positions._ | ||
import Names._ | ||
|
||
import collection.mutable | ||
import ResolveSuper._ | ||
|
||
import scala.collection.immutable.:: | ||
|
||
|
||
/** This phase rewrites calls to array constructors to newArray method in Dotty.runtime.Arrays module. | ||
* | ||
* It assummes that generic arrays have already been handled by typer(see Applications.convertNewGenericArray). | ||
* Additionally it optimizes calls to scala.Array.ofDim functions by replacing them with calls to newArray with specific dimensions | ||
*/ | ||
class ArrayConstructors extends MiniPhaseTransform { thisTransform => | ||
import ast.tpd._ | ||
|
||
override def phaseName: String = "arrayConstructors" | ||
|
||
override def transformApply(tree: tpd.Apply)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { | ||
def rewrite(elemType: Type, dims: List[Tree]) = | ||
tpd.newArray(elemType, tree.tpe, tree.pos, JavaSeqLiteral(dims, TypeTree(defn.IntClass.typeRef))) | ||
|
||
if (tree.fun.symbol eq defn.ArrayConstructor) { | ||
val TypeApply(tycon, targ :: Nil) = tree.fun | ||
rewrite(targ.tpe, tree.args) | ||
} else if ((tree.fun.symbol.maybeOwner eq defn.ArrayModule) && (tree.fun.symbol.name eq nme.ofDim) && !tree.tpe.isInstanceOf[MethodicType]) { | ||
|
||
tree.fun match { | ||
case Apply(TypeApply(t: Ident, targ), dims) if !TypeErasure.isUnboundedGeneric(targ.head.tpe) => | ||
rewrite(targ.head.tpe, dims) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens to the implicit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. implicit ClassTag argument to ofDim is the argument of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right. OK. |
||
case Apply(TypeApply(t: Select, targ), dims) if !TypeErasure.isUnboundedGeneric(targ.head.tpe) => | ||
Block(t.qualifier :: Nil, rewrite(targ.head.tpe, dims)) | ||
case _ => tree | ||
} | ||
|
||
} else tree | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is obsolete.