From 5c36c57d68ac5bab2e1f046cb91346188ddc6ddb Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Mon, 9 Oct 2017 14:59:27 -0400 Subject: [PATCH] Fixes undercompilation on inheritance on same source In sbt 0.13 days, we could ignore the relationship between two classes defined in the same `*.scala` source file, because they will be compiled anyway, and the invalidation was done at the source file level. With class-based namehashing, the invalidation is done at the class level, so we can no longer ignore inheritance relationship coming from the same source, but we still have old assumptions scattered around the xsbt-dependency implementation. This change changes two if expressions that was used to filter out dependency info coming from the same source. One might wonder why it's necessary to keep the local inheritance info, if two classes involved are compiled together anyways. The answer is transitive dependencies. Using trait-trait-211 as example, `gg.table.ObjectConcatHelpers` was changed, eventually causing `xx.EvaluatorTestSupport` to invalidate. However, because of the missing same-source inheritance, it did not invalidate `xx.EvaluatorSpecification`, and resulting to a runtime error. Fixes sbt/zinc#417 --- .../src/main/scala/xsbt/Dependency.scala | 21 ++++++---- .../scala/sbt/internal/inc/Relations.scala | 40 +++++++++---------- .../source-dependencies/patMat-scope/test | 4 +- .../sbt-test/source-dependencies/sealed/test | 4 +- .../trait-trait-211/build.json | 8 ++++ .../changes/SliceTransforms1.scala | 12 ++++++ .../mirtest/EvaluatorSpecs.scala | 9 +++++ .../trait-trait-211/mirtest/Hello.scala | 5 +++ .../mirtest/SliceTransforms.scala | 10 +++++ .../mirtest/incOptions.properties | 1 + .../source-dependencies/trait-trait-211/test | 4 ++ 11 files changed, 84 insertions(+), 34 deletions(-) create mode 100644 zinc/src/sbt-test/source-dependencies/trait-trait-211/build.json create mode 100644 zinc/src/sbt-test/source-dependencies/trait-trait-211/changes/SliceTransforms1.scala create mode 100644 zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/EvaluatorSpecs.scala create mode 100644 zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/Hello.scala create mode 100644 zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/SliceTransforms.scala create mode 100644 zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/incOptions.properties create mode 100644 zinc/src/sbt-test/source-dependencies/trait-trait-211/test diff --git a/internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala b/internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala index 20ce91c9d6..1bad94f944 100644 --- a/internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala +++ b/internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala @@ -92,16 +92,21 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with } // Define processor reusing `processDependency` definition - val memberRef = processDependency(DependencyByMemberRef) _ - val inheritance = processDependency(DependencyByInheritance) _ - val localInheritance = processDependency(LocalDependencyByInheritance) _ + val memberRef = processDependency(DependencyByMemberRef, false) _ + val inheritance = processDependency(DependencyByInheritance, true) _ + val localInheritance = processDependency(LocalDependencyByInheritance, true) _ + + @deprecated("Use processDependency that takes allowLocal.", "1.1.0") + def processDependency(context: DependencyContext)(dep: ClassDependency): Unit = + processDependency(context, true)(dep) /* * 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(context: DependencyContext)(dep: ClassDependency): Unit = { + def processDependency(context: DependencyContext, allowLocal: Boolean)( + dep: ClassDependency): Unit = { val fromClassName = classNameAsString(dep.from) def binaryDependency(file: File, binaryClassName: String) = @@ -133,11 +138,12 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with case None => debuglog(Feedback.noOriginFileForExternalSymbol(dep.to)) } - } else if (onSource.file != sourceFile) { - // Dependency is internal -- but from other file / compilation unit + } else if (onSource.file != sourceFile || allowLocal) { + // We cannot ignore dependencies coming from the same source file because + // the dependency info needs to propagate. See source-dependencies/trait-trait-211. val onClassName = classNameAsString(dep.to) callback.classDependency(onClassName, fromClassName, context) - } else () // Comes from the same file, ignore + } } } @@ -227,7 +233,6 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with val depClass = enclOrModuleClass(dep) val dependency = ClassDependency(fromClass, depClass) if (!cache.contains(dependency) && - fromClass.associatedFile != depClass.associatedFile && !depClass.isRefinementClass) { process(dependency) cache.add(dependency) diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala index 16318212f6..6f78eb1524 100644 --- a/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala @@ -275,6 +275,7 @@ object Relations { case o: ClassDependencies => internal == o.internal && external == o.external case _ => false } + override def toString: String = s"ClassDependencies(internal = $internal, external = $external)" override def hashCode = (internal, external).hashCode } @@ -661,26 +662,23 @@ private class MRelationsNameHashing( override def hashCode = (srcProd :: libraryDep :: libraryClassName :: memberRef :: inheritance :: classes :: Nil).hashCode - override def toString = ( - """ + override def toString: String = { + val internalDepsStr = (internalDependencies.dependencies map { + case (k, vs) => k + " " + relation_s(vs) + }).mkString("\n ", "\n ", "") + val externalDepsStr = (externalDependencies.dependencies map { + case (k, vs) => k + " " + relation_s(vs) + }).mkString("\n ", "\n ", "") + s""" |Relations (with name hashing enabled): - | products: %s - | library deps: %s - | library class names: %s - | class deps: %s - | ext deps: %s - | class names: %s - | used names: %s - | product class names: %s - """.trim.stripMargin.format( - List(srcProd, - libraryDep, - libraryClassName, - internalClassDep, - externalClassDep, - classes, - names, - productClassName) map relation_s: _*) - ) - + | products: ${relation_s(srcProd)} + | library deps: ${relation_s(libraryDep)} + | library class names: ${relation_s(libraryClassName)} + | internalDependencies: $internalDepsStr + | externalDependencies: $externalDepsStr + | class names: ${relation_s(classes)} + | used names: ${relation_s(names)} + | product class names: ${relation_s(productClassName)} + """.trim.stripMargin + } } diff --git a/zinc/src/sbt-test/source-dependencies/patMat-scope/test b/zinc/src/sbt-test/source-dependencies/patMat-scope/test index 48a933573b..60aa342999 100644 --- a/zinc/src/sbt-test/source-dependencies/patMat-scope/test +++ b/zinc/src/sbt-test/source-dependencies/patMat-scope/test @@ -12,9 +12,7 @@ $ sleep 1000 $ exists target/classes/foo/SealedUsedInPatMatScope.class $ exists target/classes/foo/SealedNameUsedInDefaultScope.class -# Default scopes should not change -$ newer beforeFirstCompilation target/classes/foo/SealedNameUsedInDefaultScope.class -# But PatMat and direct usage of child should be recompiled +# PatMat and direct usage of child should be recompiled $ newer target/classes/foo/SealedUsedInPatMatScope.class beforeFirstCompilation # Change type of Sealed diff --git a/zinc/src/sbt-test/source-dependencies/sealed/test b/zinc/src/sbt-test/source-dependencies/sealed/test index 412e495308..18b9631cd4 100644 --- a/zinc/src/sbt-test/source-dependencies/sealed/test +++ b/zinc/src/sbt-test/source-dependencies/sealed/test @@ -5,5 +5,5 @@ $ copy-file changes/A.scala A.scala # D.scala needs recompiling because the pattern match in D # is no longer exhaustive, which emits a warning -> checkRecompilations 1 A B C E -> checkRecompilations 2 D +> checkRecompilations 1 +> checkRecompilations 2 A B C D E diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/build.json b/zinc/src/sbt-test/source-dependencies/trait-trait-211/build.json new file mode 100644 index 0000000000..e2b0035d3b --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/trait-trait-211/build.json @@ -0,0 +1,8 @@ +{ + "projects": [ + { + "name": "mirtest", + "scalaVersion": "2.11.8" + } + ] +} diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/changes/SliceTransforms1.scala b/zinc/src/sbt-test/source-dependencies/trait-trait-211/changes/SliceTransforms1.scala new file mode 100644 index 0000000000..6b2cc5bb62 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/trait-trait-211/changes/SliceTransforms1.scala @@ -0,0 +1,12 @@ +package gg +package table + +trait SliceTransforms extends ObjectConcatHelpers { + // the use site is updated + buildNonemptyObjects(0, 1) +} + +trait ObjectConcatHelpers { + // add extra parameter here + def buildNonemptyObjects(a: Int, b: Int): Unit = () +} diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/EvaluatorSpecs.scala b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/EvaluatorSpecs.scala new file mode 100644 index 0000000000..32e7b8cf8d --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/EvaluatorSpecs.scala @@ -0,0 +1,9 @@ +package xx + +import gg.table._ + +trait EvaluatorSpecification extends EvaluatorTestSupport { +} + +trait EvaluatorTestSupport extends SliceTransforms { +} diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/Hello.scala b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/Hello.scala new file mode 100644 index 0000000000..71c5aa732b --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/Hello.scala @@ -0,0 +1,5 @@ +package xx + +object Hello extends App { + val consumer = new EvaluatorSpecification {} +} diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/SliceTransforms.scala b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/SliceTransforms.scala new file mode 100644 index 0000000000..1ec3d1fb80 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/SliceTransforms.scala @@ -0,0 +1,10 @@ +package gg +package table + +trait SliceTransforms extends ObjectConcatHelpers { + buildNonemptyObjects(0) +} + +trait ObjectConcatHelpers { + def buildNonemptyObjects(a: Int): Unit = () +} diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/incOptions.properties b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/incOptions.properties new file mode 100644 index 0000000000..adfc92c361 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/incOptions.properties @@ -0,0 +1 @@ +relationsDebug = true diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/test b/zinc/src/sbt-test/source-dependencies/trait-trait-211/test new file mode 100644 index 0000000000..1dc73abf15 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/trait-trait-211/test @@ -0,0 +1,4 @@ +> mirtest/run + +$ copy-file changes/SliceTransforms1.scala mirtest/SliceTransforms.scala +> mirtest/run