From aac19fd02be94f4ef6ba98187c9cbbc2b66a60f9 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Tue, 19 Nov 2013 21:16:06 +0100 Subject: [PATCH] Extract source code dependencies by tree walking. Previously incremental compiler was extracting source code dependencies by inspecting `CompilationUnit.depends` set. This set is constructed by Scala compiler and it contains all symbols that given compilation unit refers or even saw (in case of implicit search). There are a few problems with this approach: * The contract for `CompilationUnit.depend` is not clearly defined in Scala compiler and there are no tests around it. Read: it's not an official, maintained API. * Improvements to incremental compiler require more context information about given dependency. For example, we want to distinguish between dependency on a class when you just select members from it or inherit from it. The other example is that we might want to know dependencies of a given class instead of the whole compilation unit to make the invalidation logic more precise. That led to the idea of pushing dependency extracting logic to incremental compiler side so it can evolve indepedently from Scala compiler releases and can be refined as needed. We extract dependencies of a compilation unit by walking a type-checked tree and gathering symbols attached to them. Specifically, the tree walk is implemented as a separate phase that runs after pickler and extracts symbols from following tree nodes: * `Import` so we can track dependencies on unused imports * `Select` which is used for selecting all terms * `Ident` used for referring to local terms, package-local terms and top-level packages * `TypeTree` which is used for referring to all types Note that we do not extract just a single symbol assigned to `TypeTree` node because it might represent a complex type that mentions several symbols. We collect all those symbols by traversing the type with CollectTypeTraverser. The implementation of the traverser is inspired by `CollectTypeCollector` from Scala 2.10. The `source-dependencies/typeref-only` test covers a scenario where the dependency is introduced through a TypeRef only. --- .../inc/src/main/scala/sbt/inc/Compile.scala | 2 + .../src/main/scala/xsbt/Dependency.scala | 120 +++++++++++++++++- .../src/main/java/xsbti/AnalysisCallback.java | 12 ++ .../src/test/scala/xsbti/TestCallback.scala | 2 +- .../source-dependencies/typeref-only/A.scala | 5 + .../source-dependencies/typeref-only/B.scala | 1 + .../typeref-only/build.sbt | 5 + .../source-dependencies/typeref-only/test | 7 + 8 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 sbt/src/sbt-test/source-dependencies/typeref-only/A.scala create mode 100644 sbt/src/sbt-test/source-dependencies/typeref-only/B.scala create mode 100644 sbt/src/sbt-test/source-dependencies/typeref-only/build.sbt create mode 100644 sbt/src/sbt-test/source-dependencies/typeref-only/test diff --git a/compile/inc/src/main/scala/sbt/inc/Compile.scala b/compile/inc/src/main/scala/sbt/inc/Compile.scala index 7b36b8b97..76ca59b8b 100644 --- a/compile/inc/src/main/scala/sbt/inc/Compile.scala +++ b/compile/inc/src/main/scala/sbt/inc/Compile.scala @@ -151,6 +151,8 @@ private final class AnalysisCallback(internalMap: File => Option[File], external apis(sourceFile) = (HashAPI(source), savedSource) } + def memberRefAndInheritanceDeps: Boolean = false // TODO: define the flag in IncOptions which controls this + 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) ) diff --git a/compile/interface/src/main/scala/xsbt/Dependency.scala b/compile/interface/src/main/scala/xsbt/Dependency.scala index 8035574e6..907f62419 100644 --- a/compile/interface/src/main/scala/xsbt/Dependency.scala +++ b/compile/interface/src/main/scala/xsbt/Dependency.scala @@ -43,8 +43,23 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile { // 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) + if (global.callback.memberRefAndInheritanceDeps) { + val dependenciesByMemberRef = extractDependenciesByMemberRef(unit) + for(on <- dependenciesByMemberRef) + processDependency(on, inherited=false) + + val dependenciesByInheritance = extractDependenciesByInheritance(unit) + for(on <- dependenciesByInheritance) + processDependency(on, inherited=true) + } else { + for(on <- unit.depends) processDependency(on, inherited=false) + for(on <- inheritedDependencies.getOrElse(sourceFile, Nil: Iterable[Symbol])) processDependency(on, inherited=true) + } + /** + * Handles dependency on given symbol by trying to figure out if represents a term + * that is coming from either source code (not necessarily compiled in this compilation + * run) or from class file and calls respective callback method. + */ def processDependency(on: Symbol, inherited: Boolean) { def binaryDependency(file: File, className: String) = callback.binaryDependency(file, className, sourceFile, inherited) @@ -71,4 +86,105 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile } } + /** + * Traverses given type and collects result of applying a partial function `pf`. + * + * NOTE: This class exists in Scala 2.10 as CollectTypeCollector but does not in earlier + * versions (like 2.9) of Scala compiler that incremental cmpiler supports so we had to + * reimplement that class here. + */ + private final class CollectTypeTraverser[T](pf: PartialFunction[Type, T]) extends TypeTraverser { + var collected: List[T] = Nil + def traverse(tpe: Type): Unit = { + if (pf.isDefinedAt(tpe)) + collected = pf(tpe) :: collected + mapOver(tpe) + } + } + + private abstract class ExtractDependenciesTraverser extends Traverser { + protected val depBuf = collection.mutable.ArrayBuffer.empty[Symbol] + protected def addDependency(dep: Symbol): Unit = depBuf += dep + def dependencies: collection.immutable.Set[Symbol] = { + // convert to immutable set and remove NoSymbol if we have one + depBuf.toSet - NoSymbol + } + } + + private class ExtractDependenciesByMemberRefTraverser extends ExtractDependenciesTraverser { + override def traverse(tree: Tree): Unit = { + tree match { + case Import(expr, selectors) => + selectors.foreach { + case ImportSelector(nme.WILDCARD, _, null, _) => + // in case of wildcard import we do not rely on any particular name being defined + // on `expr`; all symbols that are being used will get caught through selections + case ImportSelector(name: Name, _, _, _) => + def lookupImported(name: Name) = expr.symbol.info.member(name) + // importing a name means importing both a term and a type (if they exist) + addDependency(lookupImported(name.toTermName)) + addDependency(lookupImported(name.toTypeName)) + } + case select: Select => + addDependency(select.symbol) + /* + * Idents are used in number of situations: + * - to refer to local variable + * - to refer to a top-level package (other packages are nested selections) + * - to refer to a term defined in the same package as an enclosing class; + * this looks fishy, see this thread: + * https://groups.google.com/d/topic/scala-internals/Ms9WUAtokLo/discussion + */ + case ident: Ident => + addDependency(ident.symbol) + case typeTree: TypeTree => + val typeSymbolCollector = new CollectTypeTraverser({ + case tpe if !tpe.typeSymbol.isPackage => tpe.typeSymbol + }) + typeSymbolCollector.traverse(typeTree.tpe) + val deps = typeSymbolCollector.collected.toSet + deps.foreach(addDependency) + case Template(parents, self, body) => + traverseTrees(body) + case other => () + } + super.traverse(tree) + } + } + + private def extractDependenciesByMemberRef(unit: CompilationUnit): collection.immutable.Set[Symbol] = { + val traverser = new ExtractDependenciesByMemberRefTraverser + traverser.traverse(unit.body) + val dependencies = traverser.dependencies + // we capture enclosing classes only because that's what CompilationUnit.depends does and we don't want + // to deviate from old behaviour too much for now + dependencies.map(_.toplevelClass) + } + + /** Copied straight from Scala 2.10 as it does not exist in Scala 2.9 compiler */ + private final def debuglog(msg: => String) { + if (settings.debug.value) + log(msg) + } + + private final class ExtractDependenciesByInheritanceTraverser extends ExtractDependenciesTraverser { + override def traverse(tree: Tree): Unit = tree match { + case Template(parents, self, body) => + // we are using typeSymbol and not typeSymbolDirect because we want + // type aliases to be expanded + val parentTypeSymbols = parents.map(parent => parent.tpe.typeSymbol).toSet + debuglog("Parent type symbols for " + tree.pos + ": " + parentTypeSymbols.map(_.fullName)) + parentTypeSymbols.foreach(addDependency) + traverseTrees(body) + case tree => super.traverse(tree) + } + } + + private def extractDependenciesByInheritance(unit: CompilationUnit): collection.immutable.Set[Symbol] = { + val traverser = new ExtractDependenciesByInheritanceTraverser + traverser.traverse(unit.body) + val dependencies = traverser.dependencies + dependencies.map(_.toplevelClass) + } + } diff --git a/interface/src/main/java/xsbti/AnalysisCallback.java b/interface/src/main/java/xsbti/AnalysisCallback.java index 55a90f011..ff239ae74 100644 --- a/interface/src/main/java/xsbti/AnalysisCallback.java +++ b/interface/src/main/java/xsbti/AnalysisCallback.java @@ -27,4 +27,16 @@ public interface AnalysisCallback /** Provides problems discovered during compilation. These may be reported (logged) or unreported. * Unreported problems are usually unreported because reporting was not enabled via a command line switch. */ public void problem(String what, Position pos, String msg, Severity severity, boolean reported); + /** + * Determines whether member reference and inheritance dependencies should be extracted in given compiler + * run. + * + * As the signature suggests, this method's implementation is meant to be side-effect free. It's added + * to AnalysisCallback because it indicates how other callback calls should be interpreted by both + * implementation of AnalysisCallback and it's clients. + * + * NOTE: This method is an implementation detail and can be removed at any point without deprecation. + * Do not depend on it, please. + */ + public boolean memberRefAndInheritanceDeps(); } \ No newline at end of file diff --git a/interface/src/test/scala/xsbti/TestCallback.scala b/interface/src/test/scala/xsbti/TestCallback.scala index f8454336a..28bee5466 100644 --- a/interface/src/test/scala/xsbti/TestCallback.scala +++ b/interface/src/test/scala/xsbti/TestCallback.scala @@ -4,7 +4,7 @@ import java.io.File import scala.collection.mutable.ArrayBuffer import xsbti.api.SourceAPI -class TestCallback extends AnalysisCallback +class TestCallback(override val memberRefAndInheritanceDeps: Boolean = false) extends AnalysisCallback { val sourceDependencies = new ArrayBuffer[(File, File, Boolean)] val binaryDependencies = new ArrayBuffer[(File, String, File, Boolean)] diff --git a/sbt/src/sbt-test/source-dependencies/typeref-only/A.scala b/sbt/src/sbt-test/source-dependencies/typeref-only/A.scala new file mode 100644 index 000000000..3b274e4a8 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/typeref-only/A.scala @@ -0,0 +1,5 @@ +class A[T] + +abstract class C { + def foo: A[B] +} diff --git a/sbt/src/sbt-test/source-dependencies/typeref-only/B.scala b/sbt/src/sbt-test/source-dependencies/typeref-only/B.scala new file mode 100644 index 000000000..179f0d275 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/typeref-only/B.scala @@ -0,0 +1 @@ +class B diff --git a/sbt/src/sbt-test/source-dependencies/typeref-only/build.sbt b/sbt/src/sbt-test/source-dependencies/typeref-only/build.sbt new file mode 100644 index 000000000..02813797f --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/typeref-only/build.sbt @@ -0,0 +1,5 @@ +logLevel := Level.Debug + +// disable recompile all which causes full recompile which +// makes it more difficult to test dependency tracking +incOptions ~= { _.copy(recompileAllFraction = 1.0) } diff --git a/sbt/src/sbt-test/source-dependencies/typeref-only/test b/sbt/src/sbt-test/source-dependencies/typeref-only/test new file mode 100644 index 000000000..fb314fd7b --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/typeref-only/test @@ -0,0 +1,7 @@ +# test case for dependency tracking in case given type (`B` in our case) +# mentioned only in type ref (as a type argument) +> compile + +$ delete B.scala + +-> compile