Skip to content

Commit

Permalink
Fix scala#14240: Fix off-by-1 when emitting Scala.js IR Positions.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sjrd authored and olsdavis committed Apr 4, 2022
1 parent 0fe4ded commit bf76b96
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 3 deletions.
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/backend/sjs/JSPositions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ class JSPositions()(using Context) {
private def sourceAndSpan2irPos(source: SourceFile, span: Span): ir.Position = {
if (!span.exists) ir.Position.NoPosition
else {
// dotty positions are 1-based but IR positions are 0-based
// dotty positions and IR positions are both 0-based
val irSource = span2irPosCache.toIRSource(source)
val point = span.point
val line = source.offsetToLine(point) - 1
val column = source.column(point) - 1
val line = source.offsetToLine(point)
val column = source.column(point)
ir.Position(irSource, line, column)
}
}
Expand Down
45 changes: 45 additions & 0 deletions sbt-test/scalajs/basic/build.sbt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
lazy val testIRPositions = taskKey[Unit]("test IR positions (#14240)")

enablePlugins(ScalaJSPlugin)

scalaVersion := sys.props("plugin.scalaVersion")
Expand All @@ -6,3 +8,46 @@ scalaVersion := sys.props("plugin.scalaVersion")
libraryDependencies += ("org.scala-js" %%% "scalajs-dom" % "1.1.0").cross(CrossVersion.for3Use2_13)

scalaJSUseMainModuleInitializer := true

// #14240 Make sure that generated IR positions are 0-based
testIRPositions := {
import scala.concurrent.{Future, _}
import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration._
import scala.util.{Failure, Success}

import org.scalajs.ir.Names._
import org.scalajs.ir.Position
import org.scalajs.ir.Trees._
import org.scalajs.linker.interface.unstable.IRFileImpl

val ir = (Compile / scalaJSIR).value
val classNameToTest = ClassName("test.Main$")

val classDefFuture = {
// This logic is copied from the implementation of `scalajsp` in sbt-scalajs
Future.traverse(ir.data) { irFile =>
val ir = IRFileImpl.fromIRFile(irFile)
ir.entryPointsInfo.map { i =>
if (i.className == classNameToTest) Success(Some(ir))
else Success(None)
}.recover { case t => Failure(t) }
}.flatMap { irs =>
irs.collectFirst {
case Success(Some(f)) => f.tree
}.getOrElse {
val t = new MessageOnlyException(s"class ${classNameToTest.nameString} not found on classpath")
irs.collect { case Failure(st) => t.addSuppressed(st) }
throw t
}
}
}
val classDef = Await.result(classDefFuture, Duration.Inf)

def testPos(pos: Position, expectedLine: Int, expectedColumn: Int): Unit = {
if (!pos.source.getPath.endsWith("/Main.scala") || pos.line != expectedLine || pos.column != expectedColumn)
throw new MessageOnlyException(s"Expected Main.scala@$expectedLine:$expectedColumn but got $pos")
}

testPos(classDef.pos, 5, 7)
}
1 change: 1 addition & 0 deletions sbt-test/scalajs/basic/test
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
> run
> testIRPositions

0 comments on commit bf76b96

Please sign in to comment.