From bf76b9679f40ebae3f48db25f43e47c040202714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Fri, 21 Jan 2022 10:49:26 +0100 Subject: [PATCH] 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. --- .../dotty/tools/backend/sjs/JSPositions.scala | 6 +-- sbt-test/scalajs/basic/build.sbt | 45 +++++++++++++++++++ sbt-test/scalajs/basic/test | 1 + 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/backend/sjs/JSPositions.scala b/compiler/src/dotty/tools/backend/sjs/JSPositions.scala index 84b6304d60eb..454b5b5c3a5c 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSPositions.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSPositions.scala @@ -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) } } diff --git a/sbt-test/scalajs/basic/build.sbt b/sbt-test/scalajs/basic/build.sbt index 92c44049aeef..431f46f40fe0 100644 --- a/sbt-test/scalajs/basic/build.sbt +++ b/sbt-test/scalajs/basic/build.sbt @@ -1,3 +1,5 @@ +lazy val testIRPositions = taskKey[Unit]("test IR positions (#14240)") + enablePlugins(ScalaJSPlugin) scalaVersion := sys.props("plugin.scalaVersion") @@ -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) +} diff --git a/sbt-test/scalajs/basic/test b/sbt-test/scalajs/basic/test index 62ea636c177f..60e897d4bee8 100644 --- a/sbt-test/scalajs/basic/test +++ b/sbt-test/scalajs/basic/test @@ -1 +1,2 @@ > run +> testIRPositions