From dba8f43cd5783f91bb22995da818dbcd9e26da8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Mon, 11 Oct 2021 18:05:46 +0200 Subject: [PATCH 1/5] Remove the last occurrence of the legacy JSInterop internal API. Now, everything goes through the extension methods in JSSymUtils. --- .../dotty/tools/backend/sjs/JSCodeGen.scala | 3 +- .../dotty/tools/backend/sjs/JSInterop.scala | 74 ------------------- 2 files changed, 1 insertion(+), 76 deletions(-) delete mode 100644 compiler/src/dotty/tools/backend/sjs/JSInterop.scala diff --git a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala index 1f707bcd17d9..e27da4a1edf4 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala @@ -42,7 +42,6 @@ import org.scalajs.ir.Trees.OptimizerHints import dotty.tools.dotc.transform.sjs.JSSymUtils._ import JSEncoding._ -import JSInterop._ import ScopedVar.withScopedVars /** Main codegen for Scala.js IR. @@ -526,7 +525,7 @@ class JSCodeGen()(using genCtx: Context) { /* We add symbols that we have to expose here. This way we also * get inherited stuff that is implemented in this class. */ - dispatchMethodNames += jsNameOf(sym) + dispatchMethodNames += sym.jsName } } diff --git a/compiler/src/dotty/tools/backend/sjs/JSInterop.scala b/compiler/src/dotty/tools/backend/sjs/JSInterop.scala deleted file mode 100644 index c22af60bd179..000000000000 --- a/compiler/src/dotty/tools/backend/sjs/JSInterop.scala +++ /dev/null @@ -1,74 +0,0 @@ -package dotty.tools.backend.sjs - -import dotty.tools.dotc.core._ -import Contexts._ -import Flags._ -import Symbols._ -import NameOps._ -import StdNames._ -import Phases._ - -import dotty.tools.dotc.transform.sjs.JSSymUtils._ - -/** Management of the interoperability with JavaScript. - * - * This object only contains forwarders for extension methods in - * `transform.sjs.JSSymUtils`. They are kept to minimize changes in - * `JSCodeGen` in the short term, but it will eventually be removed. - */ -object JSInterop { - - /** Is this symbol a JavaScript type? */ - def isJSType(sym: Symbol)(using Context): Boolean = - sym.isJSType - - /** Should this symbol be translated into a JS getter? - * - * This is true for any parameterless method, i.e., defined without `()`. - * Unlike `SymDenotations.isGetter`, it applies to user-defined methods as - * much as *accessor* methods created for `val`s and `var`s. - */ - def isJSGetter(sym: Symbol)(using Context): Boolean = - sym.isJSGetter - - /** Should this symbol be translated into a JS setter? - * - * This is true for any method whose name ends in `_=`. - * Unlike `SymDenotations.isGetter`, it applies to user-defined methods as - * much as *accessor* methods created for `var`s. - */ - def isJSSetter(sym: Symbol)(using Context): Boolean = - sym.isJSSetter - - /** Should this symbol be translated into a JS bracket access? - * - * This is true for methods annotated with `@JSBracketAccess`. - */ - def isJSBracketAccess(sym: Symbol)(using Context): Boolean = - sym.isJSBracketAccess - - /** Should this symbol be translated into a JS bracket call? - * - * This is true for methods annotated with `@JSBracketCall`. - */ - def isJSBracketCall(sym: Symbol)(using Context): Boolean = - sym.isJSBracketCall - - /** Is this symbol a default param accessor for a JS method? - * - * For default param accessors of *constructors*, we need to test whether - * the companion *class* of the owner is a JS type; not whether the owner - * is a JS type. - */ - def isJSDefaultParam(sym: Symbol)(using Context): Boolean = - sym.isJSDefaultParam - - /** Gets the unqualified JS name of a symbol. - * - * If it is not explicitly specified with an `@JSName` annotation, the - * JS name is inferred from the Scala name. - */ - def jsNameOf(sym: Symbol)(using Context): JSName = - sym.jsName - -} From db67c68acf3c8ccae5dbfb76aa5021734b3e166f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Mon, 11 Oct 2021 18:20:55 +0200 Subject: [PATCH 2/5] Scala.js: Correctly identify default accessors of native JS def params. Previously, we tested whether the default accessor itself had the `@js.native` annotation. That is however never the case. Instead, we now test whether the associated method has the annotation. We still emit the *definitions* of those default accessors, even though that they are never called anymore, because of backward binary compatibility. If we stopped emitting them, a library compiled with a newer Scala.js might not link anymore against an application built with a previous Scala.js. This is a forward port of https://github.com/scala-js/scala-js/commit/7e998b40f7ad5933233aafee6d7cb628aab7f97d --- .../dotty/tools/backend/sjs/JSCodeGen.scala | 67 ++++++++++++++++++- .../tools/dotc/transform/sjs/JSSymUtils.scala | 20 ------ 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala index e27da4a1edf4..7b1ff2d4738b 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala @@ -2007,8 +2007,46 @@ class JSCodeGen()(using genCtx: Context) { val args = tree.args val sym = tree.fun.symbol + /* Is the method a JS default accessor, which should become an + * `UndefinedParam` rather than being compiled normally. + * + * This is true iff one of the following conditions apply: + * - It is a constructor default param for the constructor of a JS class. + * - It is a default param of an instance method of a native JS type. + * - It is a default param of an instance method of a non-native JS type + * and the attached method is exposed. + * - It is a default param for a native JS def. + */ + def isJSDefaultParam: Boolean = { + sym.name.is(DefaultGetterName) && { + val info = new DefaultParamInfo(sym) + if (info.isForConstructor) { + /* This is a default accessor for a constructor parameter. Check + * whether the attached constructor is a JS constructor, which is + * the case iff the linked class is a JS type. + */ + info.constructorOwner.isJSType + } else { + if (sym.owner.isJSType) { + /* The default accessor is in a JS type. It is a JS default + * param iff the enclosing class is native or the attached method + * is exposed. + */ + !sym.owner.isNonNativeJSClass || info.attachedMethod.isJSExposed + } else { + /* The default accessor is in a Scala type. It is a JS default + * param iff the attached method is a native JS def. This can + * only happen if the owner is a module class, which we test + * first as a fast way out. + */ + sym.owner.is(ModuleClass) && info.attachedMethod.hasAnnotation(jsdefn.JSNativeAnnot) + } + } + } + } + tree.fun match { - case _ if sym.isJSDefaultParam => + case _ if isJSDefaultParam => js.Transient(UndefinedParam) case Select(Super(_, _), _) => @@ -4578,4 +4616,31 @@ object JSCodeGen { out.print("") } + /** Info about a default param accessor. + * + * The method must have a default getter name for this class to make sense. + */ + private class DefaultParamInfo(sym: Symbol)(using Context) { + private val methodName = sym.name.exclude(DefaultGetterName) + + def isForConstructor: Boolean = methodName == nme.CONSTRUCTOR + + /** When `isForConstructor` is true, returns the owner of the attached + * constructor. + */ + def constructorOwner: Symbol = sym.owner.linkedClass + + /** When `isForConstructor` is false, returns the method attached to the + * specified default accessor. + */ + def attachedMethod: Symbol = { + // If there are overloads, we need to find the one that has default params. + val overloads = sym.owner.info.decl(methodName) + if (!overloads.isOverloaded) + overloads.symbol + else + overloads.suchThat(_.is(HasDefaultParams)).symbol + } + } + } diff --git a/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala b/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala index f718d68e9588..58d93703ffc2 100644 --- a/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala @@ -156,26 +156,6 @@ object JSSymUtils { def isJSBracketCall(using Context): Boolean = sym.hasAnnotation(jsdefn.JSBracketCallAnnot) - /** Is this symbol a default param accessor for a JS method? - * - * For default param accessors of *constructors*, we need to test whether - * the companion *class* of the owner is a JS type; not whether the owner - * is a JS type. - */ - def isJSDefaultParam(using Context): Boolean = { - sym.name.is(DefaultGetterName) && { - val owner = sym.owner - val methName = sym.name.exclude(DefaultGetterName) - if (methName == nme.CONSTRUCTOR) { - owner.linkedClass.isJSType - } else { - def isAttachedMethodExposed: Boolean = - owner.info.decl(methName).hasAltWith(_.symbol.isJSExposed) - owner.isJSType && (!owner.isNonNativeJSClass || isAttachedMethodExposed) - } - } - } - /** Is this symbol a default param accessor for the constructor of a native JS class? */ def isJSNativeCtorDefaultParam(using Context): Boolean = { sym.name.is(DefaultGetterName) From 0fb52beae1ce504f84f0b47a03dd4433347ae7b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Mon, 11 Oct 2021 18:26:59 +0200 Subject: [PATCH 3/5] Scala.js: Allow default params of JS native defs to be `= js.native`. The default accessors matching that property are not emitted anymore. This is similar to how default accessors for JS native constructors are not emitted. Unfortunately, for backward binary compatibility reasons, we must still emit default accessors of JS native defs that are anything else than `= js.native`. Until Scala.js 1.7.0, those default accessors would actually be referenced by calling code (see https://github.com/scala-js/scala-js/issues/4554), so removing them now would create dangling references. This is a forward port of https://github.com/scala-js/scala-js/commit/e01dc1adcbc982860ff2248a3ae88901e95fca92 --- .../dotty/tools/backend/sjs/JSCodeGen.scala | 55 ++++++++++++++++++- .../tools/dotc/transform/sjs/JSSymUtils.scala | 7 --- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala index 7b1ff2d4738b..5d9646e239e6 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala @@ -1402,6 +1402,53 @@ class JSCodeGen()(using genCtx: Context) { val vparamss = dd.termParamss val rhs = dd.rhs + /* Is this method a default accessor that should be ignored? + * + * This is the case iff one of the following applies: + * - It is a constructor default accessor and the linked class is a + * native JS class. + * - It is a default accessor for a native JS def, but with the caveat + * that its rhs must be `js.native` because of #4553. + * + * Both of those conditions can only happen if the default accessor is in + * a module class, so we use that as a fast way out. (But omitting that + * condition would not change the result.) + * + * This is different than `isJSDefaultParam` in `genApply`: we do not + * ignore default accessors of *non-native* JS types. Neither for + * constructor default accessor nor regular default accessors. We also + * do not need to worry about non-constructor members of native JS types, + * since for those, the entire member list is ignored in `genJSClassData`. + */ + def isIgnorableDefaultParam: Boolean = { + sym.name.is(DefaultGetterName) && sym.owner.is(ModuleClass) && { + val info = new DefaultParamInfo(sym) + if (info.isForConstructor) { + /* This is a default accessor for a constructor parameter. Check + * whether the attached constructor is a native JS constructor, + * which is the case iff the linked class is a native JS type. + */ + info.constructorOwner.hasAnnotation(jsdefn.JSNativeAnnot) + } else { + /* #4553 We need to ignore default accessors for JS native defs. + * However, because Scala.js <= 1.7.0 actually emitted code calling + * those accessors, we must keep default accessors that would + * compile. The only accessors we can actually get rid of are those + * that are `= js.native`. + */ + !sym.owner.isJSType && + info.attachedMethod.hasAnnotation(jsdefn.JSNativeAnnot) && { + dd.rhs match { + case MaybeAsInstanceOf(Apply(fun, _)) => + fun.symbol == jsdefn.JSPackage_native + case _ => + false + } + } + } + } + } + withPerMethodBodyState(sym) { assert(vparamss.isEmpty || vparamss.tail.isEmpty, "Malformed parameter list: " + vparamss) @@ -1421,7 +1468,7 @@ class JSCodeGen()(using genCtx: Context) { Some(js.MethodDef(js.MemberFlags.empty, methodName, originalName, jsParams, toIRType(patchedResultType(sym)), None)( OptimizerHints.empty, None)) - } else if (sym.isJSNativeCtorDefaultParam) { + } else if (isIgnorableDefaultParam) { // #11592 None } else if (sym.is(Bridge) && sym.name.is(DefaultGetterName) && currentClassSym.isNonNativeJSClass) { @@ -2016,6 +2063,12 @@ class JSCodeGen()(using genCtx: Context) { * - It is a default param of an instance method of a non-native JS type * and the attached method is exposed. * - It is a default param for a native JS def. + * + * This is different than `isIgnorableDefaultParam` in + * `genMethodWithCurrentLocalNameScope`: we include here the default + * accessors of *non-native* JS types (unless the corresponding methods are + * not exposed). We also need to handle non-constructor members of native + * JS types. */ def isJSDefaultParam: Boolean = { sym.name.is(DefaultGetterName) && { diff --git a/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala b/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala index 58d93703ffc2..a78adaff6522 100644 --- a/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala @@ -156,13 +156,6 @@ object JSSymUtils { def isJSBracketCall(using Context): Boolean = sym.hasAnnotation(jsdefn.JSBracketCallAnnot) - /** Is this symbol a default param accessor for the constructor of a native JS class? */ - def isJSNativeCtorDefaultParam(using Context): Boolean = { - sym.name.is(DefaultGetterName) - && sym.name.exclude(DefaultGetterName) == nme.CONSTRUCTOR - && sym.owner.linkedClass.hasAnnotation(jsdefn.JSNativeAnnot) - } - def jsCallingConvention(using Context): JSCallingConvention = JSCallingConvention.of(sym) From b1bb0df9fd238365c66e2d2db11899ecb3f2f55c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 12 Oct 2021 17:47:00 +0200 Subject: [PATCH 4/5] Scala.js: Do not emit static forwarders for native JS defs. Nor for their default param accessors. This is a forward port of https://github.com/scala-js/scala-js/commit/a95f3f6ab4d844fe44ba601a57335e748f1d07d2 --- .../src/dotty/tools/backend/sjs/JSCodeGen.scala | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala index 5d9646e239e6..53885ba523ec 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala @@ -789,7 +789,21 @@ class JSCodeGen()(using genCtx: Context) { def isExcluded(m: Symbol): Boolean = { def hasAccessBoundary = m.accessBoundary(defn.RootClass) ne defn.RootClass - m.is(Deferred) || m.isConstructor || hasAccessBoundary || (m.owner eq defn.ObjectClass) + + def isOfJLObject: Boolean = m.owner eq defn.ObjectClass + + def isDefaultParamOfJSNativeDef: Boolean = { + m.name.is(DefaultGetterName) && { + val info = new DefaultParamInfo(m) + !info.isForConstructor && info.attachedMethod.hasAnnotation(jsdefn.JSNativeAnnot) + } + } + + m.is(Deferred) + || m.isConstructor + || hasAccessBoundary + || isOfJLObject + || m.hasAnnotation(jsdefn.JSNativeAnnot) || isDefaultParamOfJSNativeDef // #4557 } val forwarders = for { From 3ad19ce5712f15f2ca5f24790e74997bde9d29e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 12 Oct 2021 17:47:45 +0200 Subject: [PATCH 5/5] Upgrade to Scala.js 1.7.1. --- project/plugins.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/plugins.sbt b/project/plugins.sbt index 861d9491e13e..4e70c7f3122a 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.7.0") +addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.7.1") addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "3.6")