Skip to content

Commit

Permalink
Do best effort to detect associatedFile if empty
Browse files Browse the repository at this point in the history
Otherwise we may end up losing internal dependencies because the
compiler failed to do a good job at setting `associatedFile` in the
classfile parser. This is what happens in #590 with default arguments,
whose test passes after this implementation.
  • Loading branch information
jvican committed Aug 31, 2018
1 parent 80fad1f commit fd8329e
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 61 deletions.
49 changes: 0 additions & 49 deletions internal/compiler-bridge/src/main/scala/xsbt/API.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,55 +75,6 @@ final class API(val global: CallbackGlobal) extends Compat with GlobalHelpers wi

private case class FlattenedNames(binaryName: String, className: String)

/**
* Replicate the behaviour of `fullName` with a few changes to the code to produce
* correct file-system compatible full names for non-local classes. It mimics the
* paths of the class files produced by genbcode.
*
* Changes compared to the normal version in the compiler:
*
* 1. It will use the encoded name instead of the normal name.
* 2. It will not skip the name of the package object class (required for the class file path).
*
* Note that using `javaBinaryName` is not useful for these symbols because we
* need the encoded names. Zinc keeps track of encoded names in both the binary
* names and the Zinc names.
*
* @param symbol The symbol for which we extract the full name.
* @param separator The separator that we will apply between every name.
* @param suffix The suffix to add at the end (in case it's a module).
* @param includePackageObjectClassNames Include package object class names or not.
* @return The full name.
*/
def fullName(
symbol: Symbol,
separator: Char,
suffix: CharSequence,
includePackageObjectClassNames: Boolean
): String = {
var b: java.lang.StringBuffer = null
def loop(size: Int, sym: Symbol): Unit = {
val symName = sym.name
// Use of encoded to produce correct paths for names that have symbols
val encodedName = symName.encoded
val nSize = encodedName.length - (if (symName.endsWith(nme.LOCAL_SUFFIX_STRING)) 1 else 0)
if (sym.isRoot || sym.isRootPackage || sym == NoSymbol || sym.owner.isEffectiveRoot) {
val capacity = size + nSize
b = new java.lang.StringBuffer(capacity)
b.append(chrs, symName.start, nSize)
} else {
val next = if (sym.owner.isPackageObjectClass) sym.owner else sym.effectiveOwner.enclClass
loop(size + nSize + 1, next)
// Addition to normal `fullName` to produce correct names for nested non-local classes
if (sym.isNestedClass) b.append(nme.MODULE_SUFFIX_STRING) else b.append(separator)
b.append(chrs, symName.start, nSize)
}
}
loop(suffix.length(), symbol)
b.append(suffix)
b.toString
}

