Skip to content

Commit

Permalink
Fix #4148: Do not emit static forwarders for non-top-level objects by…
Browse files Browse the repository at this point in the history
… default.

Scala/JVM does not emit static forwarders for non-top-level
objects. The reason we do it in Scala.js is for implementations of
JDK classes that contain static methods in inner static classes
(#3950). However, this causes issues with existing Scala projects
that have non-top-level `object`s and `class`es with names
differing only in case.

In this commit, we now only emit static forwarders for top-level
objects by default, and add an opt-in option to emit them for
non-top-level objects, which should be used by implementations of
JDK classes.
  • Loading branch information
sjrd committed Aug 19, 2020
1 parent de973d6 commit 3c3b100
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 4 deletions.
14 changes: 10 additions & 4 deletions compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1008,16 +1008,22 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
* back-end considers them as colliding because they have the same name,
* but we must not.
*
* Further, unlike scalac, we emit forwarders for *any* static object, not
* just top-level ones. This is important so we can implement static methods
* By default, we only emit forwarders for top-level objects, like scalac.
* However, if requested via a compiler option, we enable them for all
* static objects. This is important so we can implement static methods
* of nested static classes of JDK APIs (see #3950).
*/

/** Is the given Scala class, interface or module class a candidate for
* static forwarders?
*/
def isCandidateForForwarders(sym: Symbol): Boolean =
!settings.noForwarders && sym.isStatic && !isImplClass(sym)
def isCandidateForForwarders(sym: Symbol): Boolean = {
!settings.noForwarders && sym.isStatic && !isImplClass(sym) && {
// Reject non-top-level objects unless opted in via the appropriate option
scalaJSOpts.genStaticForwardersForNonTopLevelObjects ||
!sym.name.containsChar('$') // this is the same test that scalac performs
}
}

/** Gen the static forwarders to the members of a class or interface for
* methods of its companion object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ trait ScalaJSOptions {
* If false, bad calls to classOf will cause an error. */
def fixClassOf: Boolean

/** Should static forwarders be emitted for non-top-level objects.
*
* Scala/JVM does not do that. Since Scala.js 1.2.0, we do not do it by
* default either, but this option can be used to opt in. This is necessary
* for implementations of JDK classes.
*/
def genStaticForwardersForNonTopLevelObjects: Boolean

/** which source locations in source maps should be relativized (or where
* should they be mapped to)? */
def sourceURIMaps: List[URIMap]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin {
object scalaJSOpts extends ScalaJSOptions {
import ScalaJSOptions.URIMap
var fixClassOf: Boolean = false
var genStaticForwardersForNonTopLevelObjects: Boolean = false
lazy val sourceURIMaps: List[URIMap] = {
if (_sourceURIMaps.nonEmpty)
_sourceURIMaps.reverse
Expand Down Expand Up @@ -118,6 +119,8 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin {
for (option <- options) {
if (option == "fixClassOf") {
fixClassOf = true
} else if (option == "genStaticForwardersForNonTopLevelObjects") {
genStaticForwardersForNonTopLevelObjects = true
} else if (option.startsWith("mapSourceURI:")) {
val uris = option.stripPrefix("mapSourceURI:").split("->")

Expand Down Expand Up @@ -170,6 +173,10 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin {
| - strips away the prefix FROM_URI (if it matches)
| - optionally prefixes the TO_URI, where stripping has been performed
| - any number of occurrences are allowed. Processing is done on a first match basis.
| -P:$name:genStaticForwardersForNonTopLevelObjects
| Generate static forwarders for non-top-level objects.
| This option should be used by codebases that implement JDK classes.
| When used together with -Xno-forwarders, this option has no effect.
| -P:$name:fixClassOf
| Repair calls to Predef.classOf that reach Scala.js.
| WARNING: This is a tremendous hack! Expect ugly errors if you use this option.
Expand Down
6 changes: 6 additions & 0 deletions project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,8 @@ object Build {
* flag prevents these mistakes from happening.
*/
scalacOptions += "-Yno-predef",
// We implement JDK classes, so we emit static forwarders for all static objects
scalacOptions += "-P:scalajs:genStaticForwardersForNonTopLevelObjects",

resourceGenerators in Compile += Def.task {
val output = (resourceManaged in Compile).value / "java/lang/Object.sjsir"
Expand Down Expand Up @@ -1059,6 +1061,8 @@ object Build {
* we rely on the Scala library.
*/
scalacOptions += "-Yno-predef",
// We implement JDK classes, so we emit static forwarders for all static objects
scalacOptions += "-P:scalajs:genStaticForwardersForNonTopLevelObjects",

headerSources in Compile ~= { srcs =>
srcs.filter { src =>
Expand Down Expand Up @@ -1706,6 +1710,8 @@ object Build {
moduleKind == ModuleKind.ESModule) // this is an approximation that works for now
},

scalacOptions in Test += "-P:scalajs:genStaticForwardersForNonTopLevelObjects",

scalaJSLinkerConfig ~= { _.withSemantics(TestSuiteLinkerOptions.semantics _) },
scalaJSModuleInitializers in Test ++= TestSuiteLinkerOptions.moduleInitializers,

Expand Down

0 comments on commit 3c3b100

Please sign in to comment.