Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prioritize TASTy files over classfiles on classpath aggregation #19431

Merged
merged 2 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 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,15 @@ 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 TASTy file with no class file entry will be chosen over a class file entry. This can happen 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 +124,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 TastyWithClassFileEntry 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 TastyWithClassFileEntry(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 class file */
private[dotty] final case class TastyWithClassFileEntry(file: AbstractFile) extends BinaryFileEntry {
def binary: Option[AbstractFile] = Some(file)
}

/** A TASTy file that does not have an associated class file */
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
Loading