/**
* Registers only non-local generated classes in the callback by extracting
* information about its names and using the names to generate class file paths.
Expand Down
60 changes: 58 additions & 2 deletions internal/compiler-bridge/src/main/scala/xsbt/CallbackGlobal.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,14 @@ sealed abstract class CallbackGlobal(settings: Settings,
extends Global(settings, reporter) {

def callback: AnalysisCallback
def findClass(name: String): Option[(AbstractFile, Boolean)]
def findClasspathOriginOf(name: String): Option[(AbstractFile, Boolean)]

def fullName(
symbol: Symbol,
separator: Char,
suffix: CharSequence,
includePackageObjectClassNames: Boolean
): String

lazy val outputDirs: Iterable[File] = {
output match {
Expand Down Expand Up @@ -126,7 +133,7 @@ sealed class ZincCompiler(settings: Settings, dreporter: DelegatingReporter, out
}

/** Returns the class file location of a fully qualified name and whether it's on the classpath. */
def findClass(fqn: String): Option[(AbstractFile, Boolean)] = {
def findClasspathOriginOf(fqn: String): Option[(AbstractFile, Boolean)] = {
def getOutputClass(name: String): Option[AbstractFile] = {
// This could be improved if a hint where to look is given.
val className = name.replace('.', '/') + ".class"
Expand All @@ -139,6 +146,55 @@ sealed class ZincCompiler(settings: Settings, dreporter: DelegatingReporter, out
getOutputClass(fqn).map(f => (f, true)).orElse(findOnClassPath(fqn).map(f => (f, false)))
}

/**
* Replicate the behaviour of `fullName` with a few changes to the code to produce
* correct file-system compatible full names for non-local classes. It mimics the
* paths of the class files produced by genbcode.
*
* Changes compared to the normal version in the compiler:
*
* 1. It will use the encoded name instead of the normal name.
* 2. It will not skip the name of the package object class (required for the class file path).
*
* Note that using `javaBinaryName` is not useful for these symbols because we
* need the encoded names. Zinc keeps track of encoded names in both the binary
* names and the Zinc names.
*
* @param symbol The symbol for which we extract the full name.
* @param separator The separator that we will apply between every name.
* @param suffix The suffix to add at the end (in case it's a module).
* @param includePackageObjectClassNames Include package object class names or not.
* @return The full name.
*/
override def fullName(
symbol: Symbol,
separator: Char,
suffix: CharSequence,
includePackageObjectClassNames: Boolean
): String = {
var b: java.lang.StringBuffer = null
def loop(size: Int, sym: Symbol): Unit = {
val symName = sym.name
// Use of encoded to produce correct paths for names that have symbols
val encodedName = symName.encoded
val nSize = encodedName.length - (if (symName.endsWith(nme.LOCAL_SUFFIX_STRING)) 1 else 0)
if (sym.isRoot || sym.isRootPackage || sym == NoSymbol || sym.owner.isEffectiveRoot) {
val capacity = size + nSize
b = new java.lang.StringBuffer(capacity)
b.append(chrs, symName.start, nSize)
} else {
val next = if (sym.owner.isPackageObjectClass) sym.owner else sym.effectiveOwner.enclClass
loop(size + nSize + 1, next)
// Addition to normal `fullName` to produce correct names for nested non-local classes
if (sym.isNestedClass) b.append(nme.MODULE_SUFFIX_STRING) else b.append(separator)
b.append(chrs, symName.start, nSize)
}
}
loop(suffix.length(), symbol)
b.append(suffix)
b.toString
}

private[this] var callback0: AnalysisCallback = null

/** Returns the active analysis callback, set by [[set]] and cleared by [[clear]]. */
Expand Down
40 changes: 32 additions & 8 deletions internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,42 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
// The dependency comes from a class file
binaryDependency(pf.file, binaryClassName)
case _ =>
// TODO: If this happens, scala internals have changed. Log error.
reporter.error(
NoPosition,
s"Internal error: ${binaryClassName} comes from unknown origin ${at}"
)
}
}

val onSource = dep.to.sourceFile
val targetSymbol = dep.to
val onSource = targetSymbol.sourceFile
if (onSource == null) {
// Dependency is external -- source is undefined
classFile(dep.to) match {
case Some((at, binaryClassName)) =>
processExternalDependency(binaryClassName, at)
case None =>
debuglog(Feedback.noOriginFileForExternalSymbol(dep.to))
// Ignore packages right away as they don't map to a class file/jar
if (targetSymbol.hasFlag(scala.tools.nsc.symtab.Flags.PACKAGE)) None
// Ignore `Any` which by default has no `associatedFile`
else if (targetSymbol == definitions.AnyClass) ()
else {
classFile(targetSymbol) match {
case Some((at, binaryClassName)) =>
// Associated file is set, so we know which classpath entry it came from
processExternalDependency(binaryClassName, at)
case None =>
/* If there is no associated file, it's likely the compiler didn't set it correctly.
* This happens very rarely, see https://github.com/sbt/zinc/issues/559 as an example,
* but when it does we must ensure the incremental compiler tries its best no to lose
* any dependency. Therefore, we do a last-time effort to get the origin of the symbol
* by inspecting the classpath manually.
*/
val fqn = fullName(targetSymbol, '.', targetSymbol.moduleSuffix, false)
global.findClasspathOriginOf(fqn) match {
case Some((at, true)) =>
processExternalDependency(fqn, at)
case Some((_, false)) | None =>
// Study the possibility of warning or adding this to the zinc profiler so that
// if users reports errors, the lost dependencies are present in the zinc profiler
debuglog(Feedback.noOriginFileForExternalSymbol(targetSymbol))
}
}
}
} else if (onSource.file != sourceFile || allowLocal) {
// We cannot ignore dependencies coming from the same source file because
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,8 +431,7 @@ class ExtractAPI[GlobalType <: Global](
def mkVar = Some(fieldDef(in, sym, keepConst = false, xsbti.api.Var.of(_, _, _, _, _)))
def mkVal = Some(fieldDef(in, sym, keepConst = true, xsbti.api.Val.of(_, _, _, _, _)))
if (isClass(sym))
if (ignoreClass(sym)) {allNonLocalClassSymbols.+=(sym); None}
else Some(classLike(in, sym))
if (ignoreClass(sym)) { allNonLocalClassSymbols.+=(sym); None } else Some(classLike(in, sym))
else if (sym.isNonClassType)
Some(typeDef(in, sym))
else if (sym.isVariable)
Expand Down

0 comments on commit fd8329e

Please sign in to comment.