Skip to content
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

improvement: Better source positions relativization #3695

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions nir/src/main/scala/scala/scalanative/nir/Position.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ object Position {
sealed trait SourceFile {
def filename: Option[String] = this match {
case SourceFile.Virtual => None
case source: SourceFile.SourceRootRelative =>
case source: SourceFile.Relative =>
Option(source.path.getFileName()).map(_.toString())
}
def directory: Option[String] = this match {
case SourceFile.Virtual => None
case source: SourceFile.SourceRootRelative =>
case source: SourceFile.Relative =>
Option(source.path.getParent()).map(_.toString())
}
}
Expand All @@ -65,7 +65,7 @@ object SourceFile {
* path relative to `-sourceroot` setting defined when compiling source -
* typically it's root directory of workspace
*/
case class SourceRootRelative(pathString: String) extends SourceFile {
case class Relative(pathString: String) extends SourceFile {
lazy val path: Path = Paths.get(pathString)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ final class BinaryDeserializer(buffer: ByteBuffer, nirSource: NIRSource) {
def getPosition(): nir.Position = in(prelude.sections.positions) {
val file = getString() match {
case "" => nir.SourceFile.Virtual
case path => nir.SourceFile.SourceRootRelative(path)
case path => nir.SourceFile.Relative(path)
}
val line = getLebUnsignedInt()
val column = getLebUnsignedInt()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ final class BinarySerializer(channel: WritableByteChannel) {
override def put(pos: nir.Position): Unit = {
putString {
pos.source match { // interned
case nir.SourceFile.Virtual => ""
case nir.SourceFile.SourceRootRelative(pathString) => pathString
case nir.SourceFile.Virtual => ""
case nir.SourceFile.Relative(pathString) => pathString
}
}
putLebUnsignedInt(pos.line)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package scala.scalanative
package nscplugin

import java.nio.file.{Path => JPath}
import java.nio.file.{Path => JPath, Paths => JPaths}
import java.util.stream.{Stream => JStream}
import java.util.function.{Consumer => JConsumer}
import scala.collection.mutable
Expand Down Expand Up @@ -206,15 +206,14 @@ abstract class NirGenPhase[G <: Global with Singleton](override val global: G)
* It returns the absolute path of `source` if it is not contained in
* `reference`.
*/
def relativePath(source: CompilerSourceFile, reference: String): String = {
def relativePath(source: CompilerSourceFile, reference: JPath): String = {
val file = source.file
val jfile = file.file
if (jfile eq null)
file.path // repl and other custom tests use abstract files with no path
else {
val sourcePath = jfile.toPath.toAbsolutePath.normalize
val refPath =
java.nio.file.Paths.get(reference).toAbsolutePath.normalize
val refPath = reference.normalize
if (sourcePath.startsWith(refPath)) {
val path = refPath.relativize(sourcePath)
import scala.collection.JavaConverters._
Expand All @@ -223,20 +222,26 @@ abstract class NirGenPhase[G <: Global with Singleton](override val global: G)
}
}

private val sourceRoot: String = {
val sourcePath = settings.sourcepath.value
if (sourcePath.isEmpty) settings.rootdir.value
else sourcePath
}
private val sourceRoot = JPaths
.get {
val sourcePath = settings.sourcepath.value
if (sourcePath.isEmpty) settings.rootdir.value
else sourcePath
}
.toAbsolutePath()

private[this] def convert(
nscSource: CompilerSourceFile
): nir.SourceFile = {
if (nscSource.file.isVirtual) nir.SourceFile.Virtual
else {
val sourceRelativePath = relativePath(nscSource, sourceRoot)
nir.SourceFile.SourceRootRelative(sourceRelativePath)
val absSourcePath = nscSource.file.absolute.file.toPath()
val relativeTo = scalaNativeOpts.positionRelativizationPaths
.find(absSourcePath.startsWith(_))
.map(_.toAbsolutePath())
.getOrElse(sourceRoot)
nir.SourceFile.Relative(relativePath(nscSource, relativeTo))
}

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -486,11 +486,11 @@ trait NirGenStat[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
// initialising and returning an instance of the class, using C.
for ((ctor, ctorIdx) <- ctors.zipWithIndex) {
val ctorSig = genMethodSig(ctor)
val ctorArgsSig = ctorSig.args.map(_.mangle).mkString
val ctorSuffix = if (ctorIdx == 0) "" else s"$$$ctorIdx"
implicit val pos: nir.Position = ctor.pos

reflectiveInstantiationInfo += ReflectiveInstantiationBuffer(
fqSymId + ctorArgsSig
fqSymId + ctorSuffix
)
val reflInstBuffer = reflectiveInstantiationInfo.last

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package scala.scalanative.nscplugin

import java.net.URI
import java.nio.file.Path

/** This trait allows to query all options to the ScalaNative Plugin
*
Expand All @@ -16,4 +16,7 @@ trait ScalaNativeOptions {
* of JDK classes.
*/
def genStaticForwardersForNonTopLevelObjects: Boolean

/** List of paths usd for relativization of source file positions */
def positionRelativizationPaths: Seq[Path]
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ package nscplugin

import scala.tools.nsc._
import scala.tools.nsc.plugins._
import java.net.URI
import java.net.URISyntaxException
import java.nio.file.{Path, Paths}

class ScalaNativePlugin(val global: Global) extends Plugin {
val name = "scalanative"
Expand All @@ -25,6 +24,8 @@ class ScalaNativePlugin(val global: Global) extends Plugin {

object scalaNativeOpts extends ScalaNativeOptions {
var genStaticForwardersForNonTopLevelObjects: Boolean = false

var positionRelativizationPaths: Seq[Path] = Nil
}

object prepNativeInterop extends PrepNativeInterop[global.type](global) {
Expand All @@ -48,6 +49,16 @@ class ScalaNativePlugin(val global: Global) extends Plugin {
options.foreach {
case "genStaticForwardersForNonTopLevelObjects" =>
genStaticForwardersForNonTopLevelObjects = true

case opt if opt.startsWith("positionRelativizationPaths:") =>
positionRelativizationPaths = {
positionRelativizationPaths ++ opt
.stripPrefix("positionRelativizationPaths:")
.split(';')
.map(Paths.get(_))
.filter(_.isAbsolute())
}.distinct.sortBy(-_.getNameCount())

case opt if opt.startsWith("mapSourceURI:") =>
global.reporter.warning(
global.NoPosition,
Expand All @@ -65,6 +76,12 @@ class ScalaNativePlugin(val global: Global) extends Plugin {
| 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:positionRelativizationPaths
| Change the source file positions in generated outputs based on list of provided paths.
| It would strip the prefix of the source file if it matches given path.
| Non-absolute paths would be ignored.
| Multiple paths should be seperated by a single semicolon ';' character.
| If none of the patches matches path would be relative to -sourcepath if defined or -sourceroot otherwise.
""".stripMargin)

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import core._
import Contexts._

import java.net.URI
import java.nio.file.Path

class GenNIR(settings: GenNIR.Settings) extends PluginPhase {
val phaseName = GenNIR.name
Expand All @@ -28,6 +29,10 @@ object GenNIR {
* this option can be used to opt in. This is necessary for
* implementations of JDK classes.
*/
genStaticForwardersForNonTopLevelObjects: Boolean = false
genStaticForwardersForNonTopLevelObjects: Boolean = false,

/** List of paths usd for relativization of source file positions
*/
positionRelativizationPaths: Seq[Path] = Nil
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,10 @@ trait GenReflectiveInstantisation(using Context) {
// initialising and returning an instance of the class, using C.
for ((ctor, ctorIdx) <- ctors.zipWithIndex) {
val ctorSig = genMethodSig(ctor)
val ctorArgsSig = ctorSig.args.map(_.mangle).mkString
given nir.Position = ctor.span
val ctorSuffix = if (ctorIdx == 0) "" else s"$$$ctorIdx"
given reflInstBuffer: ReflectiveInstantiationBuffer =
ReflectiveInstantiationBuffer(fqSymName.id + ctorArgsSig)
ReflectiveInstantiationBuffer(fqSymName.id + ctorSuffix)

// Lambda generation consists of generating a class which extends
// scala.runtime.AbstractFunction1, with an apply method that accepts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ class NirCodeGen(val settings: GenNIR.Settings)(using ctx: Context)

protected val defnNir = NirDefinitions.get
protected val nirPrimitives = new NirPrimitives()
protected val positionsConversions = new NirPositions()
protected val positionsConversions = new NirPositions(
settings.positionRelativizationPaths
)
protected val cachedMethodSig =
collection.mutable.Map.empty[(Symbol, Boolean), nir.Type.Function]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import dotty.tools.dotc.util.{SourceFile, SourcePosition}
import dotty.tools.dotc.util.Spans.Span
import scalanative.nir
import scala.compiletime.uninitialized
import java.nio.file.{Path, Paths}

class NirPositions()(using Context) {
class NirPositions(positionRelativizationPaths: Seq[Path])(using Context) {
given fromSourcePosition: Conversion[SourcePosition, nir.Position] = {
sourcePos =>
sourceAndSpanToNirPos(sourcePos.source, sourcePos.span)
Expand Down Expand Up @@ -44,16 +45,24 @@ class NirPositions()(using Context) {
lastNIRSource
}

private val sourceRoot =
if !ctx.settings.sourcepath.isDefault
then ctx.settings.sourcepath.value
else ctx.settings.sourceroot.value
private val sourceRoot = Paths
.get(
if !ctx.settings.sourcepath.isDefault
then ctx.settings.sourcepath.value
else ctx.settings.sourceroot.value
)
.toAbsolutePath()
private def convert(dotcSource: SourceFile): nir.SourceFile = {
if dotcSource.file.isVirtual
then nir.SourceFile.Virtual
else
val sourceRelativePath = SourceFile.relativePath(dotcSource, sourceRoot)
nir.SourceFile.SourceRootRelative(sourceRelativePath)
else {
val absSourcePath = dotcSource.file.absolute.jpath
val relativeTo = positionRelativizationPaths
.find(absSourcePath.startsWith(_))
.getOrElse(sourceRoot)
.toString()
nir.SourceFile.Relative(SourceFile.relativePath(dotcSource, relativeTo))
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import dotty.tools.dotc.core.Contexts.NoContext
import java.net.URI
import java.net.URISyntaxException
import dotty.tools.dotc.core.Contexts.Context
import java.nio.file.Paths

class ScalaNativePlugin extends StandardPlugin:
val name: String = "scalanative"
Expand All @@ -17,13 +18,26 @@ class ScalaNativePlugin extends StandardPlugin:
| 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:positionRelativizationPaths
| Change the source file positions in generated outputs based on list of provided paths.
| It would strip the prefix of the source file if it matches given path.
| Non-absolute paths would be ignored.
| Multiple paths should be seperated by a single semicolon ';' character.
| If none of the patches matches path would be relative to -sourcepath if defined or -sourceroot otherwise.
""".stripMargin)

override def init(options: List[String]): List[PluginPhase] = {
val genNirSettings = options
.foldLeft(GenNIR.Settings()) {
case (config, "genStaticForwardersForNonTopLevelObjects") =>
config.copy(genStaticForwardersForNonTopLevelObjects = true)
case (config, s"positionRelativizationPaths:${paths}") =>
config.copy(positionRelativizationPaths =
(config.positionRelativizationPaths ++ paths
.split(';')
.map(Paths.get(_))
.filter(_.isAbsolute())).distinct.sortBy(-_.getNameCount())
)
case (config, s"mapSourceURI:${mapping}") =>
given Context = NoContext
report.warning("'mapSourceURI' is deprecated, it's ignored.")
Expand Down
12 changes: 6 additions & 6 deletions project/Settings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -720,12 +720,12 @@ object Settings {
libraryDependencies += "org.scala-lang" % libraryName % scalaVersion.value,
fetchScalaSource / artifactPath :=
baseDirectory.value.getParentFile / "target" / "scalaSources" / scalaVersion.value,
scalacOptions ++= Seq(
// Create nir.SourceFile relative to Scala sources dir instead of root dir
// It should use -sourcepath for both, but it fails to compile under Scala 2
if (scalaVersion.value.startsWith("2.")) "-rootdir" else "-sourcepath",
(fetchScalaSource / artifactPath).value.toString
),
// Create nir.SourceFile relative to Scala sources dir instead of root dir
// It should use -sourcepath for both, but it fails to compile under Scala 2
scalacOptions ++=
scalaNativeCompilerOptions(
s"positionRelativizationPaths:${crossTarget.value / "patched"};${(fetchScalaSource / artifactPath).value}"
),
// Scala.js original comment modified to clarify issue is Scala.js.
/* Work around for https://github.com/scala-js/scala-js/issues/2649
* We would like to always use `update`, but
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ object ScalaNativePluginInternal {
* times per project.
*/
def scalaNativeConfigSettings(testConfig: Boolean): Seq[Setting[_]] = Seq(
scalacOptions +=
s"-P:scalanative:positionRelativizationPaths:${sourceDirectories.value.map(_.getAbsolutePath()).mkString(";")}",
nativeLinkReleaseFull := Def
.task {
val sbtLogger = streams.value.log
Expand Down