From 5c6b6dec32f4822784ba031ce56581224105c56f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Fri, 1 Dec 2023 13:24:05 +0100 Subject: [PATCH 01/10] use new zinc api for virtualfile [Cherry-picked a7cec5fcbd018e900a7cabdbc90b60b6d7695530][modified] --- .../src/dotty/tools/backend/jvm/CodeGen.scala | 10 ++-- .../tools/dotc/config/ScalaSettings.scala | 1 + .../src/dotty/tools/dotc/core/Contexts.scala | 32 ++++++++----- .../src/dotty/tools/dotc/sbt/APIUtils.scala | 2 +- .../src/dotty/tools/dotc/sbt/ExtractAPI.scala | 10 ++-- .../tools/dotc/sbt/ExtractDependencies.scala | 26 ++++++---- .../dotty/tools/dotc/util/SourceFile.scala | 27 +++++++++++ .../src/dotty/tools/io/AbstractFile.scala | 3 ++ project/Build.scala | 11 +++-- project/Dependencies.scala | 3 +- .../tools/xsbt/CompilerBridgeDriver.java | 48 ++++++++++++------- .../dotty/tools/xsbt/DelegatingReporter.java | 14 ++++-- .../src/dotty/tools/xsbt/PositionBridge.java | 21 +++----- sbt-bridge/src/dotty/tools/xsbt/Problem.java | 19 +++++--- .../src/dotty/tools/xsbt/ZincPlainFile.java | 3 +- .../src/dotty/tools/xsbt/ZincVirtualFile.java | 22 ++++----- sbt-bridge/src/xsbt/CachedCompilerImpl.java | 2 +- sbt-bridge/src/xsbt/DottydocRunner.java | 2 +- .../xsbt/ScalaCompilerForUnitTesting.scala | 13 +++-- sbt-bridge/test/xsbt/TestVirtualFile.scala | 14 +++++- sbt-bridge/test/xsbti/TestCallback.scala | 31 ++++++------ sbt-test/sbt-bridge/zinc-13-compat/test | 2 +- .../compiler-plugin/plugin/DivideZero.scala | 4 +- .../compactify/src/main/scala/Nested.scala | 4 +- sbt-test/source-dependencies/compactify/test | 2 +- 25 files changed, 198 insertions(+), 128 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/CodeGen.scala b/compiler/src/dotty/tools/backend/jvm/CodeGen.scala index c9f9e4e23d90..fdd104951c20 100644 --- a/compiler/src/dotty/tools/backend/jvm/CodeGen.scala +++ b/compiler/src/dotty/tools/backend/jvm/CodeGen.scala @@ -30,7 +30,7 @@ import scala.tools.asm import scala.tools.asm.tree._ import tpd._ import dotty.tools.io.AbstractFile -import dotty.tools.dotc.util.NoSourcePosition +import dotty.tools.dotc.util.{NoSourcePosition, SourceFile} class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)( val bTypes: BTypesFromSymbols[int.type]) { self => @@ -106,7 +106,7 @@ class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)( } // Creates a callback that will be evaluated in PostProcessor after creating a file - private def onFileCreated(cls: ClassNode, claszSymbol: Symbol, sourceFile: interfaces.SourceFile): AbstractFile => Unit = clsFile => { + private def onFileCreated(cls: ClassNode, claszSymbol: Symbol, sourceFile: SourceFile): AbstractFile => Unit = clsFile => { val (fullClassName, isLocal) = atPhase(sbtExtractDependenciesPhase) { (ExtractDependencies.classNameAsString(claszSymbol), claszSymbol.isLocal) } @@ -116,10 +116,10 @@ class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)( ctx.compilerCallback.onClassGenerated(sourceFile, convertAbstractFile(clsFile), className) if (ctx.sbtCallback != null) { - val jSourceFile = sourceFile.jfile.orElse(null) + val jSourceFile = sourceFile.underlyingZincFile val cb = ctx.sbtCallback - if (isLocal) cb.generatedLocalClass(jSourceFile, clsFile.file) - else cb.generatedNonLocalClass(jSourceFile, clsFile.file, className, fullClassName) + if (isLocal) cb.generatedLocalClass(jSourceFile, clsFile.jpath) + else cb.generatedNonLocalClass(jSourceFile, clsFile.jpath, className, fullClassName) } } diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 8f2a58b43850..ad363ec8b747 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -325,6 +325,7 @@ private sealed trait YSettings: val YdebugTypeError: Setting[Boolean] = BooleanSetting("-Ydebug-type-error", "Print the stack trace when a TypeError is caught", false) val YdebugError: Setting[Boolean] = BooleanSetting("-Ydebug-error", "Print the stack trace when any error is caught.", false) val YdebugUnpickling: Setting[Boolean] = BooleanSetting("-Ydebug-unpickling", "Print the stack trace when an error occurs when reading Tasty.", false) + val YdebugVirtualFiles: Setting[Boolean] = BooleanSetting("-Ydebug-virtual-files", "Debug usage of virtual files, e.g. remote cache in sbt", false) val YtermConflict: Setting[String] = ChoiceSetting("-Yresolve-term-conflict", "strategy", "Resolve term conflicts", List("package", "object", "error"), "error") val Ylog: Setting[List[String]] = PhasesSetting("-Ylog", "Log operations during") val YlogClasspath: Setting[Boolean] = BooleanSetting("-Ylog-classpath", "Output information about what classpath is being applied.") diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index e0e43169820a..9c7da5de1947 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -39,22 +39,25 @@ import util.Store import xsbti.AnalysisCallback import plugins._ import java.util.concurrent.atomic.AtomicInteger +import java.util.Map as JMap import java.nio.file.InvalidPathException + object Contexts { - private val (compilerCallbackLoc, store1) = Store.empty.newLocation[CompilerCallback]() - private val (sbtCallbackLoc, store2) = store1.newLocation[AnalysisCallback]() - private val (printerFnLoc, store3) = store2.newLocation[Context => Printer](new RefinedPrinter(_)) - private val (settingsStateLoc, store4) = store3.newLocation[SettingsState]() - private val (compilationUnitLoc, store5) = store4.newLocation[CompilationUnit]() - private val (runLoc, store6) = store5.newLocation[Run | Null]() - private val (profilerLoc, store7) = store6.newLocation[Profiler]() - private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]() - private val (importInfoLoc, store9) = store8.newLocation[ImportInfo | Null]() - private val (typeAssignerLoc, store10) = store9.newLocation[TypeAssigner](TypeAssigner) + private val (compilerCallbackLoc, store1) = Store.empty.newLocation[CompilerCallback]() + private val (sbtCallbackLoc, store2) = store1.newLocation[AnalysisCallback]() + private val (printerFnLoc, store3) = store2.newLocation[Context => Printer](new RefinedPrinter(_)) + private val (settingsStateLoc, store4) = store3.newLocation[SettingsState]() + private val (compilationUnitLoc, store5) = store4.newLocation[CompilationUnit]() + private val (runLoc, store6) = store5.newLocation[Run | Null]() + private val (profilerLoc, store7) = store6.newLocation[Profiler]() + private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]() + private val (importInfoLoc, store9) = store8.newLocation[ImportInfo | Null]() + private val (typeAssignerLoc, store10) = store9.newLocation[TypeAssigner](TypeAssigner) + private val (zincVirtualFilesLoc, store11) = store10.newLocation[JMap[String, xsbti.VirtualFile] | Null]() - private val initialStore = store10 + private val initialStore = store11 /** The current context */ inline def ctx(using ctx: Context): Context = ctx @@ -164,9 +167,12 @@ object Contexts { /** The compiler callback implementation, or null if no callback will be called. */ def compilerCallback: CompilerCallback = store(compilerCallbackLoc) - /** The sbt callback implementation if we are run from sbt, null otherwise */ + /** The Zinc callback implementation if we are run from Zinc, null otherwise */ def sbtCallback: AnalysisCallback = store(sbtCallbackLoc) + /** A map from absolute path to VirtualFile if we are run from Zinc, null otherwise */ + def zincVirtualFiles: JMap[String, xsbti.VirtualFile] | Null = store(zincVirtualFilesLoc) + /** The current plain printer */ def printerFn: Context => Printer = store(printerFnLoc) @@ -665,6 +671,8 @@ object Contexts { def setCompilerCallback(callback: CompilerCallback): this.type = updateStore(compilerCallbackLoc, callback) def setSbtCallback(callback: AnalysisCallback): this.type = updateStore(sbtCallbackLoc, callback) + def setZincVirtualFiles(map: JMap[String, xsbti.VirtualFile]): this.type = + updateStore(zincVirtualFilesLoc, map) def setPrinterFn(printer: Context => Printer): this.type = updateStore(printerFnLoc, printer) def setSettings(settingsState: SettingsState): this.type = updateStore(settingsStateLoc, settingsState) def setRun(run: Run | Null): this.type = updateStore(runLoc, run) diff --git a/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala b/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala index aa98f79c8e3b..2c82d3d57b11 100644 --- a/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala +++ b/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala @@ -37,7 +37,7 @@ object APIUtils { def registerDummyClass(classSym: ClassSymbol)(using Context): Unit = { if (ctx.sbtCallback != null) { val classLike = emptyClassLike(classSym) - ctx.sbtCallback.api(ctx.compilationUnit.source.file.file, classLike) + ctx.sbtCallback.api(ctx.compilationUnit.source.underlyingZincFile, classLike) } } diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala index f54baeb7256c..b4adf30ebb4b 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala @@ -65,9 +65,9 @@ class ExtractAPI extends Phase { override def run(using Context): Unit = { val unit = ctx.compilationUnit - val sourceFile = unit.source.file + val sourceFile = unit.source if (ctx.sbtCallback != null) - ctx.sbtCallback.startSource(sourceFile.file) + ctx.sbtCallback.startSource(sourceFile.underlyingZincFile) val apiTraverser = new ExtractAPICollector val classes = apiTraverser.apiSource(unit.tpdTree) @@ -75,7 +75,7 @@ class ExtractAPI extends Phase { if (ctx.settings.YdumpSbtInc.value) { // Append to existing file that should have been created by ExtractDependencies - val pw = new PrintWriter(File(sourceFile.jpath).changeExtension("inc").toFile + val pw = new PrintWriter(File(sourceFile.file.jpath).changeExtension("inc").toFile .bufferedWriter(append = true), true) try { classes.foreach(source => pw.println(DefaultShowAPI(source))) @@ -85,8 +85,8 @@ class ExtractAPI extends Phase { if ctx.sbtCallback != null && !ctx.compilationUnit.suspendedAtInliningPhase // already registered before this unit was suspended then - classes.foreach(ctx.sbtCallback.api(sourceFile.file, _)) - mainClasses.foreach(ctx.sbtCallback.mainClass(sourceFile.file, _)) + classes.foreach(ctx.sbtCallback.api(sourceFile.underlyingZincFile, _)) + mainClasses.foreach(ctx.sbtCallback.mainClass(sourceFile.underlyingZincFile, _)) } } diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index ddd89b156fd4..2a9c58b82e7c 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -4,6 +4,7 @@ package sbt import scala.language.unsafeNulls import java.io.File +import java.nio.file.Path import java.util.{Arrays, EnumSet} import dotty.tools.dotc.ast.tpd @@ -18,6 +19,7 @@ import dotty.tools.dotc.core.Denotations.StaleSymbol import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.transform.SymUtils._ import dotty.tools.dotc.util.{SrcPos, NoSourcePosition} +import dotty.tools.uncheckedNN import dotty.tools.io import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive} import xsbti.UseScope @@ -55,7 +57,7 @@ class ExtractDependencies extends Phase { override def isRunnable(using Context): Boolean = { def forceRun = ctx.settings.YdumpSbtInc.value || ctx.settings.YforceSbtPhases.value - super.isRunnable && (ctx.sbtCallback != null || forceRun) + super.isRunnable && (ctx.sbtCallback != null && ctx.sbtCallback.enabled() || forceRun) } // Check no needed. Does not transform trees @@ -90,7 +92,7 @@ class ExtractDependencies extends Phase { } finally pw.close() } - if (ctx.sbtCallback != null) { + if (ctx.sbtCallback != null && ctx.sbtCallback.enabled()) { collector.usedNames.foreach { case (clazz, usedNames) => val className = classNameAsString(clazz) @@ -111,17 +113,17 @@ class ExtractDependencies extends Phase { */ def recordDependency(dep: ClassDependency)(using Context): Unit = { val fromClassName = classNameAsString(dep.from) - val sourceFile = ctx.compilationUnit.source.file.file + val zincSourceFile = ctx.compilationUnit.source.underlyingZincFile - def binaryDependency(file: File, binaryClassName: String) = - ctx.sbtCallback.binaryDependency(file, binaryClassName, fromClassName, sourceFile, dep.context) + def binaryDependency(file: Path, binaryClassName: String) = + ctx.sbtCallback.binaryDependency(file, binaryClassName, fromClassName, zincSourceFile, dep.context) def processExternalDependency(depFile: AbstractFile, binaryClassName: String) = { depFile match { case ze: ZipArchive#Entry => // The dependency comes from a JAR ze.underlyingSource match - case Some(zip) if zip.file != null => - binaryDependency(zip.file, binaryClassName) + case Some(zip) if zip.jpath != null => + binaryDependency(zip.jpath, binaryClassName) case _ => case pf: PlainFile => // The dependency comes from a class file // FIXME: pf.file is null for classfiles coming from the modulepath @@ -130,8 +132,8 @@ class ExtractDependencies extends Phase { // java.io.File, this means that we cannot record dependencies coming // from the modulepath. For now this isn't a big deal since we only // support having the standard Java library on the modulepath. - if pf.file != null then - binaryDependency(pf.file, binaryClassName) + if pf.jpath != null then + binaryDependency(pf.jpath, binaryClassName) case _ => internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.from.srcPos) } @@ -139,12 +141,16 @@ class ExtractDependencies extends Phase { val depFile = dep.to.associatedFile if (depFile != null) { + def depIsSameSource = + val depVF: xsbti.VirtualFile | Null = ctx.zincVirtualFiles.uncheckedNN.get(depFile.absolutePath) + depVF != null && depVF.id() == zincSourceFile.id() + // Cannot ignore inheritance relationship coming from the same source (see sbt/zinc#417) def allowLocal = dep.context == DependencyByInheritance || dep.context == LocalDependencyByInheritance if (depFile.extension == "class") { // Dependency is external -- source is undefined processExternalDependency(depFile, dep.to.binaryClassName) - } else if (allowLocal || depFile.file != sourceFile) { + else if (allowLocal || !depIsSameSource /* old: depFile.file != sourceFile.file */) { // We cannot ignore dependencies coming from the same source file because // the dependency info needs to propagate. See source-dependencies/trait-trait-211. val toClassName = classNameAsString(dep.to) diff --git a/compiler/src/dotty/tools/dotc/util/SourceFile.scala b/compiler/src/dotty/tools/dotc/util/SourceFile.scala index 3462036d7ba6..ca307104bad4 100644 --- a/compiler/src/dotty/tools/dotc/util/SourceFile.scala +++ b/compiler/src/dotty/tools/dotc/util/SourceFile.scala @@ -64,6 +64,33 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends import SourceFile._ private var myContent: Array[Char] | Null = null + private var myUnderlyingZincFile: xsbti.VirtualFile | Null = null + + def underlyingZincFile(using Context): xsbti.VirtualFile = + val local = myUnderlyingZincFile + if local == null then + // usually without -sourcepath then the `underlying` will be set by Zinc. + val maybeUnderlying = file.underlying + val underlying0 = + if maybeUnderlying == null then + // When we have `-sourcepath` set then the file could come from the filesystem, + // rather than a zinc managed file, so then we need to check if we have a virtual file for it. + // TODO: we should consider in the future if there is a use case for sourcepath to possibly be + // made of virtual files. + val fromLookup = ctx.zincVirtualFiles.uncheckedNN.get(file.absolutePath) + if fromLookup != null then + fromLookup + else + sys.error(s"no underlying file for ${file.absolutePath}, possible paths = ${ctx.zincVirtualFiles.keySet}") + else maybeUnderlying + if ctx.settings.YdebugVirtualFiles.value then + val isVirtual = !underlying0.isInstanceOf[xsbti.PathBasedFile] + println(s"found underlying zinc file ${underlying0.id} for ${file.absolutePath} [virtual = $isVirtual]") + + myUnderlyingZincFile = underlying0 + underlying0 + else + local /** The contents of the original source file. Note that this can be empty, for example when * the source is read from Tasty. */ diff --git a/compiler/src/dotty/tools/io/AbstractFile.scala b/compiler/src/dotty/tools/io/AbstractFile.scala index f34fe6f40b9c..a8405923700c 100644 --- a/compiler/src/dotty/tools/io/AbstractFile.scala +++ b/compiler/src/dotty/tools/io/AbstractFile.scala @@ -118,6 +118,9 @@ abstract class AbstractFile extends Iterable[AbstractFile] { /** Returns the underlying Path if any and null otherwise. */ def jpath: JPath + /** Overridden in sbt-bridge ZincPlainFile and ZincVirtualFile */ + def underlying: xsbti.VirtualFile | Null = null + /** An underlying source, if known. Mostly, a zip/jar file. */ def underlyingSource: Option[AbstractFile] = None diff --git a/project/Build.scala b/project/Build.scala index 0e7535c1b83e..13b23f1f0b38 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -551,7 +551,7 @@ object Build { // get libraries onboard libraryDependencies ++= Seq( "org.scala-lang.modules" % "scala-asm" % "9.5.0-scala-1", // used by the backend - Dependencies.oldCompilerInterface, // we stick to the old version to avoid deprecation warnings + Dependencies.compilerInterface, "org.jline" % "jline-reader" % "3.19.0", // used by the REPL "org.jline" % "jline-terminal" % "3.19.0", "org.jline" % "jline-terminal-jna" % "3.19.0", // needed for Windows @@ -668,7 +668,8 @@ object Build { val dottyTastyInspector = jars("scala3-tasty-inspector") val dottyInterfaces = jars("scala3-interfaces") val tastyCore = jars("tasty-core") - run(insertClasspathInArgs(args1, List(dottyCompiler, dottyInterfaces, asm, dottyStaging, dottyTastyInspector, tastyCore).mkString(File.pathSeparator))) + val compilerInterface = findArtifactPath(externalDeps, "compiler-interface") + run(insertClasspathInArgs(args1, List(dottyCompiler, dottyInterfaces, asm, dottyStaging, dottyTastyInspector, tastyCore, compilerInterface).mkString(File.pathSeparator))) } else run(args) }, @@ -707,7 +708,8 @@ object Build { val dottyTastyInspector = jars("scala3-tasty-inspector") val tastyCore = jars("tasty-core") val asm = findArtifactPath(externalDeps, "scala-asm") - extraClasspath ++= Seq(dottyCompiler, dottyInterfaces, asm, dottyStaging, dottyTastyInspector, tastyCore) + val compilerInterface = findArtifactPath(externalDeps, "compiler-interface") + extraClasspath ++= Seq(dottyCompiler, dottyInterfaces, asm, dottyStaging, dottyTastyInspector, tastyCore, compilerInterface) } val fullArgs = main :: (if (printTasty) args else insertClasspathInArgs(args, extraClasspath.mkString(File.pathSeparator))) @@ -1051,8 +1053,7 @@ object Build { // when sbt reads the settings. Test / test := (LocalProject("scala3-sbt-bridge-tests") / Test / test).value, - // The `newCompilerInterface` is backward compatible with the `oldCompilerInterface` - libraryDependencies += Dependencies.newCompilerInterface % Provided + libraryDependencies += Dependencies.compilerInterface % Provided ) // We use a separate project for the bridge tests since they can only be run diff --git a/project/Dependencies.scala b/project/Dependencies.scala index f22601346803..4b2a9e2815be 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -28,6 +28,5 @@ object Dependencies { "com.vladsch.flexmark" % "flexmark-ext-yaml-front-matter" % flexmarkVersion, ) - val newCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.9.0" - val oldCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.3.5" + val compilerInterface = "org.scala-sbt" % "compiler-interface" % "1.9.0" } diff --git a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java index 12291120b157..885feacf2767 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java +++ b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.Comparator; +import java.util.HashMap; import java.util.Arrays; public class CompilerBridgeDriver extends Driver { @@ -51,14 +52,40 @@ public boolean sourcesRequired() { } synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, Logger log, Reporter delegate) { - DelegatingReporter reporter = new DelegatingReporter(delegate); + // convert sources to a HashMap from this.id to itself + HashMap sourcesMap = new HashMap<>(); + + VirtualFile[] sortedSources = new VirtualFile[sources.length]; + System.arraycopy(sources, 0, sortedSources, 0, sources.length); + Arrays.sort(sortedSources, (x0, x1) -> x0.id().compareTo(x1.id())); + + ListBuffer sourcesBuffer = new ListBuffer<>(); + + for (int i = 0; i < sources.length; i++) { + VirtualFile source = sortedSources[i]; + AbstractFile abstractFile = asDottyFile(source); + sourcesBuffer.append(abstractFile); + sourcesMap.put(abstractFile.absolutePath(), source); + } + + DelegatingReporter reporter = new DelegatingReporter(delegate, sourceFile -> { + String pathId = sourceFile.file().absolutePath(); + VirtualFile source = sourcesMap.get(pathId); + + if (source != null) + return source.id(); + else + return pathId; + }); + try { log.debug(this::infoOnCachedCompiler); Contexts.Context initialCtx = initCtx() .fresh() .setReporter(reporter) - .setSbtCallback(callback); + .setSbtCallback(callback) + .setZincVirtualFiles(sourcesMap); Contexts.Context context = setup(args, initialCtx).map(t -> t._2).getOrElse(() -> initialCtx); @@ -70,28 +97,13 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L log.debug(this::prettyPrintCompilationArguments); Compiler compiler = newCompiler(context); - VirtualFile[] sortedSources = new VirtualFile[sources.length]; - System.arraycopy(sources, 0, sortedSources, 0, sources.length); - Arrays.sort( - sortedSources, - new Comparator() { - @Override - public int compare(VirtualFile x0, VirtualFile x1) { - return x0.id().compareTo(x1.id()); - } - } - ); - - ListBuffer sourcesBuffer = new ListBuffer<>(); - for (VirtualFile file: sortedSources) - sourcesBuffer.append(asDottyFile(file)); doCompile(compiler, sourcesBuffer.toList(), context); for (xsbti.Problem problem: delegate.problems()) { callback.problem(problem.category(), problem.position(), problem.message(), problem.severity(), true); } - } else { + } else { delegate.printSummary(); } diff --git a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java index dba1889b393f..f45e3768378c 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java +++ b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java @@ -19,12 +19,16 @@ import xsbti.Position; import xsbti.Severity; +import java.util.function.*; + final public class DelegatingReporter extends AbstractReporter { private xsbti.Reporter delegate; + private final Function lookup; - public DelegatingReporter(xsbti.Reporter delegate) { + public DelegatingReporter(xsbti.Reporter delegate, Function lookup) { super(); this.delegate = delegate; + this.lookup = lookup; } public void dropDelegate() { @@ -53,7 +57,7 @@ public void doReport(Diagnostic dia, Context ctx) { messageBuilder.append(System.lineSeparator()).append(explanation(message, ctx)); } - delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions)); + delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions, lookup)); } private static Severity severityOf(int level) { @@ -68,9 +72,9 @@ private static Severity severityOf(int level) { return severity; } - private static Position positionOf(SourcePosition pos) { - if (pos.exists()){ - return new PositionBridge(pos, pos.source()); + private Position positionOf(SourcePosition pos) { + if (pos.exists()) { + return new PositionBridge(pos, lookup.apply(pos.source())); } else { return PositionBridge.noPosition; } diff --git a/sbt-bridge/src/dotty/tools/xsbt/PositionBridge.java b/sbt-bridge/src/dotty/tools/xsbt/PositionBridge.java index 6b3c25e2e27c..eb01da25ba1c 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/PositionBridge.java +++ b/sbt-bridge/src/dotty/tools/xsbt/PositionBridge.java @@ -12,10 +12,12 @@ import java.io.File; import java.util.Optional; +import java.util.function.Function; public class PositionBridge implements Position { private final SourcePosition pos; private final SourceFile src; + private final String pathId; public static final Position noPosition = new Position() { public Optional sourceFile() { @@ -45,9 +47,10 @@ public String toString() { } }; - public PositionBridge(SourcePosition pos, SourceFile src) { + public PositionBridge(SourcePosition pos, String path) { this.pos = pos; - this.src = src; + this.src = pos.source(); + this.pathId = path; } @Override @@ -82,17 +85,7 @@ public Optional offset() { @Override public Optional sourcePath() { - if (!src.exists()) - return Optional.empty(); - - AbstractFile sourceFile = pos.source().file(); - if (sourceFile instanceof ZincPlainFile) { - return Optional.of(((ZincPlainFile) sourceFile).underlying().id()); - } else if (sourceFile instanceof ZincVirtualFile) { - return Optional.of(((ZincVirtualFile) sourceFile).underlying().id()); - } else { - return Optional.of(sourceFile.path()); - } + return Optional.of(pathId); } @Override @@ -131,7 +124,7 @@ public String toString() { else return path; } - + @Override public Optional startOffset() { if (src.content().length == 0) diff --git a/sbt-bridge/src/dotty/tools/xsbt/Problem.java b/sbt-bridge/src/dotty/tools/xsbt/Problem.java index 9bde5ce76ebb..9eed04056752 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/Problem.java +++ b/sbt-bridge/src/dotty/tools/xsbt/Problem.java @@ -2,11 +2,14 @@ import java.util.List; import java.util.Optional; +import java.util.function.Function; + import static java.util.stream.Collectors.toList; import dotty.tools.dotc.reporting.CodeAction; import dotty.tools.dotc.rewrites.Rewrites.ActionPatch; import dotty.tools.dotc.util.SourcePosition; +import dotty.tools.dotc.util.SourceFile; import scala.jdk.javaapi.CollectionConverters; import scala.jdk.javaapi.OptionConverters; @@ -14,6 +17,7 @@ import xsbti.Position; import xsbti.Severity; + final public class Problem implements xsbti.Problem { private final Position _position; private final String _message; @@ -21,8 +25,10 @@ final public class Problem implements xsbti.Problem { private final Optional _rendered; private final String _diagnosticCode; private final List _actions; + private final Function _lookup; - public Problem(Position position, String message, Severity severity, String rendered, String diagnosticCode, List actions) { + public Problem(Position position, String message, Severity severity, String rendered, String diagnosticCode, List actions, + Function lookup) { super(); this._position = position; this._message = message; @@ -30,6 +36,7 @@ public Problem(Position position, String message, Severity severity, String rend this._rendered = Optional.of(rendered); this._diagnosticCode = diagnosticCode; this._actions = actions; + this._lookup = lookup; } public String category() { @@ -78,23 +85,23 @@ public List actions() { // never getting called. return _actions .stream() - .map(action -> new Action(action.title(), OptionConverters.toJava(action.description()), toWorkspaceEdit(CollectionConverters.asJava(action.patches())))) + .map(action -> new Action(action.title(), OptionConverters.toJava(action.description()), toWorkspaceEdit(CollectionConverters.asJava(action.patches()), _lookup))) .collect(toList()); } } - private static WorkspaceEdit toWorkspaceEdit(List patches) { + private static WorkspaceEdit toWorkspaceEdit(List patches, Function lookup) { return new WorkspaceEdit( patches .stream() - .map(patch -> new TextEdit(positionOf(patch.srcPos()), patch.replacement())) + .map(patch -> new TextEdit(positionOf(patch.srcPos(), lookup), patch.replacement())) .collect(toList()) ); } - private static Position positionOf(SourcePosition pos) { + private static Position positionOf(SourcePosition pos, Function lookup) { if (pos.exists()){ - return new PositionBridge(pos, pos.source()); + return new PositionBridge(pos, lookup.apply(pos.source())); } else { return PositionBridge.noPosition; } diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java index 68b3494cb84b..dad8a80ef798 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java @@ -15,7 +15,8 @@ public ZincPlainFile(PathBasedFile underlying) { this._underlying = underlying; } + @Override public PathBasedFile underlying() { return _underlying; } -} \ No newline at end of file +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java index a79686270f34..74813f87fb20 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java @@ -20,20 +20,20 @@ public ZincVirtualFile(VirtualFile underlying) throws IOException { this._underlying = underlying; // fill in the content - OutputStream output = output(); - try { - Streamable.Bytes bytes = new Streamable.Bytes() { - @Override - public InputStream inputStream() { - return underlying.input(); - } - }; - output.write(bytes.toByteArray()); - } finally { - output.close(); + try (OutputStream output = output()) { + try (InputStream input = underlying.input()) { + Streamable.Bytes bytes = new Streamable.Bytes() { + @Override + public InputStream inputStream() { + return input; + } + }; + output.write(bytes.toByteArray()); + } } } + @Override public VirtualFile underlying() { return _underlying; } diff --git a/sbt-bridge/src/xsbt/CachedCompilerImpl.java b/sbt-bridge/src/xsbt/CachedCompilerImpl.java index 0b876475e51e..1310c76f70db 100644 --- a/sbt-bridge/src/xsbt/CachedCompilerImpl.java +++ b/sbt-bridge/src/xsbt/CachedCompilerImpl.java @@ -62,7 +62,7 @@ synchronized public void run(File[] sources, DependencyChanges changes, Analysis Context ctx = new ContextBase().initialCtx().fresh() .setSbtCallback(callback) - .setReporter(new DelegatingReporter(delegate)); + .setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath())); dotty.tools.dotc.reporting.Reporter reporter = Main.process(commandArguments(sources), ctx); if (reporter.hasErrors()) { diff --git a/sbt-bridge/src/xsbt/DottydocRunner.java b/sbt-bridge/src/xsbt/DottydocRunner.java index e4c35a317e71..a91ff087cea9 100644 --- a/sbt-bridge/src/xsbt/DottydocRunner.java +++ b/sbt-bridge/src/xsbt/DottydocRunner.java @@ -53,7 +53,7 @@ public void run() { args = retained.toArray(new String[retained.size()]); Context ctx = new ContextBase().initialCtx().fresh() - .setReporter(new DelegatingReporter(delegate)); + .setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath())); try { Class dottydocMainClass = Class.forName("dotty.tools.dottydoc.Main"); diff --git a/sbt-bridge/test/xsbt/ScalaCompilerForUnitTesting.scala b/sbt-bridge/test/xsbt/ScalaCompilerForUnitTesting.scala index e58f9fefd92d..51f10e90f932 100644 --- a/sbt-bridge/test/xsbt/ScalaCompilerForUnitTesting.scala +++ b/sbt-bridge/test/xsbt/ScalaCompilerForUnitTesting.scala @@ -19,7 +19,6 @@ import TestCallback.ExtractedClassDependencies * source code using Scala compiler. */ class ScalaCompilerForUnitTesting { - import scala.language.reflectiveCalls /** * Compiles given source code using Scala compiler and returns API representation @@ -122,7 +121,7 @@ class ScalaCompilerForUnitTesting { * The sequence of temporary files corresponding to passed snippets and analysis * callback is returned as a result. */ - def compileSrcs(groupedSrcs: List[List[String]]): (Seq[File], TestCallback) = { + def compileSrcs(groupedSrcs: List[List[String]]): (Seq[VirtualFile], TestCallback) = { val temp = IO.createTemporaryDirectory val analysisCallback = new TestCallback val classesDir = new File(temp, "classes") @@ -137,13 +136,13 @@ class ScalaCompilerForUnitTesting { prepareSrcFile(temp, fileName, src) } - val virtualSrcFiles = srcFiles.map(file => TestVirtualFile(file.toPath)).toArray + val virtualSrcFiles = srcFiles.toArray val classesDirPath = classesDir.getAbsolutePath.toString val output = new SingleOutput: def getOutputDirectory() = classesDir bridge.run( - virtualSrcFiles.toArray, + virtualSrcFiles, new TestDependencyChanges, Array("-Yforce-sbt-phases", "-classpath", classesDirPath, "-usejavacp", "-d", classesDirPath), output, @@ -158,14 +157,14 @@ class ScalaCompilerForUnitTesting { (files.flatten.toSeq, analysisCallback) } - def compileSrcs(srcs: String*): (Seq[File], TestCallback) = { + def compileSrcs(srcs: String*): (Seq[VirtualFile], TestCallback) = { compileSrcs(List(srcs.toList)) } - private def prepareSrcFile(baseDir: File, fileName: String, src: String): File = { + private def prepareSrcFile(baseDir: File, fileName: String, src: String): VirtualFile = { val srcFile = new File(baseDir, fileName) IO.write(srcFile, src) - srcFile + new TestVirtualFile(srcFile.toPath) } } diff --git a/sbt-bridge/test/xsbt/TestVirtualFile.scala b/sbt-bridge/test/xsbt/TestVirtualFile.scala index db00038272a8..2c7729d0a6cd 100644 --- a/sbt-bridge/test/xsbt/TestVirtualFile.scala +++ b/sbt-bridge/test/xsbt/TestVirtualFile.scala @@ -1,6 +1,6 @@ package xsbt -import xsbti.PathBasedFile +import xsbti.{PathBasedFile, VirtualFileRef} import java.nio.file.{Files, Path} import scala.io.Source import scala.io.Codec @@ -8,7 +8,17 @@ import scala.io.Codec class TestVirtualFile(path: Path) extends PathBasedFile: override def contentHash(): Long = ??? override def input(): java.io.InputStream = Files.newInputStream(path) - override def id(): String = name() + lazy val absolutePath: String = path.toAbsolutePath.toString() + override def id(): String = absolutePath override def name(): String = path.toFile.getName override def names(): Array[String] = ??? override def toPath(): Path = path + + + override def hashCode(): Int = absolutePath.hashCode() + + override def equals(x: Any): Boolean = this.eq(x.asInstanceOf[AnyRef]) || x.match { + case vf: VirtualFileRef => vf.id() == id() + } + + diff --git a/sbt-bridge/test/xsbti/TestCallback.scala b/sbt-bridge/test/xsbti/TestCallback.scala index a0919dc69bc4..0f48ad9a18ef 100644 --- a/sbt-bridge/test/xsbti/TestCallback.scala +++ b/sbt-bridge/test/xsbti/TestCallback.scala @@ -14,40 +14,37 @@ class TestCallback extends AnalysisCallback { case class TestUsedName(name: String, scopes: EnumSet[UseScope]) val classDependencies = new ArrayBuffer[(String, String, DependencyContext)] - val binaryDependencies = new ArrayBuffer[(File, String, String, File, DependencyContext)] - val products = new ArrayBuffer[(File, File)] + val binaryDependencies = new ArrayBuffer[(Path, String, String, VirtualFileRef, DependencyContext)] + val products = new ArrayBuffer[(VirtualFileRef, Path)] val usedNamesAndScopes = scala.collection.mutable.Map.empty[String, Set[TestUsedName]].withDefaultValue(Set.empty) - val classNames = scala.collection.mutable.Map.empty[File, Set[(String, String)]].withDefaultValue(Set.empty) - val apis: scala.collection.mutable.Map[File, Seq[ClassLike]] = scala.collection.mutable.Map.empty + val classNames = scala.collection.mutable.Map.empty[VirtualFileRef, Set[(String, String)]].withDefaultValue(Set.empty) + val apis: scala.collection.mutable.Map[VirtualFileRef, Seq[ClassLike]] = scala.collection.mutable.Map.empty def usedNames = usedNamesAndScopes.view.mapValues(_.map(_.name)).toMap - override def startSource(source: File): Unit = { + override def startSource(source: File): Unit = ??? + override def startSource(source: VirtualFile): Unit = { assert(!apis.contains(source), s"startSource can be called only once per source file: $source") apis(source) = Seq.empty } - override def startSource(source: VirtualFile): Unit = ??? - override def binaryDependency(binary: File, name: String, fromClassName: String, source: File, context: DependencyContext): Unit = { + override def binaryDependency(binary: File, name: String, fromClassName: String, source: File, context: DependencyContext): Unit = ??? + override def binaryDependency(binary: Path, name: String, fromClassName: String, source: VirtualFileRef, context: DependencyContext): Unit = { binaryDependencies += ((binary, name, fromClassName, source, context)) } - override def binaryDependency(binary: Path, name: String, fromClassName: String, source: VirtualFileRef, context: DependencyContext): Unit = ??? - override def generatedNonLocalClass(source: File, - module: File, - binaryClassName: String, - srcClassName: String): Unit = { + override def generatedNonLocalClass(source: File, module: File, binaryClassName: String, srcClassName: String): Unit = ??? + override def generatedNonLocalClass(source: VirtualFileRef, module: Path, binaryClassName: String, srcClassName: String): Unit = { products += ((source, module)) classNames(source) += ((srcClassName, binaryClassName)) () } - override def generatedNonLocalClass(source: VirtualFileRef, module: Path, binaryClassName: String, srcClassName: String): Unit = ??? - override def generatedLocalClass(source: File, module: File): Unit = { + override def generatedLocalClass(source: File, module: File): Unit = ??? + override def generatedLocalClass(source: VirtualFileRef, module: Path): Unit = { products += ((source, module)) () } - override def generatedLocalClass(source: VirtualFileRef, module: Path): Unit = ??? override def classDependency(onClassName: String, sourceClassName: String, context: DependencyContext): Unit = { if (onClassName != sourceClassName) classDependencies += ((onClassName, sourceClassName, context)) @@ -57,10 +54,10 @@ class TestCallback extends AnalysisCallback usedNamesAndScopes(className) += TestUsedName(name, scopes) } - override def api(source: File, classApi: ClassLike): Unit = { + override def api(source: File, classApi: ClassLike): Unit = ??? + override def api(source: VirtualFileRef, classApi: ClassLike): Unit = { apis(source) = classApi +: apis(source) } - override def api(source: VirtualFileRef, classApi: ClassLike): Unit = ??? override def problem(category: String, pos: xsbti.Position, message: String, severity: xsbti.Severity, reported: Boolean): Unit = () override def dependencyPhaseCompleted(): Unit = () diff --git a/sbt-test/sbt-bridge/zinc-13-compat/test b/sbt-test/sbt-bridge/zinc-13-compat/test index f7b3295e155c..d37c2434b1a3 100644 --- a/sbt-test/sbt-bridge/zinc-13-compat/test +++ b/sbt-test/sbt-bridge/zinc-13-compat/test @@ -1,3 +1,3 @@ # this little app test that scala3-sbt-bridge is compatible with Zinc 1.3 # this is necessary to maintain the compatibility with Bloop (see https://github.com/lampepfl/dotty/issues/10816) -> run +-> run diff --git a/sbt-test/sbt-dotty/compiler-plugin/plugin/DivideZero.scala b/sbt-test/sbt-dotty/compiler-plugin/plugin/DivideZero.scala index b7f1b1007420..c6fac6b796c0 100644 --- a/sbt-test/sbt-dotty/compiler-plugin/plugin/DivideZero.scala +++ b/sbt-test/sbt-dotty/compiler-plugin/plugin/DivideZero.scala @@ -19,8 +19,8 @@ class DivideZero extends PluginPhase with StandardPlugin { val phaseName = name - override val runsAfter = Set(Staging.name) - override val runsBefore = Set(Pickler.name) + override val runsAfter = Set(Pickler.name) + override val runsBefore = Set(Staging.name) def init(options: List[String]): List[PluginPhase] = this :: Nil diff --git a/sbt-test/source-dependencies/compactify/src/main/scala/Nested.scala b/sbt-test/source-dependencies/compactify/src/main/scala/Nested.scala index 798868d72640..4b1597d287d4 100644 --- a/sbt-test/source-dependencies/compactify/src/main/scala/Nested.scala +++ b/sbt-test/source-dependencies/compactify/src/main/scala/Nested.scala @@ -38,4 +38,6 @@ class TopLevel2 object TopLevel3 -class TopLevel4 \ No newline at end of file +class TopLevel4 + +object TopLevelModuleSuffix$ diff --git a/sbt-test/source-dependencies/compactify/test b/sbt-test/source-dependencies/compactify/test index b56be3e5d4aa..64be9af369b5 100644 --- a/sbt-test/source-dependencies/compactify/test +++ b/sbt-test/source-dependencies/compactify/test @@ -5,4 +5,4 @@ -> outputEmpty $ delete src/main/scala/For.scala src/main/scala/Nested.scala > compile -> outputEmpty \ No newline at end of file +> outputEmpty From 725352393da70f3376c75f3dac6cae9679bb8e5a Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 10 Jul 2023 15:17:41 +0200 Subject: [PATCH 02/10] use compiler impl of protobuf for presentation compiler zinc 1.4 removes the dependency on protobuf-java, so fails to compile mtags-shared due to the now missing transitive dependency. The compiler implements protobuf streams for semanticdb, so reuse it in the source generator for mtags-shared. [Cherry-picked 34a5e45cfecc5288373df19d93c7bed37d76ad6a] --- project/Build.scala | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/project/Build.scala b/project/Build.scala index 13b23f1f0b38..107a7ed25eb6 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -525,6 +525,33 @@ object Build { recur(lines, false) } + /** replace imports of `com.google.protobuf.*` with compiler implemented version */ + def replaceProtobuf(lines: List[String]): List[String] = { + def recur(ls: List[String]): List[String] = ls match { + case l :: rest => + val lt = l.trim() + if (lt.isEmpty || lt.startsWith("package ") || lt.startsWith("import ")) { + val newLine = + if (lt.startsWith("import com.google.protobuf.")) { + if (lt == "import com.google.protobuf.CodedInputStream") { + "import dotty.tools.dotc.semanticdb.internal.SemanticdbInputStream as CodedInputStream" + } else if (lt == "import com.google.protobuf.CodedOutputStream") { + "import dotty.tools.dotc.semanticdb.internal.SemanticdbOutputStream as CodedOutputStream" + } else { + l + } + } else { + l + } + newLine :: recur(rest) + } else { + ls // don't check rest of file + } + case _ => ls + } + recur(lines) + } + // Settings shared between scala3-compiler and scala3-compiler-bootstrapped lazy val commonDottyCompilerSettings = Seq( // Generate compiler.properties, used by sbt @@ -1135,7 +1162,8 @@ object Build { val mtagsSharedSources = (targetDir ** "*.scala").get.toSet mtagsSharedSources.foreach(f => { val lines = IO.readLines(f) - IO.writeLines(f, insertUnsafeNullsImport(lines)) + val substitutions = (replaceProtobuf(_)) andThen (insertUnsafeNullsImport(_)) + IO.writeLines(f, substitutions(lines)) }) mtagsSharedSources } (Set(mtagsSharedSourceJar)).toSeq From 2fc2e3a9853988a7ed1fc5f32289757bc6a56e22 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Wed, 12 Jul 2023 14:55:04 +0200 Subject: [PATCH 03/10] remain compatible with Zinc 1.3 [Cherry-picked 2458b8f48d175b67fd4aac5caa0b27325e0db8b3] --- .../src/dotty/tools/backend/jvm/CodeGen.scala | 13 ++-- .../tools/dotc/config/ScalaSettings.scala | 1 - .../src/dotty/tools/dotc/core/Contexts.scala | 38 +++++++--- .../src/dotty/tools/dotc/sbt/APIUtils.scala | 4 +- .../src/dotty/tools/dotc/sbt/ExtractAPI.scala | 15 ++-- .../tools/dotc/sbt/ExtractDependencies.scala | 31 ++++---- .../sbt/interfaces/IncrementalCallback.java | 39 ++++++++++ .../dotty/tools/dotc/util/SourceFile.scala | 27 ------- .../src/dotty/tools/io/AbstractFile.scala | 3 - .../dotty/tools/xsbt/AbstractZincFile.java | 7 ++ .../tools/xsbt/CompilerBridgeDriver.java | 46 +++++++++--- .../dotty/tools/xsbt/IncrementalCallback.java | 60 +++++++++++++++ .../tools/xsbt/OldIncrementalCallback.java | 74 +++++++++++++++++++ .../src/dotty/tools/xsbt/ZincPlainFile.java | 2 +- .../src/dotty/tools/xsbt/ZincVirtualFile.java | 2 +- sbt-bridge/src/xsbt/CachedCompilerImpl.java | 7 +- sbt-bridge/src/xsbt/CompilerInterface.java | 1 + sbt-test/sbt-bridge/zinc-13-compat/test | 2 +- 18 files changed, 279 insertions(+), 93 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java create mode 100644 sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java create mode 100644 sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java create mode 100644 sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java diff --git a/compiler/src/dotty/tools/backend/jvm/CodeGen.scala b/compiler/src/dotty/tools/backend/jvm/CodeGen.scala index fdd104951c20..84d85a02ecf8 100644 --- a/compiler/src/dotty/tools/backend/jvm/CodeGen.scala +++ b/compiler/src/dotty/tools/backend/jvm/CodeGen.scala @@ -30,7 +30,7 @@ import scala.tools.asm import scala.tools.asm.tree._ import tpd._ import dotty.tools.io.AbstractFile -import dotty.tools.dotc.util.{NoSourcePosition, SourceFile} +import dotty.tools.dotc.util.NoSourcePosition class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)( val bTypes: BTypesFromSymbols[int.type]) { self => @@ -106,7 +106,7 @@ class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)( } // Creates a callback that will be evaluated in PostProcessor after creating a file - private def onFileCreated(cls: ClassNode, claszSymbol: Symbol, sourceFile: SourceFile): AbstractFile => Unit = clsFile => { + private def onFileCreated(cls: ClassNode, claszSymbol: Symbol, sourceFile: interfaces.SourceFile): AbstractFile => Unit = clsFile => { val (fullClassName, isLocal) = atPhase(sbtExtractDependenciesPhase) { (ExtractDependencies.classNameAsString(claszSymbol), claszSymbol.isLocal) } @@ -115,12 +115,9 @@ class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)( if (ctx.compilerCallback != null) ctx.compilerCallback.onClassGenerated(sourceFile, convertAbstractFile(clsFile), className) - if (ctx.sbtCallback != null) { - val jSourceFile = sourceFile.underlyingZincFile - val cb = ctx.sbtCallback - if (isLocal) cb.generatedLocalClass(jSourceFile, clsFile.jpath) - else cb.generatedNonLocalClass(jSourceFile, clsFile.jpath, className, fullClassName) - } + ctx.withIncCallback: cb => + if (isLocal) cb.generatedLocalClass(sourceFile, clsFile.jpath) + else cb.generatedNonLocalClass(sourceFile, clsFile.jpath, className, fullClassName) } /** Convert a `dotty.tools.io.AbstractFile` into a diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index ad363ec8b747..8f2a58b43850 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -325,7 +325,6 @@ private sealed trait YSettings: val YdebugTypeError: Setting[Boolean] = BooleanSetting("-Ydebug-type-error", "Print the stack trace when a TypeError is caught", false) val YdebugError: Setting[Boolean] = BooleanSetting("-Ydebug-error", "Print the stack trace when any error is caught.", false) val YdebugUnpickling: Setting[Boolean] = BooleanSetting("-Ydebug-unpickling", "Print the stack trace when an error occurs when reading Tasty.", false) - val YdebugVirtualFiles: Setting[Boolean] = BooleanSetting("-Ydebug-virtual-files", "Debug usage of virtual files, e.g. remote cache in sbt", false) val YtermConflict: Setting[String] = ChoiceSetting("-Yresolve-term-conflict", "strategy", "Resolve term conflicts", List("package", "object", "error"), "error") val Ylog: Setting[List[String]] = PhasesSetting("-Ylog", "Log operations during") val YlogClasspath: Setting[Boolean] = BooleanSetting("-Ylog-classpath", "Output information about what classpath is being applied.") diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 9c7da5de1947..31b809297005 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -34,19 +34,17 @@ import scala.annotation.internal.sharable import DenotTransformers.DenotTransformer import dotty.tools.dotc.profile.Profiler +import dotty.tools.dotc.sbt.interfaces.IncrementalCallback import util.Property.Key import util.Store -import xsbti.AnalysisCallback import plugins._ import java.util.concurrent.atomic.AtomicInteger -import java.util.Map as JMap import java.nio.file.InvalidPathException - object Contexts { private val (compilerCallbackLoc, store1) = Store.empty.newLocation[CompilerCallback]() - private val (sbtCallbackLoc, store2) = store1.newLocation[AnalysisCallback]() + private val (incCallbackLoc, store2) = store1.newLocation[IncrementalCallback | Null]() private val (printerFnLoc, store3) = store2.newLocation[Context => Printer](new RefinedPrinter(_)) private val (settingsStateLoc, store4) = store3.newLocation[SettingsState]() private val (compilationUnitLoc, store5) = store4.newLocation[CompilationUnit]() @@ -55,7 +53,7 @@ object Contexts { private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]() private val (importInfoLoc, store9) = store8.newLocation[ImportInfo | Null]() private val (typeAssignerLoc, store10) = store9.newLocation[TypeAssigner](TypeAssigner) - private val (zincVirtualFilesLoc, store11) = store10.newLocation[JMap[String, xsbti.VirtualFile] | Null]() + private val (zincInitialFilesLoc, store11) = store10.newLocation[util.ReadOnlySet[AbstractFile] | Null]() private val initialStore = store11 @@ -168,10 +166,18 @@ object Contexts { def compilerCallback: CompilerCallback = store(compilerCallbackLoc) /** The Zinc callback implementation if we are run from Zinc, null otherwise */ - def sbtCallback: AnalysisCallback = store(sbtCallbackLoc) + def incCallback: IncrementalCallback | Null = store(incCallbackLoc) + def zincInitialFiles: util.ReadOnlySet[AbstractFile] | Null = store(zincInitialFilesLoc) + + /** Run `op` if there exists an incremental callback */ + inline def withIncCallback(inline op: IncrementalCallback => Unit): Unit = + val local = incCallback + if local != null then op(local) - /** A map from absolute path to VirtualFile if we are run from Zinc, null otherwise */ - def zincVirtualFiles: JMap[String, xsbti.VirtualFile] | Null = store(zincVirtualFilesLoc) + def incrementalEnabled: Boolean = + val local = incCallback + if local != null then local.enabled + else false /** The current plain printer */ def printerFn: Context => Printer = store(printerFnLoc) @@ -240,7 +246,16 @@ object Contexts { /** Sourcefile corresponding to given abstract file, memoized */ def getSource(file: AbstractFile, codec: => Codec = Codec(settings.encoding.value)) = { util.Stats.record("Context.getSource") - base.sources.getOrElseUpdate(file, SourceFile(file, codec)) + base.sources.getOrElseUpdate(file, { + val zincSources = zincInitialFiles + val cachedFile = + if zincSources != null then zincSources.lookup(file) match + case null => file + case cached => cached + else + file + SourceFile(cachedFile, codec) + }) } /** SourceFile with given path name, memoized */ @@ -670,9 +685,8 @@ object Contexts { } def setCompilerCallback(callback: CompilerCallback): this.type = updateStore(compilerCallbackLoc, callback) - def setSbtCallback(callback: AnalysisCallback): this.type = updateStore(sbtCallbackLoc, callback) - def setZincVirtualFiles(map: JMap[String, xsbti.VirtualFile]): this.type = - updateStore(zincVirtualFilesLoc, map) + def setIncCallback(callback: IncrementalCallback): this.type = updateStore(incCallbackLoc, callback) + def setZincInitialFiles(zincInitialFiles: util.ReadOnlySet[AbstractFile]): this.type = updateStore(zincInitialFilesLoc, zincInitialFiles) def setPrinterFn(printer: Context => Printer): this.type = updateStore(printerFnLoc, printer) def setSettings(settingsState: SettingsState): this.type = updateStore(settingsStateLoc, settingsState) def setRun(run: Run | Null): this.type = updateStore(runLoc, run) diff --git a/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala b/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala index 2c82d3d57b11..833cf7f2e0ff 100644 --- a/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala +++ b/compiler/src/dotty/tools/dotc/sbt/APIUtils.scala @@ -35,9 +35,9 @@ object APIUtils { * a dummy empty class can be registered instead, using this method. */ def registerDummyClass(classSym: ClassSymbol)(using Context): Unit = { - if (ctx.sbtCallback != null) { + ctx.withIncCallback { cb => val classLike = emptyClassLike(classSym) - ctx.sbtCallback.api(ctx.compilationUnit.source.underlyingZincFile, classLike) + cb.api(ctx.compilationUnit.source, classLike) } } diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala index b4adf30ebb4b..0c5168f3c21a 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala @@ -50,7 +50,7 @@ class ExtractAPI extends Phase { override def isRunnable(using Context): Boolean = { def forceRun = ctx.settings.YdumpSbtInc.value || ctx.settings.YforceSbtPhases.value - super.isRunnable && (ctx.sbtCallback != null || forceRun) + super.isRunnable && (ctx.incrementalEnabled || forceRun) } // Check no needed. Does not transform trees @@ -66,8 +66,8 @@ class ExtractAPI extends Phase { override def run(using Context): Unit = { val unit = ctx.compilationUnit val sourceFile = unit.source - if (ctx.sbtCallback != null) - ctx.sbtCallback.startSource(sourceFile.underlyingZincFile) + ctx.withIncCallback: cb => + cb.startSource(sourceFile) val apiTraverser = new ExtractAPICollector val classes = apiTraverser.apiSource(unit.tpdTree) @@ -82,11 +82,10 @@ class ExtractAPI extends Phase { } finally pw.close() } - if ctx.sbtCallback != null && - !ctx.compilationUnit.suspendedAtInliningPhase // already registered before this unit was suspended - then - classes.foreach(ctx.sbtCallback.api(sourceFile.underlyingZincFile, _)) - mainClasses.foreach(ctx.sbtCallback.mainClass(sourceFile.underlyingZincFile, _)) + ctx.withIncCallback: cb => + if !ctx.compilationUnit.suspendedAtInliningPhase then // already registered before this unit was suspended + classes.foreach(cb.api(sourceFile, _)) + mainClasses.foreach(cb.mainClass(sourceFile, _)) } } diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index 2a9c58b82e7c..131dbdb9ba15 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -19,7 +19,6 @@ import dotty.tools.dotc.core.Denotations.StaleSymbol import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.transform.SymUtils._ import dotty.tools.dotc.util.{SrcPos, NoSourcePosition} -import dotty.tools.uncheckedNN import dotty.tools.io import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive} import xsbti.UseScope @@ -57,7 +56,7 @@ class ExtractDependencies extends Phase { override def isRunnable(using Context): Boolean = { def forceRun = ctx.settings.YdumpSbtInc.value || ctx.settings.YforceSbtPhases.value - super.isRunnable && (ctx.sbtCallback != null && ctx.sbtCallback.enabled() || forceRun) + super.isRunnable && (ctx.incrementalEnabled || forceRun) } // Check no needed. Does not transform trees @@ -92,15 +91,16 @@ class ExtractDependencies extends Phase { } finally pw.close() } - if (ctx.sbtCallback != null && ctx.sbtCallback.enabled()) { - collector.usedNames.foreach { - case (clazz, usedNames) => - val className = classNameAsString(clazz) - usedNames.names.foreach { - case (usedName, scopes) => - ctx.sbtCallback.usedName(className, usedName.toString, scopes) - } - } + if (ctx.incrementalEnabled) { + ctx.withIncCallback: cb => + collector.usedNames.foreach { + case (clazz, usedNames) => + val className = classNameAsString(clazz) + usedNames.names.foreach { + case (usedName, scopes) => + cb.usedName(className, usedName.toString, scopes) + } + } collector.dependencies.foreach(recordDependency) } @@ -113,10 +113,10 @@ class ExtractDependencies extends Phase { */ def recordDependency(dep: ClassDependency)(using Context): Unit = { val fromClassName = classNameAsString(dep.from) - val zincSourceFile = ctx.compilationUnit.source.underlyingZincFile + val sourceFile = ctx.compilationUnit.source def binaryDependency(file: Path, binaryClassName: String) = - ctx.sbtCallback.binaryDependency(file, binaryClassName, fromClassName, zincSourceFile, dep.context) + ctx.withIncCallback(_.binaryDependency(file, binaryClassName, fromClassName, sourceFile, dep.context)) def processExternalDependency(depFile: AbstractFile, binaryClassName: String) = { depFile match { @@ -142,8 +142,7 @@ class ExtractDependencies extends Phase { val depFile = dep.to.associatedFile if (depFile != null) { def depIsSameSource = - val depVF: xsbti.VirtualFile | Null = ctx.zincVirtualFiles.uncheckedNN.get(depFile.absolutePath) - depVF != null && depVF.id() == zincSourceFile.id() + depFile.absolutePath == sourceFile.file.absolutePath // Cannot ignore inheritance relationship coming from the same source (see sbt/zinc#417) def allowLocal = dep.context == DependencyByInheritance || dep.context == LocalDependencyByInheritance @@ -154,7 +153,7 @@ class ExtractDependencies extends Phase { // We cannot ignore dependencies coming from the same source file because // the dependency info needs to propagate. See source-dependencies/trait-trait-211. val toClassName = classNameAsString(dep.to) - ctx.sbtCallback.classDependency(toClassName, fromClassName, dep.context) + ctx.withIncCallback(_.classDependency(toClassName, fromClassName, dep.context)) } } } diff --git a/compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java b/compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java new file mode 100644 index 000000000000..e0a45784bd94 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java @@ -0,0 +1,39 @@ +package dotty.tools.dotc.sbt.interfaces; + +import dotty.tools.dotc.interfaces.SourceFile; + +import java.util.EnumSet; +import java.nio.file.Path; + +/* User code should not implement this interface, it is intended to be a wrapper around xsbti.AnalysisCallback. */ +public interface IncrementalCallback { + default void api(SourceFile sourceFile, xsbti.api.ClassLike classApi) { + } + + default void startSource(SourceFile sourceFile) { + } + + default void mainClass(SourceFile sourceFile, String className) { + } + + default boolean enabled() { + return false; + } + + default void usedName(String className, String name, EnumSet useScopes) { + } + + default void binaryDependency(Path onBinaryEntry, String onBinaryClassName, String fromClassName, + SourceFile fromSourceFile, xsbti.api.DependencyContext context) { + } + + default void classDependency(String onClassName, String sourceClassName, xsbti.api.DependencyContext context) { + } + + default void generatedLocalClass(SourceFile source, Path classFile) { + } + + default void generatedNonLocalClass(SourceFile source, Path classFile, String binaryClassName, + String srcClassName) { + } +} diff --git a/compiler/src/dotty/tools/dotc/util/SourceFile.scala b/compiler/src/dotty/tools/dotc/util/SourceFile.scala index ca307104bad4..3462036d7ba6 100644 --- a/compiler/src/dotty/tools/dotc/util/SourceFile.scala +++ b/compiler/src/dotty/tools/dotc/util/SourceFile.scala @@ -64,33 +64,6 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends import SourceFile._ private var myContent: Array[Char] | Null = null - private var myUnderlyingZincFile: xsbti.VirtualFile | Null = null - - def underlyingZincFile(using Context): xsbti.VirtualFile = - val local = myUnderlyingZincFile - if local == null then - // usually without -sourcepath then the `underlying` will be set by Zinc. - val maybeUnderlying = file.underlying - val underlying0 = - if maybeUnderlying == null then - // When we have `-sourcepath` set then the file could come from the filesystem, - // rather than a zinc managed file, so then we need to check if we have a virtual file for it. - // TODO: we should consider in the future if there is a use case for sourcepath to possibly be - // made of virtual files. - val fromLookup = ctx.zincVirtualFiles.uncheckedNN.get(file.absolutePath) - if fromLookup != null then - fromLookup - else - sys.error(s"no underlying file for ${file.absolutePath}, possible paths = ${ctx.zincVirtualFiles.keySet}") - else maybeUnderlying - if ctx.settings.YdebugVirtualFiles.value then - val isVirtual = !underlying0.isInstanceOf[xsbti.PathBasedFile] - println(s"found underlying zinc file ${underlying0.id} for ${file.absolutePath} [virtual = $isVirtual]") - - myUnderlyingZincFile = underlying0 - underlying0 - else - local /** The contents of the original source file. Note that this can be empty, for example when * the source is read from Tasty. */ diff --git a/compiler/src/dotty/tools/io/AbstractFile.scala b/compiler/src/dotty/tools/io/AbstractFile.scala index a8405923700c..f34fe6f40b9c 100644 --- a/compiler/src/dotty/tools/io/AbstractFile.scala +++ b/compiler/src/dotty/tools/io/AbstractFile.scala @@ -118,9 +118,6 @@ abstract class AbstractFile extends Iterable[AbstractFile] { /** Returns the underlying Path if any and null otherwise. */ def jpath: JPath - /** Overridden in sbt-bridge ZincPlainFile and ZincVirtualFile */ - def underlying: xsbti.VirtualFile | Null = null - /** An underlying source, if known. Mostly, a zip/jar file. */ def underlyingSource: Option[AbstractFile] = None diff --git a/sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java b/sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java new file mode 100644 index 000000000000..788bd74fceab --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java @@ -0,0 +1,7 @@ +package dotty.tools.xsbt; + +import xsbti.VirtualFile; + +interface AbstractZincFile { + VirtualFile underlying(); +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java index 885feacf2767..e5e456226cc9 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java +++ b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java @@ -10,8 +10,10 @@ import dotty.tools.dotc.ScalacCommand; import dotty.tools.dotc.config.Properties; import dotty.tools.dotc.core.Contexts; +import dotty.tools.dotc.util.SourceFile; import dotty.tools.io.AbstractFile; import scala.collection.mutable.ListBuffer; +import scala.jdk.javaapi.CollectionConverters; import scala.io.Codec; import xsbti.Problem; import xsbti.*; @@ -20,6 +22,7 @@ import java.io.IOException; import java.util.Comparator; import java.util.HashMap; +import java.util.Map; import java.util.Arrays; public class CompilerBridgeDriver extends Driver { @@ -51,31 +54,50 @@ public boolean sourcesRequired() { return false; } - synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, Logger log, Reporter delegate) { - // convert sources to a HashMap from this.id to itself - HashMap sourcesMap = new HashMap<>(); + private static VirtualFile asVirtualFileOrNull(SourceFile sourceFile) { + if (sourceFile.file() instanceof AbstractZincFile) { + return ((AbstractZincFile) sourceFile.file()).underlying(); + } else { + return null; + } + } + synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, Logger log, Reporter delegate) { VirtualFile[] sortedSources = new VirtualFile[sources.length]; System.arraycopy(sources, 0, sortedSources, 0, sources.length); Arrays.sort(sortedSources, (x0, x1) -> x0.id().compareTo(x1.id())); ListBuffer sourcesBuffer = new ListBuffer<>(); + dotty.tools.dotc.util.HashSet sourcesSet = new dotty.tools.dotc.util.HashSet<>(8, 2); for (int i = 0; i < sources.length; i++) { VirtualFile source = sortedSources[i]; AbstractFile abstractFile = asDottyFile(source); sourcesBuffer.append(abstractFile); - sourcesMap.put(abstractFile.absolutePath(), source); + sourcesSet.put(abstractFile); } DelegatingReporter reporter = new DelegatingReporter(delegate, sourceFile -> { - String pathId = sourceFile.file().absolutePath(); - VirtualFile source = sourcesMap.get(pathId); + VirtualFile vf = asVirtualFileOrNull(sourceFile); + if (vf != null) { + return vf.id(); + } else { + return sourceFile.path(); // same as in Zinc for 2.13 + } + }); - if (source != null) - return source.id(); - else - return pathId; + IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile -> { + if (sourceFile instanceof SourceFile) { + SourceFile sf = (SourceFile) sourceFile; + VirtualFile vf = asVirtualFileOrNull(sf); + if (vf != null) { + return vf; + } else { + throw new IllegalStateException("Unknown source file: " + sourceFile.path()); + } + } else { + throw new IllegalStateException("Unknown source file: " + sourceFile.path()); + } }); try { @@ -84,8 +106,8 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L Contexts.Context initialCtx = initCtx() .fresh() .setReporter(reporter) - .setSbtCallback(callback) - .setZincVirtualFiles(sourcesMap); + .setZincInitialFiles(sourcesSet) + .setIncCallback(incCallback); Contexts.Context context = setup(args, initialCtx).map(t -> t._2).getOrElse(() -> initialCtx); diff --git a/sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java b/sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java new file mode 100644 index 000000000000..58032cbeb72c --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java @@ -0,0 +1,60 @@ +package dotty.tools.xsbt; + +import dotty.tools.dotc.interfaces.SourceFile; +import java.util.function.Function; + +public final class IncrementalCallback implements dotty.tools.dotc.sbt.interfaces.IncrementalCallback { + + private final xsbti.AnalysisCallback delegate; + private final Function asVirtualFile; + + public IncrementalCallback(xsbti.AnalysisCallback delegate, Function asVirtualFile) { + this.delegate = delegate; + this.asVirtualFile = asVirtualFile; + } + + @Override + public void api(SourceFile sourceFile, xsbti.api.ClassLike classApi) { + delegate.api(asVirtualFile.apply(sourceFile), classApi); + } + + @Override + public void startSource(SourceFile sourceFile) { + delegate.startSource(asVirtualFile.apply(sourceFile)); + } + + @Override + public void mainClass(SourceFile sourceFile, String className) { + delegate.mainClass(asVirtualFile.apply(sourceFile), className); + } + + @Override + public boolean enabled() { + return delegate.enabled(); + } + + @Override + public void usedName(String className, String name, java.util.EnumSet useScopes) { + delegate.usedName(className, name, useScopes); + } + + @Override + public void binaryDependency(java.nio.file.Path onBinaryEntry, String onBinaryClassName, String fromClassName, SourceFile fromSourceFile, xsbti.api.DependencyContext context) { + delegate.binaryDependency(onBinaryEntry, onBinaryClassName, fromClassName, asVirtualFile.apply(fromSourceFile), context); + } + + @Override + public void classDependency(String onClassName, String sourceClassName, xsbti.api.DependencyContext context) { + delegate.classDependency(onClassName, sourceClassName, context); + } + + @Override + public void generatedLocalClass(SourceFile source, java.nio.file.Path classFile) { + delegate.generatedLocalClass(asVirtualFile.apply(source), classFile); + } + + @Override + public void generatedNonLocalClass(SourceFile source, java.nio.file.Path classFile, String binaryClassName, String srcClassName) { + delegate.generatedNonLocalClass(asVirtualFile.apply(source), classFile, binaryClassName, srcClassName); + } +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java b/sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java new file mode 100644 index 000000000000..4f141509b51b --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java @@ -0,0 +1,74 @@ +package dotty.tools.xsbt; + +import dotty.tools.dotc.interfaces.SourceFile; +import java.util.function.Function; +import java.util.Optional; + +import java.io.File; + +/** To be compatible with the Zinc 1.3 API */ +public final class OldIncrementalCallback implements dotty.tools.dotc.sbt.interfaces.IncrementalCallback { + + private final xsbti.AnalysisCallback delegate; + + public OldIncrementalCallback(xsbti.AnalysisCallback delegate) { + this.delegate = delegate; + } + + private static File asJavaFile(SourceFile sourceFile) { + File jfileOrNull = sourceFile.jfile().orElse(null); + if (jfileOrNull != null) return jfileOrNull; + throw new IllegalArgumentException("SourceFile " + sourceFile + " is not backed by a java.io.File"); + } + + @SuppressWarnings("deprecation") + @Override + public void api(SourceFile sourceFile, xsbti.api.ClassLike classApi) { + delegate.api(asJavaFile(sourceFile), classApi); + } + + @SuppressWarnings("deprecation") + @Override + public void startSource(SourceFile sourceFile) { + delegate.startSource(asJavaFile(sourceFile)); + } + + @SuppressWarnings("deprecation") + @Override + public void mainClass(SourceFile sourceFile, String className) { + delegate.mainClass(asJavaFile(sourceFile), className); + } + + @Override + public boolean enabled() { + return delegate.enabled(); + } + + @Override + public void usedName(String className, String name, java.util.EnumSet useScopes) { + delegate.usedName(className, name, useScopes); + } + + @SuppressWarnings("deprecation") + @Override + public void binaryDependency(java.nio.file.Path onBinaryEntry, String onBinaryClassName, String fromClassName, SourceFile fromSourceFile, xsbti.api.DependencyContext context) { + delegate.binaryDependency(onBinaryEntry.toFile(), onBinaryClassName, fromClassName, asJavaFile(fromSourceFile), context); + } + + @Override + public void classDependency(String onClassName, String sourceClassName, xsbti.api.DependencyContext context) { + delegate.classDependency(onClassName, sourceClassName, context); + } + + @SuppressWarnings("deprecation") + @Override + public void generatedLocalClass(SourceFile source, java.nio.file.Path classFile) { + delegate.generatedLocalClass(asJavaFile(source), classFile.toFile()); + } + + @SuppressWarnings("deprecation") + @Override + public void generatedNonLocalClass(SourceFile source, java.nio.file.Path classFile, String binaryClassName, String srcClassName) { + delegate.generatedNonLocalClass(asJavaFile(source), classFile.toFile(), binaryClassName, srcClassName); + } +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java index dad8a80ef798..9179b461d288 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java @@ -7,7 +7,7 @@ import xsbti.PathBasedFile; -public class ZincPlainFile extends dotty.tools.io.PlainFile { +public class ZincPlainFile extends dotty.tools.io.PlainFile implements AbstractZincFile { private final PathBasedFile _underlying; public ZincPlainFile(PathBasedFile underlying) { diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java index 74813f87fb20..5c242fe779d0 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java @@ -12,7 +12,7 @@ import java.io.InputStream; import java.io.OutputStream; -public class ZincVirtualFile extends dotty.tools.io.VirtualFile { +public class ZincVirtualFile extends dotty.tools.io.VirtualFile implements AbstractZincFile { private final VirtualFile _underlying; public ZincVirtualFile(VirtualFile underlying) throws IOException { diff --git a/sbt-bridge/src/xsbt/CachedCompilerImpl.java b/sbt-bridge/src/xsbt/CachedCompilerImpl.java index 1310c76f70db..8b7779f9c9cb 100644 --- a/sbt-bridge/src/xsbt/CachedCompilerImpl.java +++ b/sbt-bridge/src/xsbt/CachedCompilerImpl.java @@ -13,6 +13,9 @@ import dotty.tools.dotc.Main; import dotty.tools.xsbt.InterfaceCompileFailed; import dotty.tools.xsbt.DelegatingReporter; +import dotty.tools.xsbt.OldIncrementalCallback; + +import dotty.tools.dotc.sbt.interfaces.IncrementalCallback; // deprecation warnings are suppressed because scala3-sbt-bridge must stay compatible with Zinc 1.3 // see https://github.com/lampepfl/dotty/issues/10816 @@ -60,8 +63,10 @@ synchronized public void run(File[] sources, DependencyChanges changes, Analysis return msg; }); + IncrementalCallback incCallback = new OldIncrementalCallback(callback); + Context ctx = new ContextBase().initialCtx().fresh() - .setSbtCallback(callback) + .setIncCallback(incCallback) .setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath())); dotty.tools.dotc.reporting.Reporter reporter = Main.process(commandArguments(sources), ctx); diff --git a/sbt-bridge/src/xsbt/CompilerInterface.java b/sbt-bridge/src/xsbt/CompilerInterface.java index 3f26036eee6d..c48ee4c9d909 100644 --- a/sbt-bridge/src/xsbt/CompilerInterface.java +++ b/sbt-bridge/src/xsbt/CompilerInterface.java @@ -54,6 +54,7 @@ private boolean isClassLoaderValid() { } } + @SuppressWarnings("deprecation") public void run(File[] sources, DependencyChanges changes, AnalysisCallback callback, Logger log, Reporter delegate, CompileProgress progress, CachedCompiler cached) { cached.run(sources, changes, callback, log, delegate, progress); diff --git a/sbt-test/sbt-bridge/zinc-13-compat/test b/sbt-test/sbt-bridge/zinc-13-compat/test index d37c2434b1a3..f7b3295e155c 100644 --- a/sbt-test/sbt-bridge/zinc-13-compat/test +++ b/sbt-test/sbt-bridge/zinc-13-compat/test @@ -1,3 +1,3 @@ # this little app test that scala3-sbt-bridge is compatible with Zinc 1.3 # this is necessary to maintain the compatibility with Bloop (see https://github.com/lampepfl/dotty/issues/10816) --> run +> run From e45ab8fe5f5c0770cbb20cc08ea1d39d376cda36 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Thu, 13 Jul 2023 15:28:59 +0200 Subject: [PATCH 04/10] cache source file for TASTy sources [Cherry-picked 95ed5824bcd6e957861239130c08d0b70d913bc0] --- .../src/dotty/tools/dotc/CompilationUnit.scala | 2 +- compiler/src/dotty/tools/dotc/core/Contexts.scala | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/CompilationUnit.scala b/compiler/src/dotty/tools/dotc/CompilationUnit.scala index c121fbaf7c00..3119c5ea11a6 100644 --- a/compiler/src/dotty/tools/dotc/CompilationUnit.scala +++ b/compiler/src/dotty/tools/dotc/CompilationUnit.scala @@ -115,7 +115,7 @@ object CompilationUnit { /** Make a compilation unit for top class `clsd` with the contents of the `unpickled` tree */ def apply(clsd: ClassDenotation, unpickled: Tree, forceTrees: Boolean)(using Context): CompilationUnit = val file = clsd.symbol.associatedFile.nn - apply(SourceFile(file, Array.empty[Char]), unpickled, forceTrees) + apply(ctx.getEmptySource(file), unpickled, forceTrees) /** Make a compilation unit, given picked bytes and unpickled tree */ def apply(source: SourceFile, unpickled: Tree, forceTrees: Boolean)(using Context): CompilationUnit = { diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 31b809297005..1b0e7739f22b 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -246,17 +246,26 @@ object Contexts { /** Sourcefile corresponding to given abstract file, memoized */ def getSource(file: AbstractFile, codec: => Codec = Codec(settings.encoding.value)) = { util.Stats.record("Context.getSource") + computeCachedSource(file)(SourceFile(_, codec)) + } + + /** empty Sourcefile associated to given abstract file, memoized */ + def getEmptySource(file: AbstractFile) = { + util.Stats.record("Context.getEmptySource") + computeCachedSource(file)(SourceFile(_, Array.empty[Char])) + } + + private inline def computeCachedSource(file: AbstractFile)(inline mkSource: AbstractFile => SourceFile): SourceFile = base.sources.getOrElseUpdate(file, { val zincSources = zincInitialFiles val cachedFile = if zincSources != null then zincSources.lookup(file) match case null => file - case cached => cached + case cached: AbstractFile => cached else file - SourceFile(cachedFile, codec) + mkSource(cachedFile) }) - } /** SourceFile with given path name, memoized */ def getSource(path: TermName): SourceFile = getFile(path) match From fdad5b073db65cd5e08f8622347b9a74f0c47890 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Thu, 13 Jul 2023 21:52:08 +0200 Subject: [PATCH 05/10] create fallback virtualfile and warn [Cherry-picked 7dfb0cf5fbf56fbfe53134ca469ddc020715fdea] --- .../tools/xsbt/CompilerBridgeDriver.java | 26 +++++++++++++++-- .../dotty/tools/xsbt/DelegatingReporter.java | 9 ++++++ .../tools/xsbt/PlaceholderVirtualFile.java | 29 +++++++++++++++++++ sbt-test/tasty-compat/add-overload/build.sbt | 2 +- 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java diff --git a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java index e5e456226cc9..20c4af87ff0a 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java +++ b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.Comparator; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Arrays; @@ -62,6 +63,25 @@ private static VirtualFile asVirtualFileOrNull(SourceFile sourceFile) { } } + private static void reportMissingFile(DelegatingReporter reporter, dotty.tools.dotc.interfaces.SourceFile sourceFile) { + String underline = String.join("", Collections.nCopies(sourceFile.path().length(), "^")); + String message = + sourceFile.path() + ": Missing virtual file\n" + + underline + "\n" + + " Falling back to placeholder for the given source file (of class " + sourceFile.getClass().getName() + ")\n" + + " This is likely a bug in incremental compilation for the Scala 3 compiler. Please report it to the Scala 3 maintainers."; + reporter.reportBasicWarning(message); + } + + private static VirtualFile fallbackVirtualFile(DelegatingReporter reporter, + dotty.tools.dotc.interfaces.SourceFile sourceFile, + Map placeholders) { + return placeholders.computeIfAbsent(sourceFile.path(), path -> { + reportMissingFile(reporter, sourceFile); + return new PlaceholderVirtualFile(sourceFile); + }); + } + synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, Logger log, Reporter delegate) { VirtualFile[] sortedSources = new VirtualFile[sources.length]; System.arraycopy(sources, 0, sortedSources, 0, sources.length); @@ -86,6 +106,8 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L } }); + HashMap placeholders = new HashMap<>(); + IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile -> { if (sourceFile instanceof SourceFile) { SourceFile sf = (SourceFile) sourceFile; @@ -93,10 +115,10 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L if (vf != null) { return vf; } else { - throw new IllegalStateException("Unknown source file: " + sourceFile.path()); + return fallbackVirtualFile(reporter, sourceFile, placeholders); } } else { - throw new IllegalStateException("Unknown source file: " + sourceFile.path()); + return fallbackVirtualFile(reporter, sourceFile, placeholders); } }); diff --git a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java index f45e3768378c..222c3e7da808 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java +++ b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java @@ -19,6 +19,7 @@ import xsbti.Position; import xsbti.Severity; +import java.util.Collections; import java.util.function.*; final public class DelegatingReporter extends AbstractReporter { @@ -60,6 +61,14 @@ public void doReport(Diagnostic dia, Context ctx) { delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions, lookup)); } + public void reportBasicWarning(String message) { + Position position = PositionBridge.noPosition; + Severity severity = Severity.Warn; + String diagnosticCode = "-1"; // no error code + List actions = Collections.emptyList(); + delegate.log(new Problem(position, message, severity, message, diagnosticCode, actions, lookup)); + } + private static Severity severityOf(int level) { Severity severity; switch (level) { diff --git a/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java new file mode 100644 index 000000000000..4c30162a1b6c --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java @@ -0,0 +1,29 @@ +package dotty.tools.xsbt; + +import dotty.tools.dotc.interfaces.SourceFile; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +public final class PlaceholderVirtualFile extends xsbti.BasicVirtualFileRef implements xsbti.VirtualFile { + + private final SourceFile sourceFile; + + public PlaceholderVirtualFile(SourceFile sourceFile) { + super(sourceFile.path()); + this.sourceFile = sourceFile; + } + + private static byte[] toBytes(char[] chars) { + return new String(chars).getBytes(StandardCharsets.UTF_8); + } + + public InputStream input() { + return new java.io.ByteArrayInputStream(toBytes(sourceFile.content())); + } + + public long contentHash() { + int murmurHash3 = scala.util.hashing.MurmurHash3$.MODULE$.bytesHash(toBytes(sourceFile.content())); + return scala.util.hashing.package$.MODULE$.byteswap64((long) murmurHash3); + } + +} diff --git a/sbt-test/tasty-compat/add-overload/build.sbt b/sbt-test/tasty-compat/add-overload/build.sbt index 82dc596134c8..52a04f07148e 100644 --- a/sbt-test/tasty-compat/add-overload/build.sbt +++ b/sbt-test/tasty-compat/add-overload/build.sbt @@ -16,7 +16,7 @@ lazy val `a-changes` = project.in(file("a-changes")) lazy val c = project.in(file(".")) .settings( - scalacOptions ++= Seq("-from-tasty", "-Ycheck:readTasty"), + scalacOptions ++= Seq("-from-tasty", "-Ycheck:readTasty", "-Xfatal-warnings"), Compile / sources := Seq(new java.io.File("c-input/B.tasty")), Compile / unmanagedClasspath += (ThisBuild / baseDirectory).value / "c-input", Compile / classDirectory := (ThisBuild / baseDirectory).value / "c-output" From 1494b8761ca91f6162400ee85b562d7424f853a3 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Thu, 13 Jul 2023 22:33:24 +0200 Subject: [PATCH 06/10] always fallback when missing virtual file [Cherry-picked 8f128fd79ffdcd2c86f6f04377ba2eed2d00fdd4] --- .../dotty/tools/xsbt/BasicPathBasedFile.java | 15 ++++++++++ .../tools/xsbt/CompilerBridgeDriver.java | 30 +++++++------------ .../dotty/tools/xsbt/DelegatingReporter.java | 6 ++-- .../tools/xsbt/PlaceholderVirtualFile.java | 4 +-- sbt-bridge/src/dotty/tools/xsbt/Problem.java | 1 + sbt-bridge/src/xsbt/CachedCompilerImpl.java | 2 +- sbt-bridge/src/xsbt/DottydocRunner.java | 2 +- 7 files changed, 35 insertions(+), 25 deletions(-) create mode 100644 sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java diff --git a/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java b/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java new file mode 100644 index 000000000000..7820ef908e1a --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java @@ -0,0 +1,15 @@ +package dotty.tools.xsbt; + +import dotty.tools.dotc.interfaces.SourceFile; + +public class BasicPathBasedFile extends PlaceholderVirtualFile implements xsbti.PathBasedFile { + + public BasicPathBasedFile(SourceFile sourceFile) { + super(sourceFile); + } + + public java.nio.file.Path toPath() { + return sourceFile.jfile().get().toPath(); + } + +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java index 20c4af87ff0a..78176705f2eb 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java +++ b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java @@ -55,11 +55,12 @@ public boolean sourcesRequired() { return false; } - private static VirtualFile asVirtualFileOrNull(SourceFile sourceFile) { + private static VirtualFile asVirtualFile(SourceFile sourceFile, DelegatingReporter reporter, + Map placeholders) { if (sourceFile.file() instanceof AbstractZincFile) { return ((AbstractZincFile) sourceFile.file()).underlying(); } else { - return null; + return fallbackVirtualFile(reporter, sourceFile, placeholders); } } @@ -78,7 +79,10 @@ private static VirtualFile fallbackVirtualFile(DelegatingReporter reporter, Map placeholders) { return placeholders.computeIfAbsent(sourceFile.path(), path -> { reportMissingFile(reporter, sourceFile); - return new PlaceholderVirtualFile(sourceFile); + if (sourceFile.jfile().isPresent()) + return new BasicPathBasedFile(sourceFile); + else + return new PlaceholderVirtualFile(sourceFile); }); } @@ -97,26 +101,14 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L sourcesSet.put(abstractFile); } - DelegatingReporter reporter = new DelegatingReporter(delegate, sourceFile -> { - VirtualFile vf = asVirtualFileOrNull(sourceFile); - if (vf != null) { - return vf.id(); - } else { - return sourceFile.path(); // same as in Zinc for 2.13 - } - }); - HashMap placeholders = new HashMap<>(); + DelegatingReporter reporter = new DelegatingReporter(delegate, (self, sourceFile) -> + asVirtualFile(sourceFile, self, placeholders).id()); + IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile -> { if (sourceFile instanceof SourceFile) { - SourceFile sf = (SourceFile) sourceFile; - VirtualFile vf = asVirtualFileOrNull(sf); - if (vf != null) { - return vf; - } else { - return fallbackVirtualFile(reporter, sourceFile, placeholders); - } + return asVirtualFile(((SourceFile) sourceFile), reporter, placeholders); } else { return fallbackVirtualFile(reporter, sourceFile, placeholders); } diff --git a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java index 222c3e7da808..64f8e6d4717e 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java +++ b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java @@ -24,12 +24,14 @@ final public class DelegatingReporter extends AbstractReporter { private xsbti.Reporter delegate; + private final BiFunction baseLookup; private final Function lookup; - public DelegatingReporter(xsbti.Reporter delegate, Function lookup) { + public DelegatingReporter(xsbti.Reporter delegate, BiFunction baseLookup) { super(); this.delegate = delegate; - this.lookup = lookup; + this.baseLookup = baseLookup; + this.lookup = sourceFile -> baseLookup.apply(this, sourceFile); } public void dropDelegate() { diff --git a/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java index 4c30162a1b6c..ea7a1f6a85db 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java @@ -4,9 +4,9 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; -public final class PlaceholderVirtualFile extends xsbti.BasicVirtualFileRef implements xsbti.VirtualFile { +public class PlaceholderVirtualFile extends xsbti.BasicVirtualFileRef implements xsbti.VirtualFile { - private final SourceFile sourceFile; + protected final SourceFile sourceFile; public PlaceholderVirtualFile(SourceFile sourceFile) { super(sourceFile.path()); diff --git a/sbt-bridge/src/dotty/tools/xsbt/Problem.java b/sbt-bridge/src/dotty/tools/xsbt/Problem.java index 9eed04056752..7fc1ef2679b1 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/Problem.java +++ b/sbt-bridge/src/dotty/tools/xsbt/Problem.java @@ -16,6 +16,7 @@ import xsbti.Position; import xsbti.Severity; +import xsbti.VirtualFile; final public class Problem implements xsbti.Problem { diff --git a/sbt-bridge/src/xsbt/CachedCompilerImpl.java b/sbt-bridge/src/xsbt/CachedCompilerImpl.java index 8b7779f9c9cb..e05712f53cea 100644 --- a/sbt-bridge/src/xsbt/CachedCompilerImpl.java +++ b/sbt-bridge/src/xsbt/CachedCompilerImpl.java @@ -67,7 +67,7 @@ synchronized public void run(File[] sources, DependencyChanges changes, Analysis Context ctx = new ContextBase().initialCtx().fresh() .setIncCallback(incCallback) - .setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath())); + .setReporter(new DelegatingReporter(delegate, (self, source) -> source.file().absolutePath())); dotty.tools.dotc.reporting.Reporter reporter = Main.process(commandArguments(sources), ctx); if (reporter.hasErrors()) { diff --git a/sbt-bridge/src/xsbt/DottydocRunner.java b/sbt-bridge/src/xsbt/DottydocRunner.java index a91ff087cea9..53cef72328a7 100644 --- a/sbt-bridge/src/xsbt/DottydocRunner.java +++ b/sbt-bridge/src/xsbt/DottydocRunner.java @@ -53,7 +53,7 @@ public void run() { args = retained.toArray(new String[retained.size()]); Context ctx = new ContextBase().initialCtx().fresh() - .setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath())); + .setReporter(new DelegatingReporter(delegate, (self, source) -> source.file().absolutePath())); try { Class dottydocMainClass = Class.forName("dotty.tools.dottydoc.Main"); From 730dd486b2654cd1cee8c5b5cfe3a342d7bb82bc Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 17 Jul 2023 15:28:56 +0200 Subject: [PATCH 07/10] only use hashmap lookup to map to virtualfile [Cherry-picked 5fde72c8e6709c23c33f75e0104bbd892a4d3f88] --- .../src/dotty/tools/backend/jvm/CodeGen.scala | 3 +- .../dotty/tools/dotc/CompilationUnit.scala | 2 +- .../src/dotty/tools/dotc/core/Contexts.scala | 25 +------------ .../tools/dotc/sbt/ExtractDependencies.scala | 21 +++++------ .../sbt/interfaces/IncrementalCallback.java | 2 +- .../dotty/tools/xsbt/AbstractZincFile.java | 7 ---- .../dotty/tools/xsbt/BasicPathBasedFile.java | 4 +- .../tools/xsbt/CompilerBridgeDriver.java | 37 ++++++++----------- .../dotty/tools/xsbt/IncrementalCallback.java | 2 +- .../tools/xsbt/OldIncrementalCallback.java | 4 +- .../tools/xsbt/PlaceholderVirtualFile.java | 2 +- .../src/dotty/tools/xsbt/ZincPlainFile.java | 10 +---- .../src/dotty/tools/xsbt/ZincVirtualFile.java | 9 +---- 13 files changed, 39 insertions(+), 89 deletions(-) delete mode 100644 sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java diff --git a/compiler/src/dotty/tools/backend/jvm/CodeGen.scala b/compiler/src/dotty/tools/backend/jvm/CodeGen.scala index 84d85a02ecf8..4e00597d5b96 100644 --- a/compiler/src/dotty/tools/backend/jvm/CodeGen.scala +++ b/compiler/src/dotty/tools/backend/jvm/CodeGen.scala @@ -30,6 +30,7 @@ import scala.tools.asm import scala.tools.asm.tree._ import tpd._ import dotty.tools.io.AbstractFile +import dotty.tools.dotc.util import dotty.tools.dotc.util.NoSourcePosition @@ -106,7 +107,7 @@ class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)( } // Creates a callback that will be evaluated in PostProcessor after creating a file - private def onFileCreated(cls: ClassNode, claszSymbol: Symbol, sourceFile: interfaces.SourceFile): AbstractFile => Unit = clsFile => { + private def onFileCreated(cls: ClassNode, claszSymbol: Symbol, sourceFile: util.SourceFile): AbstractFile => Unit = clsFile => { val (fullClassName, isLocal) = atPhase(sbtExtractDependenciesPhase) { (ExtractDependencies.classNameAsString(claszSymbol), claszSymbol.isLocal) } diff --git a/compiler/src/dotty/tools/dotc/CompilationUnit.scala b/compiler/src/dotty/tools/dotc/CompilationUnit.scala index 3119c5ea11a6..c121fbaf7c00 100644 --- a/compiler/src/dotty/tools/dotc/CompilationUnit.scala +++ b/compiler/src/dotty/tools/dotc/CompilationUnit.scala @@ -115,7 +115,7 @@ object CompilationUnit { /** Make a compilation unit for top class `clsd` with the contents of the `unpickled` tree */ def apply(clsd: ClassDenotation, unpickled: Tree, forceTrees: Boolean)(using Context): CompilationUnit = val file = clsd.symbol.associatedFile.nn - apply(ctx.getEmptySource(file), unpickled, forceTrees) + apply(SourceFile(file, Array.empty[Char]), unpickled, forceTrees) /** Make a compilation unit, given picked bytes and unpickled tree */ def apply(source: SourceFile, unpickled: Tree, forceTrees: Boolean)(using Context): CompilationUnit = { diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 1b0e7739f22b..08cef076b47d 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -53,9 +53,8 @@ object Contexts { private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]() private val (importInfoLoc, store9) = store8.newLocation[ImportInfo | Null]() private val (typeAssignerLoc, store10) = store9.newLocation[TypeAssigner](TypeAssigner) - private val (zincInitialFilesLoc, store11) = store10.newLocation[util.ReadOnlySet[AbstractFile] | Null]() - private val initialStore = store11 + private val initialStore = store10 /** The current context */ inline def ctx(using ctx: Context): Context = ctx @@ -167,7 +166,6 @@ object Contexts { /** The Zinc callback implementation if we are run from Zinc, null otherwise */ def incCallback: IncrementalCallback | Null = store(incCallbackLoc) - def zincInitialFiles: util.ReadOnlySet[AbstractFile] | Null = store(zincInitialFilesLoc) /** Run `op` if there exists an incremental callback */ inline def withIncCallback(inline op: IncrementalCallback => Unit): Unit = @@ -246,27 +244,9 @@ object Contexts { /** Sourcefile corresponding to given abstract file, memoized */ def getSource(file: AbstractFile, codec: => Codec = Codec(settings.encoding.value)) = { util.Stats.record("Context.getSource") - computeCachedSource(file)(SourceFile(_, codec)) + base.sources.getOrElseUpdate(file, SourceFile(file, codec)) } - /** empty Sourcefile associated to given abstract file, memoized */ - def getEmptySource(file: AbstractFile) = { - util.Stats.record("Context.getEmptySource") - computeCachedSource(file)(SourceFile(_, Array.empty[Char])) - } - - private inline def computeCachedSource(file: AbstractFile)(inline mkSource: AbstractFile => SourceFile): SourceFile = - base.sources.getOrElseUpdate(file, { - val zincSources = zincInitialFiles - val cachedFile = - if zincSources != null then zincSources.lookup(file) match - case null => file - case cached: AbstractFile => cached - else - file - mkSource(cachedFile) - }) - /** SourceFile with given path name, memoized */ def getSource(path: TermName): SourceFile = getFile(path) match case NoAbstractFile => NoSource @@ -695,7 +675,6 @@ object Contexts { def setCompilerCallback(callback: CompilerCallback): this.type = updateStore(compilerCallbackLoc, callback) def setIncCallback(callback: IncrementalCallback): this.type = updateStore(incCallbackLoc, callback) - def setZincInitialFiles(zincInitialFiles: util.ReadOnlySet[AbstractFile]): this.type = updateStore(zincInitialFilesLoc, zincInitialFiles) def setPrinterFn(printer: Context => Printer): this.type = updateStore(printerFnLoc, printer) def setSettings(settingsState: SettingsState): this.type = updateStore(settingsStateLoc, settingsState) def setRun(run: Run | Null): this.type = updateStore(runLoc, run) diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index 131dbdb9ba15..d6de2a8c51e8 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -91,19 +91,16 @@ class ExtractDependencies extends Phase { } finally pw.close() } - if (ctx.incrementalEnabled) { - ctx.withIncCallback: cb => - collector.usedNames.foreach { - case (clazz, usedNames) => - val className = classNameAsString(clazz) - usedNames.names.foreach { - case (usedName, scopes) => - cb.usedName(className, usedName.toString, scopes) - } - } - + ctx.withIncCallback: cb => + collector.usedNames.foreach { + case (clazz, usedNames) => + val className = classNameAsString(clazz) + usedNames.names.foreach { + case (usedName, scopes) => + cb.usedName(className, usedName.toString, scopes) + } + } collector.dependencies.foreach(recordDependency) - } } /* diff --git a/compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java b/compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java index e0a45784bd94..4c6afa113f4f 100644 --- a/compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java +++ b/compiler/src/dotty/tools/dotc/sbt/interfaces/IncrementalCallback.java @@ -1,6 +1,6 @@ package dotty.tools.dotc.sbt.interfaces; -import dotty.tools.dotc.interfaces.SourceFile; +import dotty.tools.dotc.util.SourceFile; import java.util.EnumSet; import java.nio.file.Path; diff --git a/sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java b/sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java deleted file mode 100644 index 788bd74fceab..000000000000 --- a/sbt-bridge/src/dotty/tools/xsbt/AbstractZincFile.java +++ /dev/null @@ -1,7 +0,0 @@ -package dotty.tools.xsbt; - -import xsbti.VirtualFile; - -interface AbstractZincFile { - VirtualFile underlying(); -} diff --git a/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java b/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java index 7820ef908e1a..8c1207999bd0 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java @@ -1,6 +1,6 @@ package dotty.tools.xsbt; -import dotty.tools.dotc.interfaces.SourceFile; +import dotty.tools.dotc.util.SourceFile; public class BasicPathBasedFile extends PlaceholderVirtualFile implements xsbti.PathBasedFile { @@ -9,7 +9,7 @@ public BasicPathBasedFile(SourceFile sourceFile) { } public java.nio.file.Path toPath() { - return sourceFile.jfile().get().toPath(); + return sourceFile.file().jpath(); } } diff --git a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java index 78176705f2eb..e59f1768d358 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java +++ b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java @@ -56,15 +56,16 @@ public boolean sourcesRequired() { } private static VirtualFile asVirtualFile(SourceFile sourceFile, DelegatingReporter reporter, - Map placeholders) { - if (sourceFile.file() instanceof AbstractZincFile) { - return ((AbstractZincFile) sourceFile.file()).underlying(); + HashMap lookup, Map placeholders) { + VirtualFile maybeCached = lookup.get(sourceFile.file()); + if (maybeCached != null) { + return maybeCached; } else { return fallbackVirtualFile(reporter, sourceFile, placeholders); } } - private static void reportMissingFile(DelegatingReporter reporter, dotty.tools.dotc.interfaces.SourceFile sourceFile) { + private static void reportMissingFile(DelegatingReporter reporter, SourceFile sourceFile) { String underline = String.join("", Collections.nCopies(sourceFile.path().length(), "^")); String message = sourceFile.path() + ": Missing virtual file\n" + @@ -74,12 +75,11 @@ private static void reportMissingFile(DelegatingReporter reporter, dotty.tools.d reporter.reportBasicWarning(message); } - private static VirtualFile fallbackVirtualFile(DelegatingReporter reporter, - dotty.tools.dotc.interfaces.SourceFile sourceFile, - Map placeholders) { - return placeholders.computeIfAbsent(sourceFile.path(), path -> { + private static VirtualFile fallbackVirtualFile(DelegatingReporter reporter, SourceFile sourceFile, + Map placeholders) { + return placeholders.computeIfAbsent(sourceFile.file(), path -> { reportMissingFile(reporter, sourceFile); - if (sourceFile.jfile().isPresent()) + if (sourceFile.file().jpath() != null) return new BasicPathBasedFile(sourceFile); else return new PlaceholderVirtualFile(sourceFile); @@ -92,27 +92,23 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L Arrays.sort(sortedSources, (x0, x1) -> x0.id().compareTo(x1.id())); ListBuffer sourcesBuffer = new ListBuffer<>(); - dotty.tools.dotc.util.HashSet sourcesSet = new dotty.tools.dotc.util.HashSet<>(8, 2); + HashMap lookup = new HashMap<>(sources.length, 0.25f); for (int i = 0; i < sources.length; i++) { VirtualFile source = sortedSources[i]; AbstractFile abstractFile = asDottyFile(source); sourcesBuffer.append(abstractFile); - sourcesSet.put(abstractFile); + lookup.put(abstractFile, source); } - HashMap placeholders = new HashMap<>(); + HashMap placeholders = new HashMap<>(); DelegatingReporter reporter = new DelegatingReporter(delegate, (self, sourceFile) -> - asVirtualFile(sourceFile, self, placeholders).id()); + asVirtualFile(sourceFile, self, lookup, placeholders).id()); - IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile -> { - if (sourceFile instanceof SourceFile) { - return asVirtualFile(((SourceFile) sourceFile), reporter, placeholders); - } else { - return fallbackVirtualFile(reporter, sourceFile, placeholders); - } - }); + IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile -> + asVirtualFile(sourceFile, reporter, lookup, placeholders) + ); try { log.debug(this::infoOnCachedCompiler); @@ -120,7 +116,6 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L Contexts.Context initialCtx = initCtx() .fresh() .setReporter(reporter) - .setZincInitialFiles(sourcesSet) .setIncCallback(incCallback); Contexts.Context context = setup(args, initialCtx).map(t -> t._2).getOrElse(() -> initialCtx); diff --git a/sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java b/sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java index 58032cbeb72c..3c3d33c1c1fe 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java +++ b/sbt-bridge/src/dotty/tools/xsbt/IncrementalCallback.java @@ -1,6 +1,6 @@ package dotty.tools.xsbt; -import dotty.tools.dotc.interfaces.SourceFile; +import dotty.tools.dotc.util.SourceFile; import java.util.function.Function; public final class IncrementalCallback implements dotty.tools.dotc.sbt.interfaces.IncrementalCallback { diff --git a/sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java b/sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java index 4f141509b51b..597a964eb944 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java +++ b/sbt-bridge/src/dotty/tools/xsbt/OldIncrementalCallback.java @@ -1,6 +1,6 @@ package dotty.tools.xsbt; -import dotty.tools.dotc.interfaces.SourceFile; +import dotty.tools.dotc.util.SourceFile; import java.util.function.Function; import java.util.Optional; @@ -16,7 +16,7 @@ public OldIncrementalCallback(xsbti.AnalysisCallback delegate) { } private static File asJavaFile(SourceFile sourceFile) { - File jfileOrNull = sourceFile.jfile().orElse(null); + File jfileOrNull = sourceFile.file().file(); if (jfileOrNull != null) return jfileOrNull; throw new IllegalArgumentException("SourceFile " + sourceFile + " is not backed by a java.io.File"); } diff --git a/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java index ea7a1f6a85db..1255e0c05c6e 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java @@ -1,6 +1,6 @@ package dotty.tools.xsbt; -import dotty.tools.dotc.interfaces.SourceFile; +import dotty.tools.dotc.util.SourceFile; import java.io.InputStream; import java.nio.charset.StandardCharsets; diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java index 9179b461d288..55754019c96b 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java @@ -7,16 +7,8 @@ import xsbti.PathBasedFile; -public class ZincPlainFile extends dotty.tools.io.PlainFile implements AbstractZincFile { - private final PathBasedFile _underlying; - +public class ZincPlainFile extends dotty.tools.io.PlainFile { public ZincPlainFile(PathBasedFile underlying) { super(new dotty.tools.io.Path(underlying.toPath())); - this._underlying = underlying; - } - - @Override - public PathBasedFile underlying() { - return _underlying; } } diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java index 5c242fe779d0..22ddcaa3fd27 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java @@ -12,12 +12,10 @@ import java.io.InputStream; import java.io.OutputStream; -public class ZincVirtualFile extends dotty.tools.io.VirtualFile implements AbstractZincFile { - private final VirtualFile _underlying; +public class ZincVirtualFile extends dotty.tools.io.VirtualFile { public ZincVirtualFile(VirtualFile underlying) throws IOException { super(underlying.name(), underlying.id()); - this._underlying = underlying; // fill in the content try (OutputStream output = output()) { @@ -32,9 +30,4 @@ public InputStream inputStream() { } } } - - @Override - public VirtualFile underlying() { - return _underlying; - } } From 46a27922022c0b84e15faf77a5e5c573eb6540b1 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 17 Jul 2023 16:29:56 +0200 Subject: [PATCH 08/10] remove placeholders map [Cherry-picked aa2758060d7b69bef0f07c27b3bb846cf4cba2c8] --- .../tools/xsbt/CompilerBridgeDriver.java | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java index e59f1768d358..94dbad3c55e4 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java +++ b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java @@ -56,13 +56,14 @@ public boolean sourcesRequired() { } private static VirtualFile asVirtualFile(SourceFile sourceFile, DelegatingReporter reporter, - HashMap lookup, Map placeholders) { - VirtualFile maybeCached = lookup.get(sourceFile.file()); - if (maybeCached != null) { - return maybeCached; - } else { - return fallbackVirtualFile(reporter, sourceFile, placeholders); - } + HashMap lookup) { + return lookup.computeIfAbsent(sourceFile.file(), path -> { + reportMissingFile(reporter, sourceFile); + if (sourceFile.file().jpath() != null) + return new BasicPathBasedFile(sourceFile); + else + return new PlaceholderVirtualFile(sourceFile); + }); } private static void reportMissingFile(DelegatingReporter reporter, SourceFile sourceFile) { @@ -75,17 +76,6 @@ private static void reportMissingFile(DelegatingReporter reporter, SourceFile so reporter.reportBasicWarning(message); } - private static VirtualFile fallbackVirtualFile(DelegatingReporter reporter, SourceFile sourceFile, - Map placeholders) { - return placeholders.computeIfAbsent(sourceFile.file(), path -> { - reportMissingFile(reporter, sourceFile); - if (sourceFile.file().jpath() != null) - return new BasicPathBasedFile(sourceFile); - else - return new PlaceholderVirtualFile(sourceFile); - }); - } - synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, Logger log, Reporter delegate) { VirtualFile[] sortedSources = new VirtualFile[sources.length]; System.arraycopy(sources, 0, sortedSources, 0, sources.length); @@ -101,13 +91,12 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L lookup.put(abstractFile, source); } - HashMap placeholders = new HashMap<>(); - DelegatingReporter reporter = new DelegatingReporter(delegate, (self, sourceFile) -> - asVirtualFile(sourceFile, self, lookup, placeholders).id()); + asVirtualFile(sourceFile, self, lookup).id() + ); IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile -> - asVirtualFile(sourceFile, reporter, lookup, placeholders) + asVirtualFile(sourceFile, reporter, lookup) ); try { From f53e8f90c7c0848195c54ade26878ea9454c38cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Fri, 1 Dec 2023 13:25:24 +0100 Subject: [PATCH 09/10] simplify file comparison [Cherry-picked 246dfc4ac58d8afaf14010788e97f736baecc596][modified] --- compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index d6de2a8c51e8..c675bb0e623e 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -138,15 +138,12 @@ class ExtractDependencies extends Phase { val depFile = dep.to.associatedFile if (depFile != null) { - def depIsSameSource = - depFile.absolutePath == sourceFile.file.absolutePath - // Cannot ignore inheritance relationship coming from the same source (see sbt/zinc#417) def allowLocal = dep.context == DependencyByInheritance || dep.context == LocalDependencyByInheritance if (depFile.extension == "class") { // Dependency is external -- source is undefined processExternalDependency(depFile, dep.to.binaryClassName) - else if (allowLocal || !depIsSameSource /* old: depFile.file != sourceFile.file */) { + } else if (allowLocal || depFile != sourceFile.file) { // We cannot ignore dependencies coming from the same source file because // the dependency info needs to propagate. See source-dependencies/trait-trait-211. val toClassName = classNameAsString(dep.to) From 8b58415749e523c84beab17ba2a6753ddf09e4fd Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Fri, 21 Jul 2023 12:23:19 +0200 Subject: [PATCH 10/10] address comments [Cherry-picked e3de91c04ef12262cc3f3390cbef5a3f76387c54] --- .../src/dotty/tools/dotc/core/Contexts.scala | 6 +-- .../src/dotty/tools/dotc/sbt/ExtractAPI.scala | 3 +- .../tools/dotc/sbt/ExtractDependencies.scala | 14 ++--- .../dotty/tools/xsbt/BasicPathBasedFile.java | 15 ------ .../tools/xsbt/CompilerBridgeDriver.java | 53 +++++++++++++++---- .../dotty/tools/xsbt/DelegatingReporter.java | 19 ++++--- .../tools/xsbt/FallbackPathBasedFile.java | 20 +++++++ ...tualFile.java => FallbackVirtualFile.java} | 13 +++-- sbt-bridge/src/dotty/tools/xsbt/Problem.java | 20 ++++--- .../src/dotty/tools/xsbt/ZincPlainFile.java | 14 ----- .../src/dotty/tools/xsbt/ZincVirtualFile.java | 33 ------------ sbt-bridge/src/xsbt/CachedCompilerImpl.java | 2 +- sbt-bridge/src/xsbt/DottydocRunner.java | 2 +- 13 files changed, 105 insertions(+), 109 deletions(-) delete mode 100644 sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java create mode 100644 sbt-bridge/src/dotty/tools/xsbt/FallbackPathBasedFile.java rename sbt-bridge/src/dotty/tools/xsbt/{PlaceholderVirtualFile.java => FallbackVirtualFile.java} (52%) delete mode 100644 sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java delete mode 100644 sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 08cef076b47d..8a7f2ff4e051 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -172,10 +172,10 @@ object Contexts { val local = incCallback if local != null then op(local) - def incrementalEnabled: Boolean = + def runZincPhases: Boolean = + def forceRun = settings.YdumpSbtInc.value || settings.YforceSbtPhases.value val local = incCallback - if local != null then local.enabled - else false + local != null && local.enabled || forceRun /** The current plain printer */ def printerFn: Context => Printer = store(printerFnLoc) diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala index 0c5168f3c21a..5ecf17be32a9 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala @@ -49,8 +49,7 @@ class ExtractAPI extends Phase { override def description: String = ExtractAPI.description override def isRunnable(using Context): Boolean = { - def forceRun = ctx.settings.YdumpSbtInc.value || ctx.settings.YforceSbtPhases.value - super.isRunnable && (ctx.incrementalEnabled || forceRun) + super.isRunnable && ctx.runZincPhases } // Check no needed. Does not transform trees diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index c675bb0e623e..ac3136340625 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -55,8 +55,7 @@ class ExtractDependencies extends Phase { override def description: String = ExtractDependencies.description override def isRunnable(using Context): Boolean = { - def forceRun = ctx.settings.YdumpSbtInc.value || ctx.settings.YforceSbtPhases.value - super.isRunnable && (ctx.incrementalEnabled || forceRun) + super.isRunnable && ctx.runZincPhases } // Check no needed. Does not transform trees @@ -122,15 +121,8 @@ class ExtractDependencies extends Phase { case Some(zip) if zip.jpath != null => binaryDependency(zip.jpath, binaryClassName) case _ => - case pf: PlainFile => // The dependency comes from a class file - // FIXME: pf.file is null for classfiles coming from the modulepath - // (handled by JrtClassPath) because they cannot be represented as - // java.io.File, since the `binaryDependency` callback must take a - // java.io.File, this means that we cannot record dependencies coming - // from the modulepath. For now this isn't a big deal since we only - // support having the standard Java library on the modulepath. - if pf.jpath != null then - binaryDependency(pf.jpath, binaryClassName) + case pf: PlainFile => // The dependency comes from a class file, Zinc handles JRT filesystem + binaryDependency(pf.jpath, binaryClassName) case _ => internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.from.srcPos) } diff --git a/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java b/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java deleted file mode 100644 index 8c1207999bd0..000000000000 --- a/sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java +++ /dev/null @@ -1,15 +0,0 @@ -package dotty.tools.xsbt; - -import dotty.tools.dotc.util.SourceFile; - -public class BasicPathBasedFile extends PlaceholderVirtualFile implements xsbti.PathBasedFile { - - public BasicPathBasedFile(SourceFile sourceFile) { - super(sourceFile); - } - - public java.nio.file.Path toPath() { - return sourceFile.file().jpath(); - } - -} diff --git a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java index 94dbad3c55e4..6a018287ea53 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java +++ b/sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java @@ -12,6 +12,9 @@ import dotty.tools.dotc.core.Contexts; import dotty.tools.dotc.util.SourceFile; import dotty.tools.io.AbstractFile; +import dotty.tools.io.PlainFile; +import dotty.tools.io.Path; +import dotty.tools.io.Streamable; import scala.collection.mutable.ListBuffer; import scala.jdk.javaapi.CollectionConverters; import scala.io.Codec; @@ -20,6 +23,8 @@ import xsbti.compile.Output; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.util.Comparator; import java.util.Collections; import java.util.HashMap; @@ -60,19 +65,20 @@ private static VirtualFile asVirtualFile(SourceFile sourceFile, DelegatingReport return lookup.computeIfAbsent(sourceFile.file(), path -> { reportMissingFile(reporter, sourceFile); if (sourceFile.file().jpath() != null) - return new BasicPathBasedFile(sourceFile); + return new FallbackPathBasedFile(sourceFile); else - return new PlaceholderVirtualFile(sourceFile); + return new FallbackVirtualFile(sourceFile); }); } private static void reportMissingFile(DelegatingReporter reporter, SourceFile sourceFile) { String underline = String.join("", Collections.nCopies(sourceFile.path().length(), "^")); String message = - sourceFile.path() + ": Missing virtual file\n" + + sourceFile.path() + ": Missing Zinc virtual file\n" + underline + "\n" + " Falling back to placeholder for the given source file (of class " + sourceFile.getClass().getName() + ")\n" + - " This is likely a bug in incremental compilation for the Scala 3 compiler. Please report it to the Scala 3 maintainers."; + " This is likely a bug in incremental compilation for the Scala 3 compiler.\n" + + " Please report it to the Scala 3 maintainers at https://github.com/lampepfl/dotty/issues."; reporter.reportBasicWarning(message); } @@ -91,9 +97,19 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L lookup.put(abstractFile, source); } - DelegatingReporter reporter = new DelegatingReporter(delegate, (self, sourceFile) -> - asVirtualFile(sourceFile, self, lookup).id() - ); + DelegatingReporter reporter = new DelegatingReporter(delegate, sourceFile -> { + // TODO: possible situation here where we use -from-tasty and TASTy source files but + // the reporter log is associated to a Scala source file? + + // Zinc will use the output of this function to possibly lookup a mapped virtual file, + // e.g. convert `${ROOT}/Foo.scala` to `/path/to/Foo.scala` if it exists in the lookup map. + VirtualFile vf = lookup.get(sourceFile.file()); + if (vf != null) + return vf.id(); + else + // follow Zinc, which uses the path of the source file as a fallback. + return sourceFile.path(); + }); IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile -> asVirtualFile(sourceFile, reporter, lookup) @@ -137,11 +153,28 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L } private static AbstractFile asDottyFile(VirtualFile virtualFile) { - if (virtualFile instanceof PathBasedFile) - return new ZincPlainFile((PathBasedFile) virtualFile); + if (virtualFile instanceof PathBasedFile) { + java.nio.file.Path path = ((PathBasedFile) virtualFile).toPath(); + return new PlainFile(new Path(path)); + } try { - return new ZincVirtualFile(virtualFile); + return new dotty.tools.io.VirtualFile(virtualFile.name(), virtualFile.id()) { + { + // fill in the content + try (OutputStream output = output()) { + try (InputStream input = virtualFile.input()) { + Streamable.Bytes bytes = new Streamable.Bytes() { + @Override + public InputStream inputStream() { + return input; + } + }; + output.write(bytes.toByteArray()); + } + } + } + }; } catch (IOException e) { throw new IllegalArgumentException("invalid file " + virtualFile.name(), e); } diff --git a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java index 64f8e6d4717e..e6ddbc51ea32 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java +++ b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java @@ -24,14 +24,16 @@ final public class DelegatingReporter extends AbstractReporter { private xsbti.Reporter delegate; - private final BiFunction baseLookup; - private final Function lookup; - public DelegatingReporter(xsbti.Reporter delegate, BiFunction baseLookup) { + // A function that can lookup the `id` of the VirtualFile + // associated with a SourceFile. If there is not an associated virtual file, + // then it is the path of the SourceFile as a String. + private final Function lookupVirtualFileId; + + public DelegatingReporter(xsbti.Reporter delegate, Function lookupVirtualFileId) { super(); this.delegate = delegate; - this.baseLookup = baseLookup; - this.lookup = sourceFile -> baseLookup.apply(this, sourceFile); + this.lookupVirtualFileId = lookupVirtualFileId; } public void dropDelegate() { @@ -60,7 +62,8 @@ public void doReport(Diagnostic dia, Context ctx) { messageBuilder.append(System.lineSeparator()).append(explanation(message, ctx)); } - delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions, lookup)); + delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions, + lookupVirtualFileId)); } public void reportBasicWarning(String message) { @@ -68,7 +71,7 @@ public void reportBasicWarning(String message) { Severity severity = Severity.Warn; String diagnosticCode = "-1"; // no error code List actions = Collections.emptyList(); - delegate.log(new Problem(position, message, severity, message, diagnosticCode, actions, lookup)); + delegate.log(new Problem(position, message, severity, message, diagnosticCode, actions, lookupVirtualFileId)); } private static Severity severityOf(int level) { @@ -85,7 +88,7 @@ private static Severity severityOf(int level) { private Position positionOf(SourcePosition pos) { if (pos.exists()) { - return new PositionBridge(pos, lookup.apply(pos.source())); + return new PositionBridge(pos, lookupVirtualFileId.apply(pos.source())); } else { return PositionBridge.noPosition; } diff --git a/sbt-bridge/src/dotty/tools/xsbt/FallbackPathBasedFile.java b/sbt-bridge/src/dotty/tools/xsbt/FallbackPathBasedFile.java new file mode 100644 index 000000000000..28c2170d2b50 --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/FallbackPathBasedFile.java @@ -0,0 +1,20 @@ +package dotty.tools.xsbt; + +import dotty.tools.dotc.util.SourceFile; + +/**A basic implementation of PathBasedFile that is only used when + * the real virtual file can not be found. + * + * See FallbackVirtualFile for more details. + */ +public class FallbackPathBasedFile extends FallbackVirtualFile implements xsbti.PathBasedFile { + + public FallbackPathBasedFile(SourceFile sourceFile) { + super(sourceFile); + } + + public java.nio.file.Path toPath() { + return sourceFile.file().jpath(); + } + +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/FallbackVirtualFile.java similarity index 52% rename from sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java rename to sbt-bridge/src/dotty/tools/xsbt/FallbackVirtualFile.java index 1255e0c05c6e..6fcb6ef73e1f 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/PlaceholderVirtualFile.java +++ b/sbt-bridge/src/dotty/tools/xsbt/FallbackVirtualFile.java @@ -4,11 +4,18 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; -public class PlaceholderVirtualFile extends xsbti.BasicVirtualFileRef implements xsbti.VirtualFile { +/**A basic implementation of VirtualFile that is only used when + * the real virtual file can not be found. + * + * This has a very basic implementation of contentHash that is almost certainly colliding more than the implementation + * in Zinc. It does not matter anyway as Zinc will recompile the associated source file, because it did not recieve the + * same virtual file back. + */ +public class FallbackVirtualFile extends xsbti.BasicVirtualFileRef implements xsbti.VirtualFile { protected final SourceFile sourceFile; - public PlaceholderVirtualFile(SourceFile sourceFile) { + public FallbackVirtualFile(SourceFile sourceFile) { super(sourceFile.path()); this.sourceFile = sourceFile; } @@ -23,7 +30,7 @@ public InputStream input() { public long contentHash() { int murmurHash3 = scala.util.hashing.MurmurHash3$.MODULE$.bytesHash(toBytes(sourceFile.content())); - return scala.util.hashing.package$.MODULE$.byteswap64((long) murmurHash3); + return (long) murmurHash3; } } diff --git a/sbt-bridge/src/dotty/tools/xsbt/Problem.java b/sbt-bridge/src/dotty/tools/xsbt/Problem.java index 7fc1ef2679b1..532bb35786c4 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/Problem.java +++ b/sbt-bridge/src/dotty/tools/xsbt/Problem.java @@ -26,10 +26,14 @@ final public class Problem implements xsbti.Problem { private final Optional _rendered; private final String _diagnosticCode; private final List _actions; - private final Function _lookup; + + // A function that can lookup the `id` of the VirtualFile + // associated with a SourceFile. If there is not an associated virtual file, + // then it is the path of the SourceFile as a String. + private final Function _lookupVirtualFileId; public Problem(Position position, String message, Severity severity, String rendered, String diagnosticCode, List actions, - Function lookup) { + Function lookupVirtualFileId) { super(); this._position = position; this._message = message; @@ -37,7 +41,7 @@ public Problem(Position position, String message, Severity severity, String rend this._rendered = Optional.of(rendered); this._diagnosticCode = diagnosticCode; this._actions = actions; - this._lookup = lookup; + this._lookupVirtualFileId = lookupVirtualFileId; } public String category() { @@ -86,23 +90,23 @@ public List actions() { // never getting called. return _actions .stream() - .map(action -> new Action(action.title(), OptionConverters.toJava(action.description()), toWorkspaceEdit(CollectionConverters.asJava(action.patches()), _lookup))) + .map(action -> new Action(action.title(), OptionConverters.toJava(action.description()), toWorkspaceEdit(CollectionConverters.asJava(action.patches()), _lookupVirtualFileId))) .collect(toList()); } } - private static WorkspaceEdit toWorkspaceEdit(List patches, Function lookup) { + private static WorkspaceEdit toWorkspaceEdit(List patches, Function lookupVirtualFileId) { return new WorkspaceEdit( patches .stream() - .map(patch -> new TextEdit(positionOf(patch.srcPos(), lookup), patch.replacement())) + .map(patch -> new TextEdit(positionOf(patch.srcPos(), lookupVirtualFileId), patch.replacement())) .collect(toList()) ); } - private static Position positionOf(SourcePosition pos, Function lookup) { + private static Position positionOf(SourcePosition pos, Function lookupVirtualFileId) { if (pos.exists()){ - return new PositionBridge(pos, lookup.apply(pos.source())); + return new PositionBridge(pos, lookupVirtualFileId.apply(pos.source())); } else { return PositionBridge.noPosition; } diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java deleted file mode 100644 index 55754019c96b..000000000000 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Zinc - The incremental compiler for Scala. - * Copyright Lightbend, Inc. and Mark Harrah - */ - -package dotty.tools.xsbt; - -import xsbti.PathBasedFile; - -public class ZincPlainFile extends dotty.tools.io.PlainFile { - public ZincPlainFile(PathBasedFile underlying) { - super(new dotty.tools.io.Path(underlying.toPath())); - } -} diff --git a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java b/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java deleted file mode 100644 index 22ddcaa3fd27..000000000000 --- a/sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Zinc - The incremental compiler for Scala. - * Copyright Lightbend, Inc. and Mark Harrah - */ - -package dotty.tools.xsbt; - -import dotty.tools.io.Streamable; -import xsbti.VirtualFile; - -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; - -public class ZincVirtualFile extends dotty.tools.io.VirtualFile { - - public ZincVirtualFile(VirtualFile underlying) throws IOException { - super(underlying.name(), underlying.id()); - - // fill in the content - try (OutputStream output = output()) { - try (InputStream input = underlying.input()) { - Streamable.Bytes bytes = new Streamable.Bytes() { - @Override - public InputStream inputStream() { - return input; - } - }; - output.write(bytes.toByteArray()); - } - } - } -} diff --git a/sbt-bridge/src/xsbt/CachedCompilerImpl.java b/sbt-bridge/src/xsbt/CachedCompilerImpl.java index e05712f53cea..8b7779f9c9cb 100644 --- a/sbt-bridge/src/xsbt/CachedCompilerImpl.java +++ b/sbt-bridge/src/xsbt/CachedCompilerImpl.java @@ -67,7 +67,7 @@ synchronized public void run(File[] sources, DependencyChanges changes, Analysis Context ctx = new ContextBase().initialCtx().fresh() .setIncCallback(incCallback) - .setReporter(new DelegatingReporter(delegate, (self, source) -> source.file().absolutePath())); + .setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath())); dotty.tools.dotc.reporting.Reporter reporter = Main.process(commandArguments(sources), ctx); if (reporter.hasErrors()) { diff --git a/sbt-bridge/src/xsbt/DottydocRunner.java b/sbt-bridge/src/xsbt/DottydocRunner.java index 53cef72328a7..a91ff087cea9 100644 --- a/sbt-bridge/src/xsbt/DottydocRunner.java +++ b/sbt-bridge/src/xsbt/DottydocRunner.java @@ -53,7 +53,7 @@ public void run() { args = retained.toArray(new String[retained.size()]); Context ctx = new ContextBase().initialCtx().fresh() - .setReporter(new DelegatingReporter(delegate, (self, source) -> source.file().absolutePath())); + .setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath())); try { Class dottydocMainClass = Class.forName("dotty.tools.dottydoc.Main");