diff --git a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala index c0cf4118c997..b41a8b8f1648 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala @@ -1067,7 +1067,8 @@ class JSCodeGen()(using genCtx: Context) { val jsClassCaptures = List.newBuilder[js.ParamDef] def add(tree: ConstructorTree[_ <: JSCtor]): Unit = { - val (e, c) = genJSClassCtorDispatch(tree.ctor.sym, tree.ctor.params, tree.overloadNum) + val (e, c) = genJSClassCtorDispatch(tree.ctor.sym, + tree.ctor.paramsAndInfo, tree.overloadNum) exports += e jsClassCaptures ++= c tree.subCtors.foreach(add(_)) @@ -1133,16 +1134,19 @@ class JSCodeGen()(using genCtx: Context) { assert(jsSuperCall.isDefined, s"Did not find Super call in primary JS construtor at ${dd.sourcePos}") - val params = dd.paramss.flatten.map(_.symbol) - - new PrimaryJSCtor(sym, params, jsSuperCall.get :: jsStats.result()) + new PrimaryJSCtor(sym, genParamsAndInfo(sym, dd.paramss), jsSuperCall.get :: jsStats.result()) } private def genSecondaryJSClassCtor(dd: DefDef): SplitSecondaryJSCtor = { val sym = dd.symbol - val Block(stats, _) = dd.rhs assert(!sym.isPrimaryConstructor, s"called with primary ctor $sym") + def flattenBlocks(t: Tree): List[Tree] = t match { + case Block(stats, expr) => (stats :+ expr).flatMap(flattenBlocks) + case _ => t :: Nil + } + val stats = flattenBlocks(dd.rhs) + val beforeThisCall = List.newBuilder[js.Tree] var thisCall: Option[(Symbol, List[js.Tree])] = None val afterThisCall = List.newBuilder[js.Tree] @@ -1167,24 +1171,33 @@ class JSCodeGen()(using genCtx: Context) { } } + assert(thisCall.isDefined, + i"could not find the this() call in secondary JS constructor at ${dd.sourcePos}:\n${stats.map(_.show).mkString("\n")}") val Some((targetCtor, ctorArgs)) = thisCall - val params = dd.paramss.flatten.map(_.symbol) + new SplitSecondaryJSCtor(sym, genParamsAndInfo(sym, dd.paramss), + beforeThisCall.result(), targetCtor, ctorArgs, afterThisCall.result()) + } + + private def genParamsAndInfo(ctorSym: Symbol, + vparamss: List[ParamClause]): List[(Symbol, JSParamInfo)] = { + implicit val pos: SourcePosition = ctorSym.sourcePos - new SplitSecondaryJSCtor(sym, params, beforeThisCall.result(), targetCtor, - ctorArgs, afterThisCall.result()) + val paramSyms = if (vparamss.isEmpty) Nil else vparamss.head.map(_.symbol) + paramSyms.zip(ctorSym.jsParamInfos) } - private def genJSClassCtorDispatch(sym: Symbol, allParams: List[Symbol], + private def genJSClassCtorDispatch(ctorSym: Symbol, + allParamsAndInfos: List[(Symbol, JSParamInfo)], overloadNum: Int): (jsExportsGen.Exported, List[js.ParamDef]) = { - implicit val pos: SourcePosition = sym.sourcePos + implicit val pos: SourcePosition = ctorSym.sourcePos /* `allParams` are the parameters as seen from inside the constructor body, * i.e., the ones generated by the trees in the constructor body. */ val (captureParamsAndInfos, normalParamsAndInfos) = - allParams.zip(sym.jsParamInfos).partition(_._2.capture) + allParamsAndInfos.partition(_._2.capture) /* For class captures, we need to generate different names than the ones * used by the constructor body. This is necessary so that we can forward @@ -1203,13 +1216,14 @@ class JSCodeGen()(using genCtx: Context) { val normalInfos = normalParamsAndInfos.map(_._2).toIndexedSeq - val jsExport = new jsExportsGen.Exported(sym, normalInfos) { + val jsExport = new jsExportsGen.Exported(ctorSym, normalInfos) { def genBody(formalArgsRegistry: jsExportsGen.FormalArgsRegistry): js.Tree = { val paramAssigns = for { ((param, info), i) <- normalParamsAndInfos.zipWithIndex } yield { - val rhs = jsExportsGen.genScalaArg(this, i, formalArgsRegistry, info, static = true)( - prevArgsCount => allParams.take(prevArgsCount).map(genVarRef(_))) + val rhs = jsExportsGen.genScalaArg(this, i, formalArgsRegistry, info, static = true, + captures = captureParamsAndInfos.map(pi => genVarRef(pi._1)))( + prevArgsCount => normalParamsAndInfos.take(prevArgsCount).map(pi => genVarRef(pi._1))) js.Assign(genVarRef(param), rhs) } @@ -1256,40 +1270,58 @@ class JSCodeGen()(using genCtx: Context) { */ def preStats(tree: ConstructorTree[SplitSecondaryJSCtor], - nextParams: List[Symbol]): js.Tree = { - assert(tree.ctor.ctorArgs.size == nextParams.size, "param count mismatch") + nextParamsAndInfo: List[(Symbol, JSParamInfo)]): js.Tree = { + val inner = tree.subCtors.map(preStats(_, tree.ctor.paramsAndInfo)) - val inner = tree.subCtors.map(preStats(_, tree.ctor.params)) + assert(tree.ctor.ctorArgs.size == nextParamsAndInfo.size, "param count mismatch") + val paramsInfosAndArgs = nextParamsAndInfo.zip(tree.ctor.ctorArgs) - /* Reject undefined params (i.e. using a default value of another - * constructor) via implementation restriction. - * - * This is mostly for historical reasons. The ideal solution here would - * be to recognize calls to default param getters of JS class - * constructors and not even translate them to UndefinedParam in the - * first place. - */ - def isUndefinedParam(tree: js.Tree): Boolean = tree match { - case js.Transient(UndefinedParam) => true - case _ => false - } + val (captureParamsInfosAndArgs, normalParamsInfosAndArgs) = + paramsInfosAndArgs.partition(_._1._2.capture) - if (tree.ctor.ctorArgs.exists(isUndefinedParam)) { - report.error( - "Implementation restriction: " + - "in a JS class, a secondary constructor calling another constructor " + - "with default parameters must provide the values of all parameters.", - tree.ctor.sym.sourcePos) + val captureAssigns = for { + ((param, _), arg) <- captureParamsInfosAndArgs + } yield { + js.Assign(genVarRef(param), arg) } - val assignments = for { - (param, arg) <- nextParams.zip(tree.ctor.ctorArgs) - if !isUndefinedParam(arg) + val normalAssigns = for { + (((param, info), arg), i) <- normalParamsInfosAndArgs.zipWithIndex } yield { - js.Assign(genVarRef(param), arg) + val newArg = arg match { + case js.Transient(UndefinedParam) => + /* Go full circle: We have ignored the default param getter for + * this, we'll create it again. + * + * This seems not optimal: We could simply not ignore the calls to + * default param getters in the first place. + * + * However, this proves to be difficult: Because of translations in + * earlier phases, calls to default param getters may be assigned + * to temporary variables first (see the undefinedDefaultParams + * ScopedVar). If this happens, it becomes increasingly difficult + * to distinguish a default param getter call for a constructor + * call of *this* instance (in which case we would want to keep + * the default param getter call) from one for a *different* + * instance (in which case we would want to discard the default + * param getter call) + * + * Because of this, it ends up being easier to just re-create the + * default param getter call if necessary. + */ + implicit val pos: SourcePosition = tree.ctor.sym.sourcePos + jsExportsGen.genCallDefaultGetter(tree.ctor.sym, i, static = false, + captures = captureParamsInfosAndArgs.map(p => genVarRef(p._1._1)))( + prevArgsCount => normalParamsInfosAndArgs.take(prevArgsCount).map(p => genVarRef(p._1._1))) + + case arg => arg + } + + js.Assign(genVarRef(param), newArg) } - ifOverload(tree, js.Block(inner ++ tree.ctor.beforeCall ++ assignments)) + ifOverload(tree, js.Block( + inner ++ tree.ctor.beforeCall ++ captureAssigns ++ normalAssigns)) } def postStats(tree: ConstructorTree[SplitSecondaryJSCtor]): js.Tree = { @@ -1301,7 +1333,7 @@ class JSCodeGen()(using genCtx: Context) { val secondaryCtorTrees = ctorTree.subCtors js.Block( - secondaryCtorTrees.map(preStats(_, primaryCtor.params)) ++ + secondaryCtorTrees.map(preStats(_, primaryCtor.paramsAndInfo)) ++ primaryCtor.body ++ secondaryCtorTrees.map(postStats(_)) ) @@ -1309,14 +1341,16 @@ class JSCodeGen()(using genCtx: Context) { private sealed trait JSCtor { val sym: Symbol - val params: List[Symbol] + val paramsAndInfo: List[(Symbol, JSParamInfo)] } private class PrimaryJSCtor(val sym: Symbol, - val params: List[Symbol], val body: List[js.Tree]) extends JSCtor + val paramsAndInfo: List[(Symbol, JSParamInfo)], + val body: List[js.Tree]) extends JSCtor private class SplitSecondaryJSCtor(val sym: Symbol, - val params: List[Symbol], val beforeCall: List[js.Tree], + val paramsAndInfo: List[(Symbol, JSParamInfo)], + val beforeCall: List[js.Tree], val targetCtor: Symbol, val ctorArgs: List[js.Tree], val afterCall: List[js.Tree]) extends JSCtor @@ -3158,7 +3192,7 @@ class JSCodeGen()(using genCtx: Context) { case resType => resType } - var clauses: List[(List[js.Tree], js.Tree)] = Nil + var clauses: List[(List[js.MatchableLiteral], js.Tree)] = Nil var optDefaultClause: Option[js.Tree] = None for (caze @ CaseDef(pat, guard, body) <- cases) { @@ -3167,19 +3201,29 @@ class JSCodeGen()(using genCtx: Context) { val genBody = genStatOrExpr(body, isStat) + def invalidCase(): Nothing = + abortMatch("Invalid case") + + def genMatchableLiteral(tree: Literal): js.MatchableLiteral = { + genExpr(tree) match { + case matchableLiteral: js.MatchableLiteral => matchableLiteral + case otherExpr => invalidCase() + } + } + pat match { case lit: Literal => - clauses = (List(genExpr(lit)), genBody) :: clauses + clauses = (List(genMatchableLiteral(lit)), genBody) :: clauses case Ident(nme.WILDCARD) => optDefaultClause = Some(genBody) case Alternative(alts) => val genAlts = alts.map { - case lit: Literal => genExpr(lit) - case _ => abortMatch("Invalid case in alternative") + case lit: Literal => genMatchableLiteral(lit) + case _ => invalidCase() } clauses = (genAlts, genBody) :: clauses case _ => - abortMatch("Invalid case pattern") + invalidCase() } } @@ -3194,10 +3238,6 @@ class JSCodeGen()(using genCtx: Context) { * case is a typical product of `match`es that are full of * `case n if ... =>`, which are used instead of `if` chains for * convenience and/or readability. - * - * When no optimization applies, and any of the case values is not a - * literal int, we emit a series of `if..else` instead of a `js.Match`. - * This became necessary in 2.13.2 with strings and nulls. */ def isInt(tree: js.Tree): Boolean = tree.tpe == jstpe.IntType @@ -3217,32 +3257,8 @@ class JSCodeGen()(using genCtx: Context) { js.If(js.BinaryOp(op, genSelector, uniqueAlt), caseRhs, defaultClause)(resultType) case _ => - if (isInt(genSelector) && - clauses.forall(_._1.forall(_.isInstanceOf[js.IntLiteral]))) { - // We have int literals only: use a js.Match - val intClauses = clauses.asInstanceOf[List[(List[js.IntLiteral], js.Tree)]] - js.Match(genSelector, intClauses, defaultClause)(resultType) - } else { - // We have other stuff: generate an if..else chain - val (tempSelectorDef, tempSelectorRef) = genSelector match { - case varRef: js.VarRef => - (js.Skip(), varRef) - case _ => - val varDef = js.VarDef(freshLocalIdent(), NoOriginalName, - genSelector.tpe, mutable = false, genSelector) - (varDef, varDef.ref) - } - val ifElseChain = clauses.foldRight(defaultClause) { (caze, elsep) => - val conds = caze._1.map { caseValue => - js.BinaryOp(js.BinaryOp.===, tempSelectorRef, caseValue) - } - val cond = conds.reduceRight[js.Tree] { (left, right) => - js.If(left, js.BooleanLiteral(true), right)(jstpe.BooleanType) - } - js.If(cond, caze._2, elsep)(resultType) - } - js.Block(tempSelectorDef, ifElseChain) - } + // We have more than one case: use a js.Match + js.Match(genSelector, clauses, defaultClause)(resultType) } } @@ -3511,7 +3527,7 @@ class JSCodeGen()(using genCtx: Context) { } /** Gen a statically linked call to an instance method. */ - private def genApplyMethodMaybeStatically(receiver: js.Tree, method: Symbol, + def genApplyMethodMaybeStatically(receiver: js.Tree, method: Symbol, arguments: List[js.Tree])(implicit pos: Position): js.Tree = { if (method.isPrivate || method.isClassConstructor) genApplyMethodStatically(receiver, method, arguments) diff --git a/compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala b/compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala index 22aafc95e11b..ab2211b7073b 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala @@ -13,6 +13,7 @@ import Denotations._ import Flags._ import Names._ import NameKinds.DefaultGetterName +import NameOps._ import Periods._ import Phases._ import StdNames._ @@ -350,14 +351,8 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { val (getter, setters) = alts.partition(_.info.paramInfoss.head.isEmpty) // We can have at most one getter - if (getter.sizeIs > 1) { - /* Member export of properties should be caught earlier, so if we get - * here with a non-static export, something went horribly wrong. - */ - assert(static, s"Found more than one instance getter to export for name $jsName.") - for (duplicate <- getter.tail) - report.error(s"Duplicate static getter export with name '${jsName.displayName}'", duplicate) - } + if (getter.sizeIs > 1) + reportCannotDisambiguateError(jsName, alts) val getterBody = getter.headOption.map { getterSym => genApplyForSingleExported(new FormalArgsRegistry(0, false), new ExportedSymbol(getterSym, static), static) @@ -535,7 +530,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { // 2. The optional argument count restriction has triggered // 3. We only have (more than once) repeated parameters left // Therefore, we should fail - reportCannotDisambiguateError(jsName, alts) + reportCannotDisambiguateError(jsName, alts.map(_.sym)) js.Undefined() } else { val altsByTypeTest = groupByWithoutHashCode(alts) { exported => @@ -597,7 +592,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { } } - private def reportCannotDisambiguateError(jsName: JSName, alts: List[Exported]): Unit = { + private def reportCannotDisambiguateError(jsName: JSName, alts: List[Symbol]): Unit = { val currentClass = currentClassSym.get /* Find a position that is in the current class for decent error reporting. @@ -606,21 +601,26 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { * same error in all compilers. */ val validPositions = alts.collect { - case alt if alt.sym.owner == currentClass => alt.pos + case alt if alt.owner == currentClass => alt.sourcePos } val pos: SourcePosition = if (validPositions.isEmpty) currentClass.sourcePos else validPositions.maxBy(_.point) val kind = - if (currentClass.isJSType) "method" - else "exported method" + if (alts.head.isJSGetter) "getter" + else if (alts.head.isJSSetter) "setter" + else "method" + + val fullKind = + if (currentClass.isJSType) kind + else "exported " + kind val displayName = jsName.displayName - val altsTypesInfo = alts.map(_.typeInfo).mkString("\n ") + val altsTypesInfo = alts.map(_.info.show).sorted.mkString("\n ") report.error( - s"Cannot disambiguate overloads for $kind $displayName with types\n $altsTypesInfo", + s"Cannot disambiguate overloads for $fullKind $displayName with types\n $altsTypesInfo", pos) } @@ -682,7 +682,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { val varDefs = new mutable.ListBuffer[js.VarDef] for ((param, i) <- exported.params.zipWithIndex) { - val rhs = genScalaArg(exported, i, formalArgsRegistry, param, static)( + val rhs = genScalaArg(exported, i, formalArgsRegistry, param, static, captures = Nil)( prevArgsCount => varDefs.take(prevArgsCount).toList.map(_.ref)) varDefs += js.VarDef(freshLocalIdent("prep" + i), NoOriginalName, rhs.tpe, mutable = false, rhs) @@ -699,7 +699,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { * (unboxing and default parameter handling). */ def genScalaArg(exported: Exported, paramIndex: Int, formalArgsRegistry: FormalArgsRegistry, - param: JSParamInfo, static: Boolean)( + param: JSParamInfo, static: Boolean, captures: List[js.Tree])( previousArgsValues: Int => List[js.Tree])( implicit pos: SourcePosition): js.Tree = { @@ -714,7 +714,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { if (exported.hasDefaultAt(paramIndex)) { // If argument is undefined and there is a default getter, call it js.If(js.BinaryOp(js.BinaryOp.===, jsArg, js.Undefined()), { - genCallDefaultGetter(exported.sym, paramIndex, static)(previousArgsValues) + genCallDefaultGetter(exported.sym, paramIndex, static, captures)(previousArgsValues) }, { unboxedArg })(unboxedArg.tpe) @@ -725,7 +725,8 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { } } - private def genCallDefaultGetter(sym: Symbol, paramIndex: Int, static: Boolean)( + def genCallDefaultGetter(sym: Symbol, paramIndex: Int, + static: Boolean, captures: List[js.Tree])( previousArgsValues: Int => List[js.Tree])( implicit pos: SourcePosition): js.Tree = { @@ -736,9 +737,33 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) { assert(!defaultGetterDenot.isOverloaded, i"found overloaded default getter $defaultGetterDenot") val defaultGetter = defaultGetterDenot.symbol - val targetTree = - if (sym.isClassConstructor || static) genLoadModule(targetSym) - else js.This()(encodeClassType(targetSym)) + val targetTree = { + if (sym.isClassConstructor || static) { + if (targetSym.isStatic) { + assert(captures.isEmpty, i"expected empty captures for ${targetSym.fullName} at $pos") + genLoadModule(targetSym) + } else { + assert(captures.sizeIs == 1, "expected exactly one capture") + + // Find the module accessor. We cannot use memberBasedOnFlags because of scala-js/scala-js#4526. + val outer = targetSym.originalOwner + val name = atPhase(typerPhase)(targetSym.name.unexpandedName).sourceModuleName + val modAccessor = outer.info.allMembers.find { denot => + denot.symbol.is(Module) && denot.name.unexpandedName == name + }.getOrElse { + throw new AssertionError(i"could not find module accessor for ${targetSym.fullName} at $pos") + }.symbol + + val receiver = captures.head + if (outer.isJSType) + genApplyJSClassMethod(receiver, modAccessor, Nil) + else + genApplyMethodMaybeStatically(receiver, modAccessor, Nil) + } + } else { + js.This()(encodeClassType(targetSym)) + } + } // Pass previous arguments to defaultGetter val defaultGetterArgs = previousArgsValues(defaultGetter.info.paramInfoss.head.size) diff --git a/project/plugins.sbt b/project/plugins.sbt index 5a49901967ca..861d9491e13e 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -2,7 +2,7 @@ // // e.g. addSbtPlugin("com.github.mpeltonen" % "sbt-idea" % "1.1.0") -addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.6.0") +addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.7.0") addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "3.6") diff --git a/tests/neg-scalajs/js-non-native-members-conflicts.check b/tests/neg-scalajs/js-non-native-members-conflicts.check new file mode 100644 index 000000000000..011b52c93be8 --- /dev/null +++ b/tests/neg-scalajs/js-non-native-members-conflicts.check @@ -0,0 +1,12 @@ +-- Error: tests/neg-scalajs/js-non-native-members-conflicts.scala:7:6 -------------------------------------------------- +7 | def b: Unit = () // error + | ^ + | Cannot disambiguate overloads for getter a with types + | (): Unit + | (): Unit +-- Error: tests/neg-scalajs/js-non-native-members-conflicts.scala:10:2 ------------------------------------------------- +10 | object B1 // error + | ^ + | Cannot disambiguate overloads for getter B1 with types + | (): A.this.B1 + | (): Object diff --git a/tests/neg-scalajs/js-non-native-members-conflicts.scala b/tests/neg-scalajs/js-non-native-members-conflicts.scala new file mode 100644 index 000000000000..d3c44c489f15 --- /dev/null +++ b/tests/neg-scalajs/js-non-native-members-conflicts.scala @@ -0,0 +1,11 @@ +import scala.scalajs.js +import scala.scalajs.js.annotation.* + +class A extends js.Object { + def a: Unit = () + @JSName("a") + def b: Unit = () // error + + class B1 extends js.Object + object B1 // error +}