Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upUnable to find extant inner trait IR during fastopt #4148
Comments
|
I found the issue. At If they were at the top-level, scalac would report a warning that this is going to break on Windows, because it generates a That means that when emitting this file, the generated Try renaming either |
|
Wow! That's fascinating. What I don't understand is why series/2.x doesn't hit this, since Simulacrum is generating this same stuff. I'm going to need to play with this a bit to see if I can figure out how to fix this without breaking binary compatibility. |
|
Related: lemonlabsuk/scala-uri#152 |
|
Perhaps simulacrum generates the objects first and the trait second. In that order, things "work" because the real trait overrides the class with static forwarders, not the other way around. That can also serve as a workaround, by the way. |
|
That is only moderately horrifying. |
|
I believe we could fix this in Scala.js without breaking actual functionality, as follows. When emitting the static forwarders of a module class:
then do not emit the synthetic class with the static forwarders. The first condition makes sense because, if it has a companion class, then the code is already halfway broken on Scala/JVM and scalac already emits a warning that it's going to break on case-insensitive file systems. Since currently, emitting these static forwarders produces completely broken products, fixing the issue as above won't introduce regressions for existing correct code. @gzm0 WDYT? Does that make sense? |
|
Hum, wouldn't this break things on case insensitive systems? (i.e. a synthesized companion just "disappears" because there happens to be a thing with a name that only differs in case)? (IIUC Scala is a case sensitive language). |
|
(that was meant to be case sensitive systems) |
|
It could break something, in theory, but that would have to something that actually relies on the presence of static forwarders. And that never happens for Scala code. It's only necessary for libraries implementing JDK classes, and for JDK classes we know that there are no pair of classes that differ only in case (otherwise the JDK would be broken on Windows). |
|
What about module initializers? They rely on static forwarders, no? (The way I got to this: If only the JDK needs static forwarders, we should make them compile time opt-in. Ah no, we can't do that, module inits...) |
|
Realistically, module initializers are only used for main methods. And main methods would not work on the JVM in nested objects in the first place, since Scala/JVM never emits static forwarders for nested objects. From there, the likelihood that someone has a nested main method in an object whose name differs only in case with another class/trait nested at the same level in its enclosing object is ... close to zero. And it would already have been non-working on Windows. It could happen. In that case the solution would be to rename one or the other. Since it's at the level of a main method, this is unlikely to be a public API that would be bound by binary compatibility constraints. |
…er class. This is only relevant for case insensitive file systems. On such file systems, a static nested object could generate a static forwarder class whose name differed from a regular class' name only in case, therefore erasing the latter on case insensitive file systems. We now avoid emitting static forwarder classes for objects if they would clash with a regular class on a case insensitive file system.
…er class. This is only relevant for case insensitive file systems. On such file systems, a static nested object could generate a static forwarder class whose name differed from a regular class' name only in case, therefore erasing the latter on case insensitive file systems. We now avoid emitting static forwarder classes for objects if they would clash with a regular class on a case insensitive file system.
…er class. This is only relevant for case insensitive file systems. On such file systems, a static nested object could generate a static forwarder class whose name differed from a regular class' name only in case, therefore erasing the latter on case insensitive file systems. We now avoid emitting static forwarder classes for objects if they would clash with a regular class on a case insensitive file system.
…er class. This is only relevant for case insensitive file systems. On such file systems, a static nested object could generate a static forwarder class whose name differed from a regular class' name only in case, therefore erasing the latter on case insensitive file systems. We now avoid emitting static forwarder classes for objects if they would clash with a regular class on a case insensitive file system.
…jects by 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 (scala-js#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.
This is only relevant for case insensitive file systems. On such
file systems, a static nested object could generate a static
forwarder class whose name differed from a regular class' name only
in case, therefore erasing the latter on case insensitive file
systems.
We now avoid emitting static forwarder classes for objects if they
would clash with a regular class on a case insensitive file system.
When doing so, we emit a warning. On Scala 2.13.2+, the warning can
be silenced with `@nowarn("other")` on the object, or
`-Wconf:cat=other:s` in the scalac options.
For top-level objects, the warning is similar to a warning already
emitted by the JVM back-end. For non-top-level objects, it only
happens in Scala.js, and only when they are emitted due to the
opt-in option.
This commit complements its parent in fixing scala-js#4148 for good, as now
that issue won't present itself even when we opt in for static
forwards of non-top-level objects. We therefore add a test for
requires the opt-in for other reasons.
This is only relevant for case insensitive file systems. On such
file systems, a static nested object could generate a static
forwarder class whose name differed from a regular class' name only
in case, therefore erasing the latter on case insensitive file
systems.
We now avoid emitting static forwarder classes for objects if they
would clash with a regular class on a case insensitive file system.
When doing so, we emit a warning. On Scala 2.13.2+, the warning can
be silenced with `@nowarn("other")` on the object, or
`-Wconf:cat=other:s` in the scalac options.
For top-level objects, the warning is similar to a warning already
emitted by the JVM back-end. For non-top-level objects, it only
happens in Scala.js, and only when they are emitted due to the
opt-in option.
This commit complements its parent in fixing scala-js#4148 for good, as now
that issue won't present itself even when we opt in for static
forwards of non-top-level objects. We therefore add a test for
requires the opt-in for other reasons.
…jects by 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 (scala-js#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.
This is only relevant for case insensitive file systems. On such
file systems, a static nested object could generate a static
forwarder class whose name differed from a regular class' name only
in case, therefore erasing the latter on case insensitive file
systems.
We now avoid emitting static forwarder classes for objects if they
would clash with a regular class on a case insensitive file system.
When doing so, we emit a warning. On Scala 2.13.2+, the warning can
be silenced with `@nowarn("other")` on the object, or
`-Wconf:cat=other:s` in the scalac options.
For top-level objects, the warning is similar to a warning already
emitted by the JVM back-end. For non-top-level objects, it only
happens in Scala.js, and only when they are emitted due to the
opt-in option.
This commit complements its parent in fixing scala-js#4148 for good, as now
that issue won't present itself even when we opt in for static
forwards of non-top-level objects. We therefore add a test for
whole requires the opt-in for other reasons.
This is only relevant for case insensitive file systems. On such
file systems, a static nested object could generate a static
forwarder class whose name differed from a regular class' name only
in case, therefore erasing the latter on case insensitive file
systems.
We now avoid emitting static forwarder classes for objects if they
would clash with a regular class on a case insensitive file system.
When doing so, we emit a warning. On Scala 2.13.2+, the warning can
be silenced with `@nowarn("other")` on the object, or
`-Wconf:cat=other:s` in the scalac options.
For top-level objects, the warning is similar to a warning already
emitted by the JVM back-end. For non-top-level objects, it only
happens in Scala.js, and only when they are emitted due to the
opt-in option.
This commit complements its parent in fixing scala-js#4148 for good, as now
that issue won't present itself even when we opt in for static
forwards of non-top-level objects. We therefore add a test for
that issue, which we could not do before because the test suite as
a whole requires the opt-in for other reasons.
Please refer to this tag: https://github.com/djspiewak/cats-effect/tree/bug/scalajs-concurrentops
You can reproduce the issue with
sbt coreJS/test.lawsJS/testalso reproduces it. ScalaJS 1.1.1, Scala 2.13.3/2.12.11 (reproduces on both). Compilation finishes without a problem, but fastOpt does not:There's a bunch of that, but this is the topmost one. The perplexing thing is that the
Concurrent$Ops.sjsirfile exists, andcoreJVM/testworks just fine. The error survives throughcleanand evenrm -rf *; git reset --hard HEAD.Note that this source code used to use Simulacrum, but I removed that on this tag. You can see the pre-removal state in
HEAD^on the tag. The source code itself is identical to the series/2.x branch on the same repository, which builds and tests just fine with ScalaJS 1.1.1.I'm going to keep poking around with the plugins to see if I can figure out where the error was introduced, but this seems like an SJS bug.