From fea18a4fbeffe4dd9f9567b40b0c50de6f4c3efb Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Thu, 24 Oct 2013 12:21:53 +0200 Subject: [PATCH 1/4] Remove AnalysisCallback.{beginSource, endSource} methods. As pointed out by @harrah in #705, both beginSource and endSource are not used in sbt internally for anything meaningful. We've discussed an option of deprecating those methods but since they are not doing anything meaningful Mark prefers to have compile-time error in case somebody implements or calls those methods. I agree with that hence removal. --- compile/inc/src/main/scala/sbt/inc/Compile.scala | 5 ----- compile/interface/src/main/scala/xsbt/Analyzer.scala | 2 -- interface/src/main/java/xsbti/AnalysisCallback.java | 4 ---- interface/src/test/scala/TestCallback.scala | 7 +------ .../src/main/scala/sbt/classfile/Analyze.scala | 12 ++++-------- 5 files changed, 5 insertions(+), 25 deletions(-) diff --git a/compile/inc/src/main/scala/sbt/inc/Compile.scala b/compile/inc/src/main/scala/sbt/inc/Compile.scala index e5dae387c..ce1803ccc 100644 --- a/compile/inc/src/main/scala/sbt/inc/Compile.scala +++ b/compile/inc/src/main/scala/sbt/inc/Compile.scala @@ -151,9 +151,6 @@ private final class AnalysisCallback(internalMap: File => Option[File], external apis(sourceFile) = (HashAPI(source), savedSource) } - def endSource(sourcePath: File): Unit = - assert(apis.contains(sourcePath)) - def get: Analysis = addCompilation( addExternals( addBinaries( addProducts( addSources(Analysis.Empty) ) ) ) ) def addProducts(base: Analysis): Analysis = addAll(base, classes) { case (a, src, (prod, name)) => a.addProduct(src, prod, current product prod, name ) } def addBinaries(base: Analysis): Analysis = addAll(base, binaryDeps)( (a, src, bin) => a.addBinaryDep(src, bin, binaryClassName(bin), current binary bin) ) @@ -178,6 +175,4 @@ private final class AnalysisCallback(internalMap: File => Option[File], external (outer /: bs) { (inner, b) => f(inner, a, b) } } - - def beginSource(source: File) {} } diff --git a/compile/interface/src/main/scala/xsbt/Analyzer.scala b/compile/interface/src/main/scala/xsbt/Analyzer.scala index 0f7737305..0e1c89bf9 100644 --- a/compile/interface/src/main/scala/xsbt/Analyzer.scala +++ b/compile/interface/src/main/scala/xsbt/Analyzer.scala @@ -31,7 +31,6 @@ final class Analyzer(val global: CallbackGlobal) extends LocateClassFile { // build dependencies structure val sourceFile = unit.source.file.file - callback.beginSource(sourceFile) for(on <- unit.depends) processDependency(on, inherited=false) for(on <- inheritedDependencies.getOrElse(sourceFile, Nil: Iterable[Symbol])) processDependency(on, inherited=true) def processDependency(on: Symbol, inherited: Boolean) @@ -75,7 +74,6 @@ final class Analyzer(val global: CallbackGlobal) extends LocateClassFile else addGenerated(false) } - callback.endSource(sourceFile) } } } diff --git a/interface/src/main/java/xsbti/AnalysisCallback.java b/interface/src/main/java/xsbti/AnalysisCallback.java index d00f5b7ed..55a90f011 100644 --- a/interface/src/main/java/xsbti/AnalysisCallback.java +++ b/interface/src/main/java/xsbti/AnalysisCallback.java @@ -7,8 +7,6 @@ import java.io.File; public interface AnalysisCallback { - /** Called before the source at the given location is processed. */ - public void beginSource(File source); /** Called to indicate that the source file source depends on the source file * dependsOn. Note that only source files included in the current compilation will * passed to this method. Dependencies on classes generated by sources not in the current compilation will @@ -24,8 +22,6 @@ public interface AnalysisCallback /** Called to indicate that the source file source produces a class file at * module contain class name.*/ public void generatedClass(File source, File module, String name); - /** Called after the source at the given location has been processed. */ - public void endSource(File sourcePath); /** Called when the public API of a source file is extracted. */ public void api(File sourceFile, xsbti.api.SourceAPI source); /** Provides problems discovered during compilation. These may be reported (logged) or unreported. diff --git a/interface/src/test/scala/TestCallback.scala b/interface/src/test/scala/TestCallback.scala index 061457723..5c0de068e 100644 --- a/interface/src/test/scala/TestCallback.scala +++ b/interface/src/test/scala/TestCallback.scala @@ -5,20 +5,15 @@ package xsbti class TestCallback extends AnalysisCallback { - val beganSources = new ArrayBuffer[File] - val endedSources = new ArrayBuffer[File] val sourceDependencies = new ArrayBuffer[(File, File, Boolean)] val binaryDependencies = new ArrayBuffer[(File, String, File, Boolean)] val products = new ArrayBuffer[(File, File, String)] val apis = new ArrayBuffer[(File, xsbti.api.SourceAPI)] - def beginSource(source: File) { beganSources += source } - def sourceDependency(dependsOn: File, source: File, inherited: Boolean) { sourceDependencies += ((dependsOn, source, inherited)) } def binaryDependency(binary: File, name: String, source: File, inherited: Boolean) { binaryDependencies += ((binary, name, source, inherited)) } def generatedClass(source: File, module: File, name: String) { products += ((source, module, name)) } - def endSource(source: File) { endedSources += source } def api(source: File, sourceAPI: xsbti.api.SourceAPI) { apis += ((source, sourceAPI)) } def problem(category: String, pos: xsbti.Position, message: String, severity: xsbti.Severity, reported: Boolean) {} -} \ No newline at end of file +} diff --git a/util/classfile/src/main/scala/sbt/classfile/Analyze.scala b/util/classfile/src/main/scala/sbt/classfile/Analyze.scala index 117b718ae..724eb1a1e 100644 --- a/util/classfile/src/main/scala/sbt/classfile/Analyze.scala +++ b/util/classfile/src/main/scala/sbt/classfile/Analyze.scala @@ -33,12 +33,11 @@ private[sbt] object Analyze sourceFile <- classFile.sourceFile orElse guessSourceName(newClass.getName); source <- guessSourcePath(sourceMap, classFile, log)) { - analysis.beginSource(source) analysis.generatedClass(source, newClass, classFile.className) productToSource(newClass) = source sourceToClassFiles.getOrElseUpdate(source, new ArrayBuffer[ClassFile]) += classFile } - + // get class to class dependencies and map back to source to class dependencies for( (source, classFiles) <- sourceToClassFiles ) { @@ -65,17 +64,14 @@ private[sbt] object Analyze } } def processDependencies(tpes: Iterable[String], inherited: Boolean): Unit = tpes.foreach(tpe => processDependency(tpe, inherited)) - + val notInherited = classFiles.flatMap(_.types).toSet -- publicInherited processDependencies(notInherited, false) processDependencies(publicInherited, true) - analysis.endSource(source) } for( source <- sources filterNot sourceToClassFiles.keySet ) { - analysis.beginSource(source) analysis.api(source, new xsbti.api.SourceAPI(Array(), Array())) - analysis.endSource(source) } } private[this] def urlAsFile(url: URL, log: Logger): Option[File] = @@ -114,7 +110,7 @@ private[sbt] object Analyze } private def findSource(sourceNameMap: Map[String, Iterable[File]], pkg: List[String], sourceFileName: String): List[File] = refine( (sourceNameMap get sourceFileName).toList.flatten.map { x => (x,x.getParentFile) }, pkg.reverse) - + @tailrec private def refine(sources: List[(File, File)], pkgRev: List[String]): List[File] = { def make = sources.map(_._1) @@ -155,7 +151,7 @@ private[sbt] object Analyze method.getReturnType == unit && method.getParameterTypes.toList == strArray private def isMain(modifiers: Int): Boolean = (modifiers & mainModifiers) == mainModifiers && (modifiers & notMainModifiers) == 0 - + private val mainModifiers = STATIC | PUBLIC private val notMainModifiers = ABSTRACT } From 838416360ae0b1c27aa0862b6856600b5ab2d652 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Thu, 24 Oct 2013 12:25:37 +0200 Subject: [PATCH 2/4] Move dependency extraction into separate compiler phase. This is the first step towards using new mechanism for dependency extraction that is based on tree walking. We need dependency extraction in separate phase because the code walking trees should run before refchecks whereas analyzer phase runs at the very end of phase pipeline. This change also includes a work-around for phase ordering issue with continuations plugin. See included comment and SI-7217 for details. --- .../src/main/scala/xsbt/Analyzer.scala | 29 +-------- .../main/scala/xsbt/CompilerInterface.scala | 55 +++++++++++++++++ .../src/main/scala/xsbt/Dependency.scala | 59 +++++++++++++++++++ 3 files changed, 115 insertions(+), 28 deletions(-) create mode 100644 compile/interface/src/main/scala/xsbt/Dependency.scala diff --git a/compile/interface/src/main/scala/xsbt/Analyzer.scala b/compile/interface/src/main/scala/xsbt/Analyzer.scala index 0e1c89bf9..dd11fe0e0 100644 --- a/compile/interface/src/main/scala/xsbt/Analyzer.scala +++ b/compile/interface/src/main/scala/xsbt/Analyzer.scala @@ -23,39 +23,13 @@ final class Analyzer(val global: CallbackGlobal) extends LocateClassFile def newPhase(prev: Phase): Phase = new AnalyzerPhase(prev) private class AnalyzerPhase(prev: Phase) extends Phase(prev) { - override def description = "Extracts dependency information, finds concrete instances of provided superclasses, and application entry points." + override def description = "Finds concrete instances of provided superclasses, and application entry points." def name = Analyzer.name def run { for(unit <- currentRun.units if !unit.isJava) { - // build dependencies structure val sourceFile = unit.source.file.file - for(on <- unit.depends) processDependency(on, inherited=false) - for(on <- inheritedDependencies.getOrElse(sourceFile, Nil: Iterable[Symbol])) processDependency(on, inherited=true) - def processDependency(on: Symbol, inherited: Boolean) - { - def binaryDependency(file: File, className: String) = callback.binaryDependency(file, className, sourceFile, inherited) - val onSource = on.sourceFile - if(onSource == null) - { - classFile(on) match - { - case Some((f,className,inOutDir)) => - if(inOutDir && on.isJavaDefined) registerTopLevelSym(on) - f match - { - case ze: ZipArchive#Entry => for(zip <- ze.underlyingSource; zipFile <- Option(zip.file) ) binaryDependency(zipFile, className) - case pf: PlainFile => binaryDependency(pf.file, className) - case _ => () - } - case None => () - } - } - else - callback.sourceDependency(onSource.file, sourceFile, inherited) - } - // build list of generated classes for(iclass <- unit.icode) { @@ -77,6 +51,5 @@ final class Analyzer(val global: CallbackGlobal) extends LocateClassFile } } } - } diff --git a/compile/interface/src/main/scala/xsbt/CompilerInterface.scala b/compile/interface/src/main/scala/xsbt/CompilerInterface.scala index 7f94d1dab..ea60f1422 100644 --- a/compile/interface/src/main/scala/xsbt/CompilerInterface.scala +++ b/compile/interface/src/main/scala/xsbt/CompilerInterface.scala @@ -176,6 +176,60 @@ private final class CachedCompiler0(args: Array[String], output: Output, initial def newPhase(prev: Phase) = analyzer.newPhase(prev) def name = phaseName } + + /** Phase that extracts dependency information */ + object sbtDependency extends + { + val global: Compiler.this.type = Compiler.this + val phaseName = Dependency.name + val runsAfter = List(API.name) + override val runsBefore = List("refchecks") + /* We set runsRightAfter to work-around a bug with phase ordering related to + * continuations plugin. See SI-7217. + * + * If runsRightAfter == None, we get the following set of phases (with continuations + * begin enabled): + * + * typer 4 the meat and potatoes: type the trees + * superaccessors 5 add super accessors in traits and nested classes + * pickler 6 serialize symbol tables + * xsbt-api 7 + * selectiveanf 8 + * xsbt-dependency 9 + * refchecks 10 reference/override checking, translate nested objects + * selectivecps 11 + * liftcode 12 reify trees + * uncurry 13 uncurry, translate function values to anonymous classes + * + * Notice that `selectiveanf` (one of continuations phases) runs before `refchecks` + * and that causes NPEs in `selectiveansf`. + * However, the default ordering for Scala 2.9.2 is: + * + * typer 4 the meat and potatoes: type the trees + * superaccessors 5 add super accessors in traits and nested classes + * pickler 6 serialize symbol tables + * refchecks 7 reference/override checking, translate nested objects + * selectiveanf 8 + * liftcode 9 reify trees + * selectivecps 10 + * uncurry 11 uncurry, translate function values to anonymous classes + * + * Here `selectiveanf` runs after refchecks and that's the correct ordering. The + * true issue is that `selectiveanf` has hidden dependency on `refchecks` and + * that bites us when we insert xsbt-dependency phase. + * + * By declaring `runsRightAfter` we make the phase ordering algorithm to schedule + * `selectiveanf` to run after `refchecks` again. + */ + val runsRightAfter = Some(API.name) + } + with SubComponent + { + val dependency = new Dependency(global) + def newPhase(prev: Phase) = dependency.newPhase(prev) + def name = phaseName + } + /** This phase walks trees and constructs a representation of the public API, which is used for incremental recompilation. * * We extract the api after picklers, since that way we see the same symbol information/structure @@ -202,6 +256,7 @@ private final class CachedCompiler0(args: Array[String], output: Output, initial override lazy val phaseDescriptors = { phasesSet += sbtAnalyzer + phasesSet += sbtDependency phasesSet += apiExtractor superComputePhaseDescriptors } diff --git a/compile/interface/src/main/scala/xsbt/Dependency.scala b/compile/interface/src/main/scala/xsbt/Dependency.scala new file mode 100644 index 000000000..602eab49a --- /dev/null +++ b/compile/interface/src/main/scala/xsbt/Dependency.scala @@ -0,0 +1,59 @@ +/* sbt -- Simple Build Tool + * Copyright 2008, 2009 Mark Harrah + */ +package xsbt + +import scala.tools.nsc.{io, symtab, Phase} +import io.{AbstractFile, PlainFile, ZipArchive} +import symtab.Flags + +import java.io.File + +object Dependency +{ + def name = "xsbt-dependency" +} +final class Dependency(val global: CallbackGlobal) extends LocateClassFile +{ + import global._ + + def newPhase(prev: Phase): Phase = new DependencyPhase(prev) + private class DependencyPhase(prev: Phase) extends Phase(prev) + { + override def description = "Extracts dependency information" + def name = Dependency.name + def run + { + for(unit <- currentRun.units if !unit.isJava) + { + // build dependencies structure + val sourceFile = unit.source.file.file + for(on <- unit.depends) processDependency(on, inherited=false) + for(on <- inheritedDependencies.getOrElse(sourceFile, Nil: Iterable[Symbol])) processDependency(on, inherited=true) + def processDependency(on: Symbol, inherited: Boolean) + { + def binaryDependency(file: File, className: String) = callback.binaryDependency(file, className, sourceFile, inherited) + val onSource = on.sourceFile + if(onSource == null) + { + classFile(on) match + { + case Some((f,className,inOutDir)) => + if(inOutDir && on.isJavaDefined) registerTopLevelSym(on) + f match + { + case ze: ZipArchive#Entry => for(zip <- ze.underlyingSource; zipFile <- Option(zip.file) ) binaryDependency(zipFile, className) + case pf: PlainFile => binaryDependency(pf.file, className) + case _ => () + } + case None => () + } + } + else + callback.sourceDependency(onSource.file, sourceFile, inherited) + } + } + } + } + +} From e8746dc0c7f3739a21202987f4b36fe88bd58092 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Thu, 24 Oct 2013 12:45:00 +0200 Subject: [PATCH 3/4] Add a bit documentation to Dependency phase. It gives some high-level overview of what this phase does. --- .../src/main/scala/xsbt/Dependency.scala | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compile/interface/src/main/scala/xsbt/Dependency.scala b/compile/interface/src/main/scala/xsbt/Dependency.scala index 602eab49a..8035574e6 100644 --- a/compile/interface/src/main/scala/xsbt/Dependency.scala +++ b/compile/interface/src/main/scala/xsbt/Dependency.scala @@ -13,6 +13,21 @@ object Dependency { def name = "xsbt-dependency" } +/** + * Extracts dependency information from each compilation unit. + * + * This phase uses CompilationUnit.depends and CallbackGlobal.inheritedDependencies + * to collect all symbols that given compilation unit depends on. Those symbols are + * guaranteed to represent Class-like structures. + * + * The CallbackGlobal.inheritedDependencies is populated by the API phase. See, + * ExtractAPI class. + * + * When dependency symbol is processed, it is mapped back to either source file where + * it's defined in (if it's available in current compilation run) or classpath entry + * where it originates from. The Symbol->Classfile mapping is implemented by + * LocateClassFile that we inherit from. + */ final class Dependency(val global: CallbackGlobal) extends LocateClassFile { import global._ From 59de0f00b06b7659091e7dd9a999bbb2a5c9b11f Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Thu, 24 Oct 2013 12:53:52 +0200 Subject: [PATCH 4/4] Remove long comment that explains phase ordering issues. As pointed out by @harrah in #705, we might want to merge both API and dependency phases so we should mention that in the comment explaining phase ordering constraints instead. I'd still like to keep the old comment in the history (as separate commit) because it took me a while to figure out cryptic issues related to continuations plugin so it's valuable to keep the explanation around in case somebody else in the future tries to mess around with dependencies defined by sbt. --- .../main/scala/xsbt/CompilerInterface.scala | 42 ++----------------- 1 file changed, 4 insertions(+), 38 deletions(-) diff --git a/compile/interface/src/main/scala/xsbt/CompilerInterface.scala b/compile/interface/src/main/scala/xsbt/CompilerInterface.scala index ea60f1422..9d1285640 100644 --- a/compile/interface/src/main/scala/xsbt/CompilerInterface.scala +++ b/compile/interface/src/main/scala/xsbt/CompilerInterface.scala @@ -184,43 +184,9 @@ private final class CachedCompiler0(args: Array[String], output: Output, initial val phaseName = Dependency.name val runsAfter = List(API.name) override val runsBefore = List("refchecks") - /* We set runsRightAfter to work-around a bug with phase ordering related to - * continuations plugin. See SI-7217. - * - * If runsRightAfter == None, we get the following set of phases (with continuations - * begin enabled): - * - * typer 4 the meat and potatoes: type the trees - * superaccessors 5 add super accessors in traits and nested classes - * pickler 6 serialize symbol tables - * xsbt-api 7 - * selectiveanf 8 - * xsbt-dependency 9 - * refchecks 10 reference/override checking, translate nested objects - * selectivecps 11 - * liftcode 12 reify trees - * uncurry 13 uncurry, translate function values to anonymous classes - * - * Notice that `selectiveanf` (one of continuations phases) runs before `refchecks` - * and that causes NPEs in `selectiveansf`. - * However, the default ordering for Scala 2.9.2 is: - * - * typer 4 the meat and potatoes: type the trees - * superaccessors 5 add super accessors in traits and nested classes - * pickler 6 serialize symbol tables - * refchecks 7 reference/override checking, translate nested objects - * selectiveanf 8 - * liftcode 9 reify trees - * selectivecps 10 - * uncurry 11 uncurry, translate function values to anonymous classes - * - * Here `selectiveanf` runs after refchecks and that's the correct ordering. The - * true issue is that `selectiveanf` has hidden dependency on `refchecks` and - * that bites us when we insert xsbt-dependency phase. - * - * By declaring `runsRightAfter` we make the phase ordering algorithm to schedule - * `selectiveanf` to run after `refchecks` again. - */ + // keep API and dependency close to each other + // we might want to merge them in the future and even if don't + // do that then it makes sense to run those phases next to each other val runsRightAfter = Some(API.name) } with SubComponent @@ -276,7 +242,7 @@ private final class CachedCompiler0(args: Array[String], output: Output, initial for( (what, warnings) <- seq; (pos, msg) <- warnings) yield callback.problem(what, drep.convert(pos), msg, Severity.Warn, false) } - + def set(callback: AnalysisCallback, dreporter: DelegatingReporter) { this.callback0 = callback