Skip to content

Commit 4c865ec

Browse files
committed
Scala.js: Optimize varargs in a platform-dependent friendly way.
We now recognize the shapes of varargs, deconstruct them, and emit instead calls to Scala.js-specific runtime methods. Those methods choose at link-time the best implementation for the target platform. The changes in `genActualArgs` revealed that we cannot trust `Symbol.isPrivate`. It uses `lastDenot.flagsUNSAFE` under the hood, which returns absurd results if `lastDenot` happened to be accessed within an `atPhase(...)` call. Therefore, we change those calls to `sym.is(Private)` instead. `Symbol.is` always uses an up-to-date denotation. This commit is essentially a forward port of the Scala.js commit scala-js/scala-js@24750cf
1 parent 5f1d5cf commit 4c865ec

File tree

2 files changed

+77
-59
lines changed

2 files changed

+77
-59
lines changed

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

Lines changed: 76 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,10 +1609,10 @@ class JSCodeGen()(using genCtx: Context) {
16091609
optimizerHints, Unversioned)
16101610
} else {
16111611
val namespace = if (isMethodStaticInIR(sym)) {
1612-
if (sym.isPrivate) js.MemberNamespace.PrivateStatic
1612+
if (sym.is(Private)) js.MemberNamespace.PrivateStatic
16131613
else js.MemberNamespace.PublicStatic
16141614
} else {
1615-
if (sym.isPrivate) js.MemberNamespace.Private
1615+
if (sym.is(Private)) js.MemberNamespace.Private
16161616
else js.MemberNamespace.Public
16171617
}
16181618
val resultIRType = toIRType(patchedResultType(sym))
@@ -3770,7 +3770,7 @@ class JSCodeGen()(using genCtx: Context) {
37703770
/** Gen a statically linked call to an instance method. */
37713771
def genApplyMethodMaybeStatically(receiver: js.Tree, method: Symbol,
37723772
arguments: List[js.Tree])(implicit pos: Position): js.Tree = {
3773-
if (method.isPrivate || method.isClassConstructor)
3773+
if (method.is(Private) || method.isClassConstructor)
37743774
genApplyMethodStatically(receiver, method, arguments)
37753775
else
37763776
genApplyMethod(receiver, method, arguments)
@@ -3779,7 +3779,7 @@ class JSCodeGen()(using genCtx: Context) {
37793779
/** Gen a dynamically linked call to a Scala method. */
37803780
def genApplyMethod(receiver: js.Tree, method: Symbol, arguments: List[js.Tree])(
37813781
implicit pos: Position): js.Tree = {
3782-
assert(!method.isPrivate,
3782+
assert(!method.is(Private),
37833783
s"Cannot generate a dynamic call to private method $method at $pos")
37843784
js.Apply(js.ApplyFlags.empty, receiver, encodeMethodSym(method), arguments)(
37853785
toIRType(patchedResultType(method)))
@@ -3789,7 +3789,7 @@ class JSCodeGen()(using genCtx: Context) {
37893789
def genApplyMethodStatically(receiver: js.Tree, method: Symbol, arguments: List[js.Tree])(
37903790
implicit pos: Position): js.Tree = {
37913791
val flags = js.ApplyFlags.empty
3792-
.withPrivate(method.isPrivate && !method.isClassConstructor)
3792+
.withPrivate(method.is(Private) && !method.isClassConstructor)
37933793
.withConstructor(method.isClassConstructor)
37943794
js.ApplyStatically(flags, receiver, encodeClassName(method.owner),
37953795
encodeMethodSym(method), arguments)(
@@ -3799,7 +3799,7 @@ class JSCodeGen()(using genCtx: Context) {
37993799
/** Gen a call to a static method. */
38003800
private def genApplyStatic(method: Symbol, arguments: List[js.Tree])(
38013801
implicit pos: Position): js.Tree = {
3802-
js.ApplyStatic(js.ApplyFlags.empty.withPrivate(method.isPrivate),
3802+
js.ApplyStatic(js.ApplyFlags.empty.withPrivate(method.is(Private)),
38033803
encodeClassName(method.owner), encodeMethodSym(method), arguments)(
38043804
toIRType(patchedResultType(method)))
38053805
}
@@ -4400,34 +4400,53 @@ class JSCodeGen()(using genCtx: Context) {
44004400
* This tries to optimize repeated arguments (varargs) by turning them
44014401
* into js.WrappedArray instead of Scala wrapped arrays.
44024402
*/
4403-
private def genActualArgs(sym: Symbol, args: List[Tree])(
4404-
implicit pos: Position): List[js.Tree] = {
4405-
args.map(genExpr)
4406-
/*val wereRepeated = exitingPhase(currentRun.typerPhase) {
4407-
sym.tpe.params.map(p => isScalaRepeatedParamType(p.tpe))
4403+
private def genActualArgs(sym: Symbol, args: List[Tree]): List[js.Tree] = {
4404+
// Fast path for methods that do not have any repeated parameter
4405+
val isAnyParamRepeated: Boolean = atPhase(elimRepeatedPhase) {
4406+
sym.info.paramInfoss.flatten.exists(_.isRepeatedParam)
44084407
}
44094408

4410-
if (wereRepeated.size > args.size) {
4411-
// Should not happen, but let's not crash
4409+
if (!isAnyParamRepeated) {
44124410
args.map(genExpr)
44134411
} else {
4414-
/* Arguments that are in excess compared to the type signature after
4415-
* erasure are lambda-lifted arguments. They cannot be repeated, hence
4416-
* the extension to `false`.
4412+
/* Erasure and lambdalift both insert capture params in dotc, at
4413+
* unpredictable positions. Therefore, we use the *names* of parameters
4414+
* as the only reliable link to whether they were initially repeated.
4415+
* We also do this in JSSymUtils.jsParamInfos for JS methods.
44174416
*/
4418-
for ((arg, wasRepeated) <- args.zipAll(wereRepeated, EmptyTree, false)) yield {
4419-
if (wasRepeated) {
4420-
tryGenRepeatedParamAsJSArray(arg, handleNil = false).fold {
4421-
genExpr(arg)
4422-
} { genArgs =>
4423-
genNew(WrappedArrayClass, WrappedArray_ctor,
4424-
List(js.JSArrayConstr(genArgs)))
4417+
val namesOfRepeatedParams: Set[TermName] = atPhase(elimRepeatedPhase) {
4418+
sym.info.paramNamess.flatten.zip(sym.info.paramInfoss.flatten).collect {
4419+
case (paramName, paramInfo) if paramInfo.isRepeatedParam => paramName
4420+
}.toSet
4421+
}
4422+
4423+
for ((arg, paramName) <- args.zip(sym.info.paramNamess.flatten)) yield {
4424+
if (namesOfRepeatedParams.contains(paramName)) {
4425+
/* If the argument is a call to the compiler's chosen `wrapArray`
4426+
* method with an array literal as argument, we know it actually
4427+
* came from expanded varargs. In that case, rewrite to calling our
4428+
* custom `scala.scalajs.runtime.to*VarArgs` method. These methods
4429+
* choose the best implementation of varargs depending on the
4430+
* target platform.
4431+
*/
4432+
arg match {
4433+
case MaybeAsInstanceOf(wrapArray @ WrapArray(MaybeAsInstanceOf(arrayValue: JavaSeqLiteral))) =>
4434+
implicit val pos: SourcePosition = wrapArray.sourcePos
4435+
js.Apply(
4436+
js.ApplyFlags.empty,
4437+
js.LoadModule(ScalaJSRuntimeModClassName),
4438+
js.MethodIdent(WrapArray.wrapArraySymToToVarArgsName(wrapArray.symbol)),
4439+
List(genExpr(arrayValue))
4440+
)(jstpe.ClassType(encodeClassName(defn.SeqClass), nullable = true))
4441+
4442+
case _ =>
4443+
genExpr(arg)
44254444
}
44264445
} else {
44274446
genExpr(arg)
44284447
}
44294448
}
4430-
}*/
4449+
}
44314450
}
44324451

44334452
/** Gen actual actual arguments to a JS method call.
@@ -4496,47 +4515,28 @@ class JSCodeGen()(using genCtx: Context) {
44964515
* `js.Array`.
44974516
*/
44984517
private def genJSRepeatedParam(arg: Tree): List[js.TreeOrJSSpread] = {
4499-
tryGenRepeatedParamAsJSArray(arg, handleNil = true).getOrElse {
4500-
/* Fall back to calling runtime.genTraversableOnce2jsArray
4501-
* to perform the conversion to js.Array, then wrap in a Spread
4502-
* operator.
4503-
*/
4504-
implicit val pos: SourcePosition = arg.sourcePos
4505-
val jsArrayArg = genModuleApplyMethod(
4506-
jsdefn.Runtime_toJSVarArgs,
4507-
List(genExpr(arg)))
4508-
List(js.JSSpread(jsArrayArg))
4509-
}
4510-
}
4511-
4512-
/** Try and expand an actual argument to a repeated param `(xs: T*)`.
4513-
*
4514-
* This method recognizes the shapes of tree generated by the desugaring
4515-
* of repeated params in Scala, and expands them.
4516-
* If `arg` does not have the shape of a generated repeated param, this
4517-
* method returns `None`.
4518-
*/
4519-
private def tryGenRepeatedParamAsJSArray(arg: Tree,
4520-
handleNil: Boolean): Option[List[js.Tree]] = {
4521-
implicit val pos = arg.span
4518+
implicit val pos: SourcePosition = arg.sourcePos
45224519

45234520
// Given a method `def foo(args: T*)`
45244521
arg match {
45254522
// foo(arg1, arg2, ..., argN) where N > 0
45264523
case MaybeAsInstanceOf(WrapArray(MaybeAsInstanceOf(array: JavaSeqLiteral))) =>
45274524
/* Value classes in arrays are already boxed, so no need to use
45284525
* the type before erasure.
4529-
* TODO Is this true in dotty?
45304526
*/
4531-
Some(array.elems.map(e => box(genExpr(e), e.tpe)))
4527+
array.elems.map(e => box(genExpr(e), e.tpe))
45324528

45334529
// foo()
4534-
case Ident(_) if handleNil && arg.symbol == defn.NilModule =>
4535-
Some(Nil)
4530+
case Ident(_) if arg.symbol == defn.NilModule =>
4531+
Nil
45364532

45374533
// foo(argSeq: _*) - cannot be optimized
45384534
case _ =>
4539-
None
4535+
/* Fall back to calling runtime.toJSVarArgs to perform the conversion
4536+
* to js.Array, then wrap in a Spread operator.
4537+
*/
4538+
val jsArrayArg = genModuleApplyMethod(jsdefn.Runtime_toJSVarArgs, List(genExpr(arg)))
4539+
List(js.JSSpread(jsArrayArg))
45404540
}
45414541
}
45424542

@@ -4551,16 +4551,32 @@ class JSCodeGen()(using genCtx: Context) {
45514551
}
45524552

45534553
private object WrapArray {
4554-
lazy val isWrapArray: Set[Symbol] = {
4555-
val names0 = defn.ScalaValueClasses().map(sym => nme.wrapXArray(sym.name))
4556-
val names1 = names0 ++ Set(nme.wrapRefArray, nme.genericWrapArray)
4557-
val symsInPredef = names1.map(defn.ScalaPredefModule.requiredMethod(_))
4558-
val symsInScalaRunTime = names1.map(defn.ScalaRuntimeModule.requiredMethod(_))
4559-
(symsInPredef ++ symsInScalaRunTime).toSet
4554+
lazy val wrapArraySymToToVarArgsName: Map[Symbol, MethodName] = {
4555+
val SeqClassRef = jstpe.ClassRef(encodeClassName(defn.SeqClass))
4556+
4557+
val items: Seq[(Name, String, jstpe.TypeRef)] = Seq(
4558+
(nme.genericWrapArray, "toGenericVarArgs", jswkn.ObjectRef),
4559+
(nme.wrapRefArray, "toRefVarArgs", jstpe.ArrayTypeRef(jswkn.ObjectRef, 1)),
4560+
(tpd.wrapArrayMethodName(defn.UnitType), "toUnitVarArgs", jstpe.ArrayTypeRef(jstpe.ClassRef(jswkn.BoxedUnitClass), 1)),
4561+
(tpd.wrapArrayMethodName(defn.BooleanType), "toBooleanVarArgs", jstpe.ArrayTypeRef(jstpe.BooleanRef, 1)),
4562+
(tpd.wrapArrayMethodName(defn.CharType), "toCharVarArgs", jstpe.ArrayTypeRef(jstpe.CharRef, 1)),
4563+
(tpd.wrapArrayMethodName(defn.ByteType), "toByteVarArgs", jstpe.ArrayTypeRef(jstpe.ByteRef, 1)),
4564+
(tpd.wrapArrayMethodName(defn.ShortType), "toShortVarArgs", jstpe.ArrayTypeRef(jstpe.ShortRef, 1)),
4565+
(tpd.wrapArrayMethodName(defn.IntType), "toIntVarArgs", jstpe.ArrayTypeRef(jstpe.IntRef, 1)),
4566+
(tpd.wrapArrayMethodName(defn.LongType), "toLongVarArgs", jstpe.ArrayTypeRef(jstpe.LongRef, 1)),
4567+
(tpd.wrapArrayMethodName(defn.FloatType), "toFloatVarArgs", jstpe.ArrayTypeRef(jstpe.FloatRef, 1)),
4568+
(tpd.wrapArrayMethodName(defn.DoubleType), "toDoubleVarArgs", jstpe.ArrayTypeRef(jstpe.DoubleRef, 1))
4569+
)
4570+
4571+
items.map { case (wrapArrayName, simpleName, argTypeRef) =>
4572+
val wrapArraySym = defn.getWrapVarargsArrayModule.requiredMethod(wrapArrayName)
4573+
val toVarArgsName = MethodName(simpleName, argTypeRef :: Nil, SeqClassRef)
4574+
wrapArraySym -> toVarArgsName
4575+
}.toMap
45604576
}
45614577

45624578
def unapply(tree: Apply): Option[Tree] = tree match {
4563-
case Apply(wrapArray_?, List(wrapped)) if isWrapArray(wrapArray_?.symbol) =>
4579+
case Apply(wrapArray_?, List(wrapped)) if wrapArraySymToToVarArgsName.contains(wrapArray_?.symbol) =>
45644580
Some(wrapped)
45654581
case _ =>
45664582
None
@@ -5002,6 +5018,7 @@ object JSCodeGen {
50025018
private val JLRArrayClassName = ClassName("java.lang.reflect.Array")
50035019
private val JSObjectClassName = ClassName("scala.scalajs.js.Object")
50045020
private val JavaScriptExceptionClassName = ClassName("scala.scalajs.js.JavaScriptException")
5021+
private val ScalaJSRuntimeModClassName = ClassName("scala.scalajs.runtime.package$")
50055022

50065023
private val ObjectArrayTypeRef = jstpe.ArrayTypeRef(jswkn.ObjectRef, 1)
50075024

compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ object JSSymUtils {
188188
sym.info.paramNamess.flatten.zip(sym.info.paramInfoss.flatten)
189189

190190
val paramInfosAtElimRepeated = atPhase(elimRepeatedPhase) {
191+
// See also JSCodeGen.genActualArgs
191192
val list =
192193
for ((name, info) <- paramNamesAndTypes) yield {
193194
val v =

0 commit comments

Comments
 (0)