Skip to content

Commit 91e2a68

Browse files
sjrdsjrd
authored
Merge pull request #4537 from gzm0/allow-secondary-with-defaults
Allow delegating JS Class constructor calls with default params
2 parents 5472950 + 2249ba6 commit 91e2a68

File tree

5 files changed

+110
-58
lines changed

5 files changed

+110
-58
lines changed

‎compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala

Lines changed: 79 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,7 +1475,8 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
14751475
val jsClassCaptures = List.newBuilder[js.ParamDef]
14761476

14771477
def add(tree: ConstructorTree[_ <: JSCtor]): Unit = {
1478-
val (e, c) = genJSClassCtorDispatch(tree.ctor.sym, tree.ctor.params, tree.overloadNum)
1478+
val (e, c) = genJSClassCtorDispatch(tree.ctor.sym,
1479+
tree.ctor.paramsAndInfo, tree.overloadNum)
14791480
exports += e
14801481
jsClassCaptures ++= c
14811482
tree.subCtors.foreach(add(_))
@@ -1522,7 +1523,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
15221523
* call, and are therefore executed later than for a Scala class.
15231524
*/
15241525
withPerMethodBodyState(sym) {
1525-
stats.foreach {
1526+
flatStats(stats).foreach {
15261527
case tree @ Apply(fun @ Select(Super(This(_), _), _), args)
15271528
if fun.symbol.isClassConstructor =>
15281529
assert(jsSuperCall.isEmpty, s"Found 2 JS Super calls at ${dd.pos}")
@@ -1543,9 +1544,8 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
15431544
assert(jsSuperCall.isDefined, "Did not find Super call in primary JS " +
15441545
s"construtor at ${dd.pos}")
15451546

1546-
val params = if (vparamss.isEmpty) Nil else vparamss.head.map(_.symbol)
1547-
1548-
new PrimaryJSCtor(sym, params, jsSuperCall.get :: jsStats.result())
1547+
new PrimaryJSCtor(sym, genParamsAndInfo(sym, vparamss),
1548+
jsSuperCall.get :: jsStats.result())
15491549
}
15501550

15511551
private def genSecondaryJSClassCtor(dd: DefDef): SplitSecondaryJSCtor = {
@@ -1558,7 +1558,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
15581558
val afterThisCall = List.newBuilder[js.Tree]
15591559

15601560
withPerMethodBodyState(sym) {
1561-
stats.foreach {
1561+
flatStats(stats).foreach {
15621562
case tree @ Apply(fun @ Select(This(_), _), args)
15631563
if fun.symbol.isClassConstructor =>
15641564
assert(thisCall.isEmpty,
@@ -1579,21 +1579,27 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
15791579

15801580
val Some((targetCtor, ctorArgs)) = thisCall
15811581

1582-
val params = if (vparamss.isEmpty) Nil else vparamss.head.map(_.symbol)
1583-
1584-
new SplitSecondaryJSCtor(sym, params, beforeThisCall.result(), targetCtor,
1585-
ctorArgs, afterThisCall.result())
1582+
new SplitSecondaryJSCtor(sym, genParamsAndInfo(sym, vparamss),
1583+
beforeThisCall.result(), targetCtor, ctorArgs, afterThisCall.result())
15861584
}
15871585

1588-
private def genJSClassCtorDispatch(ctorSym: Symbol, allParamSyms: List[Symbol],
1589-
overloadNum: Int): (Exported, List[js.ParamDef]) = {
1586+
private def genParamsAndInfo(ctorSym: Symbol,
1587+
vparamss: List[List[ValDef]]): List[(js.VarRef, JSParamInfo)] = {
15901588
implicit val pos = ctorSym.pos
15911589

1592-
val allParamsAndInfos = for {
1593-
(paramSym, info) <- allParamSyms.zip(jsParamInfos(ctorSym))
1590+
val paramSyms = if (vparamss.isEmpty) Nil else vparamss.head.map(_.symbol)
1591+
1592+
for {
1593+
(paramSym, info) <- paramSyms.zip(jsParamInfos(ctorSym))
15941594
} yield {
15951595
genVarRef(paramSym) -> info
15961596
}
1597+
}
1598+
1599+
private def genJSClassCtorDispatch(ctorSym: Symbol,
1600+
allParamsAndInfos: List[(js.VarRef, JSParamInfo)],
1601+
overloadNum: Int): (Exported, List[js.ParamDef]) = {
1602+
implicit val pos = ctorSym.pos
15971603

15981604
/* `allParams` are the parameters as seen from *inside* the constructor
15991605
* body. the symbols returned in jsParamInfos are the parameters as seen
@@ -1674,39 +1680,60 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
16741680
*/
16751681

16761682
def preStats(tree: ConstructorTree[SplitSecondaryJSCtor],
1677-
nextParams: List[Symbol]): js.Tree = {
1678-
assert(tree.ctor.ctorArgs.size == nextParams.size, "param count mismatch")
1683+
nextParamsAndInfo: List[(js.VarRef, JSParamInfo)]): js.Tree = {
1684+
val inner = tree.subCtors.map(preStats(_, tree.ctor.paramsAndInfo))
16791685

1680-
val inner = tree.subCtors.map(preStats(_, tree.ctor.params))
1686+
assert(tree.ctor.ctorArgs.size == nextParamsAndInfo.size, "param count mismatch")
1687+
val paramsInfosAndArgs = nextParamsAndInfo.zip(tree.ctor.ctorArgs)
16811688

1682-
/* Reject undefined params (i.e. using a default value of another
1683-
* constructor) via implementation restriction.
1684-
*
1685-
* This is mostly for historical reasons. The ideal solution here would
1686-
* be to recognize calls to default param getters of JS class
1687-
* constructors and not even translate them to UndefinedParam in the
1688-
* first place.
1689-
*/
1690-
def isUndefinedParam(tree: js.Tree): Boolean = tree match {
1691-
case js.Transient(UndefinedParam) => true
1692-
case _ => false
1693-
}
1689+
val (captureParamsInfosAndArgs, normalParamsInfosAndArgs) =
1690+
paramsInfosAndArgs.partition(_._1._2.capture)
16941691

1695-
if (tree.ctor.ctorArgs.exists(isUndefinedParam)) {
1696-
reporter.error(tree.ctor.sym.pos,
1697-
"Implementation restriction: in a JS class, a secondary " +
1698-
"constructor calling another constructor with default " +
1699-
"parameters must provide the values of all parameters.")
1692+
val captureAssigns = for {
1693+
((param, _), arg) <- captureParamsInfosAndArgs
1694+
} yield {
1695+
js.Assign(param, arg)
17001696
}
17011697

1702-
val assignments = for {
1703-
(param, arg) <- nextParams.zip(tree.ctor.ctorArgs)
1704-
if !isUndefinedParam(arg)
1698+
val normalAssigns = for {
1699+
(((param, info), arg), i) <- normalParamsInfosAndArgs.zipWithIndex
17051700
} yield {
1706-
js.Assign(genVarRef(param), arg)
1701+
val newArg = arg match {
1702+
case js.Transient(UndefinedParam) =>
1703+
assert(info.hasDefault,
1704+
s"unexpected UndefinedParam for non default param: $param")
1705+
1706+
/* Go full circle: We have ignored the default param getter for
1707+
* this, we'll create it again.
1708+
*
1709+
* This seems not optimal: We could simply not ignore the calls to
1710+
* default param getters in the first place.
1711+
*
1712+
* However, this proves to be difficult: Because of translations in
1713+
* earlier phases, calls to default param getters may be assigned
1714+
* to temporary variables first (see the undefinedDefaultParams
1715+
* ScopedVar). If this happens, it becomes increasingly difficult
1716+
* to distinguish a default param getter call for a constructor
1717+
* call of *this* instance (in which case we would want to keep
1718+
* the default param getter call) from one for a *different*
1719+
* instance (in which case we would want to discard the default
1720+
* param getter call)
1721+
*
1722+
* Because of this, it ends up being easier to just re-create the
1723+
* default param getter call if necessary.
1724+
*/
1725+
genCallDefaultGetter(tree.ctor.sym, i, tree.ctor.sym.pos, static = false,
1726+
captures = captureParamsInfosAndArgs.map(_._1._1))(
1727+
prevArgsCount => normalParamsInfosAndArgs.take(prevArgsCount).map(_._1._1))
1728+
1729+
case arg => arg
1730+
}
1731+
1732+
js.Assign(param, newArg)
17071733
}
17081734

1709-
ifOverload(tree, js.Block(inner ++ tree.ctor.beforeCall ++ assignments))
1735+
ifOverload(tree, js.Block(
1736+
inner ++ tree.ctor.beforeCall ++ captureAssigns ++ normalAssigns))
17101737
}
17111738

17121739
def postStats(tree: ConstructorTree[SplitSecondaryJSCtor]): js.Tree = {
@@ -1718,22 +1745,24 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
17181745
val secondaryCtorTrees = ctorTree.subCtors
17191746

17201747
js.Block(
1721-
secondaryCtorTrees.map(preStats(_, primaryCtor.params)) ++
1748+
secondaryCtorTrees.map(preStats(_, primaryCtor.paramsAndInfo)) ++
17221749
primaryCtor.body ++
17231750
secondaryCtorTrees.map(postStats(_))
17241751
)
17251752
}
17261753

17271754
private sealed trait JSCtor {
17281755
val sym: Symbol
1729-
val params: List[Symbol]
1756+
val paramsAndInfo: List[(js.VarRef, JSParamInfo)]
17301757
}
17311758

17321759
private class PrimaryJSCtor(val sym: Symbol,
1733-
val params: List[Symbol], val body: List[js.Tree]) extends JSCtor
1760+
val paramsAndInfo: List[(js.VarRef, JSParamInfo)],
1761+
val body: List[js.Tree]) extends JSCtor
17341762

17351763
private class SplitSecondaryJSCtor(val sym: Symbol,
1736-
val params: List[Symbol], val beforeCall: List[js.Tree],
1764+
val paramsAndInfo: List[(js.VarRef, JSParamInfo)],
1765+
val beforeCall: List[js.Tree],
17371766
val targetCtor: Symbol, val ctorArgs: List[js.Tree],
17381767
val afterCall: List[js.Tree]) extends JSCtor
17391768

@@ -6768,6 +6797,13 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
67686797
}
67696798
}
67706799

6800+
private def flatStats(stats: List[Tree]): Iterator[Tree] = {
6801+
stats.iterator.flatMap {
6802+
case Block(stats, expr) => stats.iterator ++ Iterator.single(expr)
6803+
case tree => Iterator.single(tree)
6804+
}
6805+
}
6806+
67716807
sealed abstract class MaybeGlobalScope
67726808

67736809
object MaybeGlobalScope {

‎compiler/src/main/scala/org/scalajs/nscplugin/GenJSExports.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
692692
}
693693
}
694694

695-
private def genCallDefaultGetter(sym: Symbol, paramIndex: Int,
695+
def genCallDefaultGetter(sym: Symbol, paramIndex: Int,
696696
paramPos: Position, static: Boolean, captures: List[js.Tree])(
697697
previousArgsValues: Int => List[js.Tree])(
698698
implicit pos: Position): js.Tree = {

‎compiler/src/test/scala/org/scalajs/nscplugin/test/NonNativeJSTypeTest.scala

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -844,20 +844,6 @@ class NonNativeJSTypeTest extends DirectTest with TestHelpers {
844844
"""
845845
}
846846

847-
@Test
848-
def noCallOtherConstructorsWithLeftOutDefaultParams: Unit = {
849-
"""
850-
class A(x: Int, y: String = "default") extends js.Object {
851-
def this() = this(12)
852-
}
853-
""" hasErrors
854-
"""
855-
|newSource1.scala:6: error: Implementation restriction: in a JS class, a secondary constructor calling another constructor with default parameters must provide the values of all parameters.
856-
| def this() = this(12)
857-
| ^
858-
"""
859-
}
860-
861847
@Test // #4511
862848
def noConflictingProperties: Unit = {
863849
"""

‎test-suite/js/src/test/scala/org/scalajs/testsuite/jsinterop/NestedJSClassTest.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,10 @@ class NestedJSClassTest {
436436
// Check that we do not create two companion modules.
437437
new container.InnerJSClassDefaultParams_Issue4465()()
438438
assertEquals(1, container.moduleSideEffect)
439+
440+
// Check constructor delegation.
441+
val inner2 = new container.InnerJSClassDefaultParams_Issue4465(1)
442+
assertEquals("container 1 1 foo", inner2.foo())
439443
}
440444

441445
@Test def defaultCtorParamsInnerJSClassTraitContainer_Issue4465(): Unit = {
@@ -449,6 +453,10 @@ class NestedJSClassTest {
449453
// Check that we do not create two companion modules.
450454
new container.InnerJSClassDefaultParams_Issue4465()()
451455
assertEquals(1, container.moduleSideEffect)
456+
457+
// Check constructor delegation.
458+
val inner2 = new container.InnerJSClassDefaultParams_Issue4465(1)
459+
assertEquals("container 1 1 foo", inner2.foo())
452460
}
453461

454462
@Test def defaultCtorParamsInnerJSClassJSContainer_Issue4465(): Unit = {
@@ -467,6 +475,10 @@ class NestedJSClassTest {
467475

468476
// Check that we do not create two companion modules.
469477
assertEquals(1, container.moduleSideEffect)
478+
479+
// Check constructor delegation.
480+
val inner2 = new container.InnerJSClassDefaultParams_Issue4465(1)
481+
assertEquals("container 1 1 foo", inner2.foo())
470482
}
471483

472484
@Test def defaultCtorParamsInnerJSClassPrivateCompanion_Issue4526(): Unit = {
@@ -696,6 +708,8 @@ object NestedJSClassTest {
696708

697709
class InnerJSClassDefaultParams_Issue4465(withDefault: String = "inner")(
698710
dependentDefault: String = withDefault) extends js.Object {
711+
def this(x: Int) = this(x.toString)()
712+
699713
def foo(methodDefault: String = "foo"): String =
700714
s"$xxx $withDefault $dependentDefault $methodDefault"
701715
}
@@ -739,6 +753,8 @@ object NestedJSClassTest {
739753

740754
class InnerJSClassDefaultParams_Issue4465(withDefault: String = "inner")(
741755
dependentDefault: String = withDefault) extends js.Object {
756+
def this(x: Int) = this(x.toString)()
757+
742758
def foo(methodDefault: String = "foo"): String =
743759
s"$xxx $withDefault $dependentDefault $methodDefault"
744760
}
@@ -846,6 +862,8 @@ object NestedJSClassTest {
846862

847863
class InnerJSClassDefaultParams_Issue4465(withDefault: String = "inner")(
848864
dependentDefault: String = withDefault) extends js.Object {
865+
def this(x: Int) = this(x.toString)()
866+
849867
def foo(methodDefault: String = "foo"): String =
850868
s"$xxx $withDefault $dependentDefault $methodDefault"
851869
}

‎test-suite/js/src/test/scala/org/scalajs/testsuite/jsinterop/NonNativeJSTypeTest.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,14 @@ class NonNativeJSTypeTest {
10181018
assertEquals(7, baz10.bar)
10191019
}
10201020

1021+
@Test def secondaryConstructorUseDefaultParam(): Unit = {
1022+
val a = new SecondaryConstructorUseDefaultParam(1)
1023+
assertEquals(a.y, "1y")
1024+
1025+
val b = new SecondaryConstructorUseDefaultParam()()
1026+
assertEquals(b.y, "xy")
1027+
}
1028+
10211029
@Test def polytypeNullaryMethod_Issue2445(): Unit = {
10221030
class PolyTypeNullaryMethod extends js.Object {
10231031
def emptyArray[T]: js.Array[T] = js.Array()
@@ -2003,6 +2011,10 @@ object NonNativeJSTypeTest {
20032011
this((a + b).length, (x + y).length)
20042012
}
20052013

2014+
class SecondaryConstructorUseDefaultParam(x: String = "x")(val y: String = x + "y") extends js.Object {
2015+
def this(x: Int) = this(x.toString())()
2016+
}
2017+
20062018
class SimpleConstructorAutoFields(val x: Int, var y: Int) extends js.Object {
20072019
def sum(): Int = x + y
20082020
}

0 commit comments

Comments
 (0)