-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
abstract case class
breaks Scala.js sourcemaps?
#14240
Comments
If it's helpful, here's how I was able to re-implement |
Is this truly minimized, is all of the code necessary? Or is it just any |
Is there reason to believe the bug is in this repo and not in the scalajs-bundler repo? |
It can possibly be slightly more minimized. Like the
Not directly. The same code does work with Scala 2.x, but I can't rule out the possibility that Scala 3.x is generating different but equally valid output that the bundling thing doesn't understand. I don't understand the Scala.js pipeline well enough to know when/how sourcemaps get generated, but another experiment I did was using the Scala 2.13 artifacts for Cats Effect in a Scala 3.1 project, and I had no problems there. So whatever is different I assume is in the Scala 3 sjsir artifacts for Cats Effect. |
Okay. Let's wait for Scala.js expert to weigh in on where we should look next. |
The Scala 2 has 1-based positions, but the IR has 0-based position, so we do Edit: Oops, yes, looks like it does: |
scalac positions are 1-based, while IR positions are 0-based. There is therefore a `- 1` in the nsc plugin, that was ported over to dotc without questioning. It turns out that dotc uses 0-based positions as well (as documented in `SourceFile`), and so we should not make any adaptation there.
@sjrd Thanks for tracking this down! A question: assuming (IIUC) that this bug has affected all currently published Scala 3 SJSIR artifacts, it (mildly) |
No. It can detect if it's consuming a previous version of the IR. But it cannot know whether that was emitted by Scala 2 or Scala 3, so it would still not know whether to apply the correction or not.
Backwards compat is guaranteed when the old compiler did the right thing at the time it did it. In other words, we do not break correct IR. But if the compiler was buggy and emitted wrong code to begin with ... there is little we can do. This is not specific to Scala.js. If the JVM backend emits the wrong bytecode for some piece of program, the JVM is not going to fix it for you later. You have to recompile and republish with a fixed version of the compiler. This has happened in the past: I remember fixing myself the codegen for |
Fix #14240: Fix off-by-1 when emitting Scala.js IR Positions.
scalac positions are 1-based, while IR positions are 0-based. There is therefore a `- 1` in the nsc plugin, that was ported over to dotc without questioning. It turns out that dotc uses 0-based positions as well (as documented in `SourceFile`), and so we should not make any adaptation there.
scalac positions are 1-based, while IR positions are 0-based. There is therefore a `- 1` in the nsc plugin, that was ported over to dotc without questioning. It turns out that dotc uses 0-based positions as well (as documented in `SourceFile`), and so we should not make any adaptation there.
…sourcemaps off-by-one bug scala/scala3#14240
(originally reported by @mn98 as a possible Cats Effect bug)
Compiler version
3.1.1-RC2
Minimized code
Output
Expectation
Switching to Scala 2.13.7 there is no error.
The text was updated successfully, but these errors were encountered: