Skip to content

Commit

Permalink
Prioritize TASTy files over classfiles on classpath aggregation
Browse files Browse the repository at this point in the history
In most cases the TASTy file is chosen over the classfile in a classpath
because they are packaged together. However, for the `scala-library`
(Scala 2 compiled library) and `scala2-library-tasty` (Scala 3 compiled Scala 2 library)
we have the classfiles in one jar and the TASTy files in another jar.
Given that the classpaths order in guaranteed to be deterministic we might
end up with the classfile being loaded first and the TASTy file second.
The aggregator must be capable of choosing the TASTy file over the classfile
in this case as well. The aggregator will only choose the new TASTy over
the old classfile if the TASTy file has no classfile in its classpath. In
other words, we only use this new behaviour for TASTy only classpaths.

This also implies that we can just add the `scala2-library-tasty` as a
dependency in any project to use it. Note that this jar is not published
yet.
  • Loading branch information
nicolasstucki committed Jan 30, 2024
1 parent a20027f commit d0d5fbe
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 77 deletions.
38 changes: 26 additions & 12 deletions compiler/src/dotty/tools/dotc/classpath/AggregateClassPath.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath {
def findEntry(isSource: Boolean): Option[ClassRepresentation] =
aggregatesForPackage(PackageName(pkg)).iterator.map(_.findClass(className)).collectFirst {
case Some(s: SourceFileEntry) if isSource => s
case Some(s: ClassFileEntry) if !isSource => s
case Some(s: BinaryFileEntry) if !isSource => s
}

val classEntry = findEntry(isSource = false)
val sourceEntry = findEntry(isSource = true)

(classEntry, sourceEntry) match {
case (Some(c: ClassFileEntry), Some(s: SourceFileEntry)) => Some(ClassAndSourceFilesEntry(c.file, s.file))
case (Some(c: BinaryFileEntry), Some(s: SourceFileEntry)) => Some(BinaryAndSourceFilesEntry(c, s))
case (c @ Some(_), _) => c
case (_, s) => s
}
Expand All @@ -63,7 +63,7 @@ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath {
aggregatedPackages
}

override private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] =
override private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] =
getDistinctEntries(_.classes(inPackage))

override private[dotty] def sources(inPackage: PackageName): Seq[SourceFileEntry] =
Expand Down Expand Up @@ -102,10 +102,17 @@ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath {
ClassPathEntries(distinctPackages, distinctClassesAndSources)
}

/**
* Returns only one entry for each name. If there's both a source and a class entry, it
* creates an entry containing both of them. If there would be more than one class or source
* entries for the same class it always would use the first entry of each type found on a classpath.
/** Returns only one entry for each name.
*
* If there's both a source and a class entry, it
* creates an entry containing both of them. If there would be more than one class or source
* entries for the same class it always would use the first entry of each type found on a classpath.
*
* A class entry with a TASTy file will be chosen over one with a class file. Usually the class entries
* are already TASTy files when loading Scala 3 classes because the other classpath loaders load the TASTy.
* There is one exception if we load the Scala 2 library as it has one JAR containing the class files and one
* JAR containing the TASTy files. As classpath orders are not guaranteed to be deterministic we might end up
* having the TASTy in a later classpath entry.
*/
private def mergeClassesAndSources(entries: scala.collection.Seq[ClassRepresentation]): Seq[ClassRepresentation] = {
// based on the implementation from MergedClassPath
Expand All @@ -119,11 +126,18 @@ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath {
if (indices.contains(name)) {
val index = indices(name)
val existing = mergedEntries(index)

if (existing.binary.isEmpty && entry.binary.isDefined)
mergedEntries(index) = ClassAndSourceFilesEntry(entry.binary.get, existing.source.get)
if (existing.source.isEmpty && entry.source.isDefined)
mergedEntries(index) = ClassAndSourceFilesEntry(existing.binary.get, entry.source.get)
(entry, existing) match
case (entry: SourceFileEntry, existing: BinaryFileEntry) =>
mergedEntries(index) = BinaryAndSourceFilesEntry(existing, entry)
case (entry: BinaryFileEntry, existing: SourceFileEntry) =>
mergedEntries(index) = BinaryAndSourceFilesEntry(entry, existing)
case (entry: StandaloneTastyFileEntry, _: ClassFileEntry) =>
// Here we do not create a TastyFileEntry(entry.file) because the TASTy and the classfile
// come from different classpaths. These may not have the same TASTy UUID.
mergedEntries(index) = entry
case (entry: StandaloneTastyFileEntry, BinaryAndSourceFilesEntry(_: ClassFileEntry, sourceEntry)) =>
mergedEntries(index) = BinaryAndSourceFilesEntry(entry, sourceEntry)
case _ =>
}
else {
indices(name) = count
Expand Down
55 changes: 35 additions & 20 deletions compiler/src/dotty/tools/dotc/classpath/ClassPath.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
package dotty.tools.dotc.classpath

import dotty.tools.dotc.classpath.FileUtils.isTasty
import dotty.tools.io.AbstractFile
import dotty.tools.io.ClassRepresentation

Expand All @@ -14,14 +15,6 @@ object ClassPathEntries {
val empty = ClassPathEntries(Seq.empty, Seq.empty)
}

trait ClassFileEntry extends ClassRepresentation {
def file: AbstractFile
}

trait SourceFileEntry extends ClassRepresentation {
def file: AbstractFile
}

case class PackageName(dottedString: String) {
val dirPathTrailingSlashJar: String = FileUtils.dirPathInJar(dottedString) + "/"

Expand All @@ -48,28 +41,50 @@ trait PackageEntry {
def name: String
}

private[dotty] case class ClassFileEntryImpl(file: AbstractFile) extends ClassFileEntry {
/** A TASTy file or classfile */
sealed trait BinaryFileEntry extends ClassRepresentation {
def file: AbstractFile
final def fileName: String = file.name
def name: String = FileUtils.stripClassExtension(file.name) // class name
final def name: String = FileUtils.stripClassExtension(file.name) // class name
final def source: Option[AbstractFile] = None
}

object BinaryFileEntry {
def apply(file: AbstractFile): BinaryFileEntry =
if file.isTasty then
if file.resolveSiblingWithExtension("class") != null then TastyFileEntry(file)
else StandaloneTastyFileEntry(file)
else
ClassFileEntry(file)
}

/** A classfile or .sig that does not have an associated TASTy file */
private[dotty] final case class ClassFileEntry(file: AbstractFile) extends BinaryFileEntry {
def binary: Option[AbstractFile] = Some(file)
def source: Option[AbstractFile] = None
}

private[dotty] case class SourceFileEntryImpl(file: AbstractFile) extends SourceFileEntry {
/** A TASTy file that has an associated TASTy */
private[dotty] final case class TastyFileEntry(file: AbstractFile) extends BinaryFileEntry {
def binary: Option[AbstractFile] = Some(file)
}

/** A TASTy file that does not have an associated TASTy */
private[dotty] final case class StandaloneTastyFileEntry(file: AbstractFile) extends BinaryFileEntry {
def binary: Option[AbstractFile] = Some(file)
}

private[dotty] final case class SourceFileEntry(file: AbstractFile) extends ClassRepresentation {
final def fileName: String = file.name
def name: String = FileUtils.stripSourceExtension(file.name)

def binary: Option[AbstractFile] = None
def source: Option[AbstractFile] = Some(file)
}

private[dotty] case class ClassAndSourceFilesEntry(classFile: AbstractFile, srcFile: AbstractFile) extends ClassRepresentation {
final def fileName: String = classFile.name
def name: String = FileUtils.stripClassExtension(classFile.name)

def binary: Option[AbstractFile] = Some(classFile)
def source: Option[AbstractFile] = Some(srcFile)
private[dotty] final case class BinaryAndSourceFilesEntry(binaryEntry: BinaryFileEntry, sourceEntry: SourceFileEntry) extends ClassRepresentation {
final def fileName: String = binaryEntry.fileName
def name: String = binaryEntry.name
def binary: Option[AbstractFile] = binaryEntry.binary
def source: Option[AbstractFile] = sourceEntry.source
}

private[dotty] case class PackageEntryImpl(name: String) extends PackageEntry
Expand All @@ -81,5 +96,5 @@ private[dotty] trait NoSourcePaths {

private[dotty] trait NoClassPaths {
def findClassFile(className: String): Option[AbstractFile] = None
private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] = Seq.empty
private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] = Seq.empty
}
26 changes: 14 additions & 12 deletions compiler/src/dotty/tools/dotc/classpath/DirectoryClassPath.scala
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,12 @@ final class JrtClassPath(fs: java.nio.file.FileSystem) extends ClassPath with No
override private[dotty] def packages(inPackage: PackageName): Seq[PackageEntry] =
packageToModuleBases.keysIterator.filter(pack => packageContains(inPackage.dottedString, pack)).map(PackageEntryImpl(_)).toVector

private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] =
private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] =
if (inPackage.isRoot) Nil
else
packageToModuleBases.getOrElse(inPackage.dottedString, Nil).flatMap(x =>
Files.list(x.resolve(inPackage.dirPathTrailingSlash)).iterator().asScala.filter(_.getFileName.toString.endsWith(".class"))).map(x =>
ClassFileEntryImpl(x.toPlainFile)).toVector
ClassFileEntry(x.toPlainFile)).toVector

override private[dotty] def list(inPackage: PackageName): ClassPathEntries =
if (inPackage.isRoot) ClassPathEntries(packages(inPackage), Nil)
Expand Down Expand Up @@ -246,12 +246,12 @@ final class CtSymClassPath(ctSym: java.nio.file.Path, release: Int) extends Clas
override private[dotty] def packages(inPackage: PackageName): Seq[PackageEntry] = {
packageIndex.keysIterator.filter(pack => packageContains(inPackage.dottedString, pack)).map(PackageEntryImpl(_)).toVector
}
private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] = {
private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] = {
if (inPackage.isRoot) Nil
else {
val sigFiles = packageIndex.getOrElse(inPackage.dottedString, Nil).iterator.flatMap(p =>
Files.list(p).iterator.asScala.filter(_.getFileName.toString.endsWith(".sig")))
sigFiles.map(f => ClassFileEntryImpl(f.toPlainFile)).toVector
sigFiles.map(f => ClassFileEntry(f.toPlainFile)).toVector
}
}

Expand All @@ -273,33 +273,35 @@ final class CtSymClassPath(ctSym: java.nio.file.Path, release: Int) extends Clas
}
}

case class DirectoryClassPath(dir: JFile) extends JFileDirectoryLookup[ClassFileEntryImpl] with NoSourcePaths {
override def findClass(className: String): Option[ClassRepresentation] = findClassFile(className) map ClassFileEntryImpl.apply
case class DirectoryClassPath(dir: JFile) extends JFileDirectoryLookup[BinaryFileEntry] with NoSourcePaths {
override def findClass(className: String): Option[ClassRepresentation] =
findClassFile(className).map(BinaryFileEntry(_))

def findClassFile(className: String): Option[AbstractFile] = {
val relativePath = FileUtils.dirPath(className)
val tastyFile = new JFile(dir, relativePath + ".tasty")
if tastyFile.exists then Some(tastyFile.toPath.toPlainFile)
else
val classFile = new JFile(dir, relativePath + ".class")
if classFile.exists then Some(classFile.toPath.toPlainFile)
if classFile.exists then Some(classFile.toPath.toPlainFile)
else None
}

protected def createFileEntry(file: AbstractFile): ClassFileEntryImpl = ClassFileEntryImpl(file)
protected def createFileEntry(file: AbstractFile): BinaryFileEntry = BinaryFileEntry(file)

protected def isMatchingFile(f: JFile): Boolean =
f.isTasty || (f.isClass && f.classToTasty.isEmpty)

private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] = files(inPackage)
private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] = files(inPackage)
}

case class DirectorySourcePath(dir: JFile) extends JFileDirectoryLookup[SourceFileEntryImpl] with NoClassPaths {
case class DirectorySourcePath(dir: JFile) extends JFileDirectoryLookup[SourceFileEntry] with NoClassPaths {
def asSourcePathString: String = asClassPathString

protected def createFileEntry(file: AbstractFile): SourceFileEntryImpl = SourceFileEntryImpl(file)
protected def createFileEntry(file: AbstractFile): SourceFileEntry = SourceFileEntry(file)
protected def isMatchingFile(f: JFile): Boolean = endsScalaOrJava(f.getName)

override def findClass(className: String): Option[ClassRepresentation] = findSourceFile(className) map SourceFileEntryImpl.apply
override def findClass(className: String): Option[ClassRepresentation] = findSourceFile(className).map(SourceFileEntry(_))

private def findSourceFile(className: String): Option[AbstractFile] = {
val relativePath = FileUtils.dirPath(className)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import dotty.tools.io.{AbstractFile, VirtualDirectory}
import FileUtils.*
import java.net.{URI, URL}

case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath with DirectoryLookup[ClassFileEntryImpl] with NoSourcePaths {
case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath with DirectoryLookup[BinaryFileEntry] with NoSourcePaths {
type F = AbstractFile

// From AbstractFileClassLoader
Expand Down Expand Up @@ -38,7 +38,8 @@ case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath wi
def asURLs: Seq[URL] = Seq(new URI(dir.name).toURL)
def asClassPathStrings: Seq[String] = Seq(dir.path)

override def findClass(className: String): Option[ClassRepresentation] = findClassFile(className) map ClassFileEntryImpl.apply
override def findClass(className: String): Option[ClassRepresentation] =
findClassFile(className).map(BinaryFileEntry(_))

def findClassFile(className: String): Option[AbstractFile] = {
val pathSeq = FileUtils.dirPath(className).split(java.io.File.separator)
Expand All @@ -49,9 +50,10 @@ case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath wi
.orElse(Option(lookupPath(parentDir)(pathSeq.last + ".class" :: Nil, directory = false)))
}

private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] = files(inPackage)
private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] = files(inPackage)

protected def createFileEntry(file: AbstractFile): BinaryFileEntry = BinaryFileEntry(file)

protected def createFileEntry(file: AbstractFile): ClassFileEntryImpl = ClassFileEntryImpl(file)
protected def isMatchingFile(f: AbstractFile): Boolean =
f.isTasty || (f.isClass && f.classToTasty.isEmpty)
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,23 @@ sealed trait ZipAndJarFileLookupFactory {
*/
object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory {
private case class ZipArchiveClassPath(zipFile: File, override val release: Option[String])
extends ZipArchiveFileLookup[ClassFileEntryImpl]
extends ZipArchiveFileLookup[BinaryFileEntry]
with NoSourcePaths {

override def findClassFile(className: String): Option[AbstractFile] =
findClass(className).map(_.file)

// This method is performance sensitive as it is used by SBT's ExtractDependencies phase.
override def findClass(className: String): Option[ClassFileEntryImpl] = {
override def findClass(className: String): Option[BinaryFileEntry] = {
val (pkg, simpleClassName) = PackageNameUtils.separatePkgAndClassNames(className)
val binaries = files(PackageName(pkg), simpleClassName + ".tasty", simpleClassName + ".class")
binaries.find(_.file.isTasty).orElse(binaries.find(_.file.isClass))
}

override private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] = files(inPackage)
override private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] = files(inPackage)

override protected def createFileEntry(file: FileZipArchive#Entry): BinaryFileEntry = BinaryFileEntry(file)

override protected def createFileEntry(file: FileZipArchive#Entry): ClassFileEntryImpl = ClassFileEntryImpl(file)
override protected def isRequiredFileType(file: AbstractFile): Boolean =
file.isTasty || (file.isClass && file.classToTasty.isEmpty)
}
Expand Down Expand Up @@ -128,10 +129,10 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory {
subpackages.map(packageFile => PackageEntryImpl(inPackage.entryName(packageFile.name)))
}

override private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry] = cachedPackages.get(inPackage.dottedString) match {
override private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry] = cachedPackages.get(inPackage.dottedString) match {
case None => Seq.empty
case Some(PackageFileInfo(pkg, _)) =>
(for (file <- pkg if file.isClass) yield ClassFileEntryImpl(file)).toSeq
(for (file <- pkg if file.isClass) yield ClassFileEntry(file)).toSeq
}

override private[dotty] def hasPackage(pkg: PackageName) = cachedPackages.contains(pkg.dottedString)
Expand Down Expand Up @@ -162,7 +163,7 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory {
*/
object ZipAndJarSourcePathFactory extends ZipAndJarFileLookupFactory {
private case class ZipArchiveSourcePath(zipFile: File)
extends ZipArchiveFileLookup[SourceFileEntryImpl]
extends ZipArchiveFileLookup[SourceFileEntry]
with NoClassPaths {

def release: Option[String] = None
Expand All @@ -171,7 +172,7 @@ object ZipAndJarSourcePathFactory extends ZipAndJarFileLookupFactory {

override private[dotty] def sources(inPackage: PackageName): Seq[SourceFileEntry] = files(inPackage)

override protected def createFileEntry(file: FileZipArchive#Entry): SourceFileEntryImpl = SourceFileEntryImpl(file)
override protected def createFileEntry(file: FileZipArchive#Entry): SourceFileEntry = SourceFileEntry(file)
override protected def isRequiredFileType(file: AbstractFile): Boolean = file.isScalaOrJavaSource
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/io/AbstractFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ abstract class AbstractFile extends Iterable[AbstractFile] {
final def resolveSibling(name: String): AbstractFile | Null =
container.lookupName(name, directory = false)

final def resolveSiblingWithExtension(extension: String): AbstractFile | Null =
resolveSibling(name.stripSuffix(this.extension) + extension)

private def fileOrSubdirectoryNamed(name: String, isDir: Boolean): AbstractFile =
lookupName(name, isDir) match {
case null =>
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/io/ClassPath.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ trait ClassPath {

final def hasPackage(pkg: String): Boolean = hasPackage(PackageName(pkg))
final def packages(inPackage: String): Seq[PackageEntry] = packages(PackageName(inPackage))
final def classes(inPackage: String): Seq[ClassFileEntry] = classes(PackageName(inPackage))
final def classes(inPackage: String): Seq[BinaryFileEntry] = classes(PackageName(inPackage))
final def sources(inPackage: String): Seq[SourceFileEntry] = sources(PackageName(inPackage))
final def list(inPackage: String): ClassPathEntries = list(PackageName(inPackage))

Expand All @@ -43,7 +43,7 @@ trait ClassPath {

private[dotty] def hasPackage(pkg: PackageName): Boolean
private[dotty] def packages(inPackage: PackageName): Seq[PackageEntry]
private[dotty] def classes(inPackage: PackageName): Seq[ClassFileEntry]
private[dotty] def classes(inPackage: PackageName): Seq[BinaryFileEntry]
private[dotty] def sources(inPackage: PackageName): Seq[SourceFileEntry]

/**
Expand Down
Loading

0 comments on commit d0d5fbe

Please sign in to comment.