-
Notifications
You must be signed in to change notification settings - Fork 121
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixes undercompilation on inheritance on same source
### background 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. ### what we see without the fix ``` [info] Compiling 1 Scala source to ... .... [debug] [inv] internalDependencies: [debug] [inv] DependencyByInheritance Relation [ [debug] [inv] xx.B -> gg.table.A [debug] [inv] xx.Foo -> xx.C [debug] [inv] ] [debug] [inv] DependencyByMemberRef Relation [ [debug] [inv] xx.B -> gg.table.A [debug] [inv] xx.Hello -> gg.table.A [debug] [inv] xx.Foo -> xx.C [debug] [inv] ] .... Caused by: java.lang.AbstractMethodError: xx.Foo.buildNonemptyObjects(II)V ``` First, we see that `xx.C -> xx.B DependencyByInheritance` relationship is missing. Second, the error message seen is `java.lang.AbstractMethodError` happening on `xx.Foo`. ### what this changes 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. Here's likely what was happening: 1. `gg.table.A` was changed, 2. causing `xx.B` to invalidate. 3. However, because of the missing same-source inheritance, it did not invalidate `xx.C`. 4. This meant that neither `xx.Foo` was invalidated. 5. Calling transform method on a new `xx.Foo` causes runtime error. By tracking same-source inheritance, we will now correctly invalidate `xx.C` and `xx.Foo`. I think the assumption that's broken here is that "we don't need to track inheritance that is happening between two classes in a same source." ### Is this 2.11 only issue? No. The simple trait-trait inheritance reproduction alone will not cause problem in Scala 2.12 because of the [compile-to-interface](http://www.scala-lang.org/news/2.12.0/#traits-compile-to-interfaces) traits. However, not all traits will compile to interface. This means that if we want to take advantage of the compile-to-interface traits, we still should keep track of the same-source inheritance, but introduce some more logic to determine whether recompilation is necessary. Fixes #417
- Loading branch information
Showing
16 changed files
with
99 additions
and
43 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 8 additions & 0 deletions
8
zinc/src/sbt-test/source-dependencies/trait-trait-211/build.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"projects": [ | ||
{ | ||
"name": "mirtest", | ||
"scalaVersion": "2.11.8" | ||
} | ||
] | ||
} |
2 changes: 1 addition & 1 deletion
2
...-trait-212/changes/SliceTransforms1.scala → ...ndencies/trait-trait-211/changes/A1.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
...t-trait-212/mirtest/SliceTransforms.scala → ...endencies/trait-trait-211/mirtest/A.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
package gg | ||
package table | ||
|
||
trait SliceTransforms { | ||
trait A { | ||
def transform: Unit = { | ||
buildNonemptyObjects(0) | ||
} | ||
|
9 changes: 9 additions & 0 deletions
9
zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/B.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package xx | ||
|
||
import gg.table._ | ||
|
||
trait C extends B { | ||
} | ||
|
||
trait B extends A { | ||
} |
8 changes: 8 additions & 0 deletions
8
zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/Hello.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package xx | ||
|
||
object Hello extends App { | ||
val consumer = new Foo | ||
consumer.transform | ||
} | ||
|
||
class Foo extends C |
1 change: 1 addition & 0 deletions
1
zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/incOptions.properties
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
relationsDebug = true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
> mirtest/run | ||
|
||
## After copying the Good implementation, we should be able to run successfully. | ||
$ copy-file changes/A1.scala mirtest/A.scala | ||
> mirtest/run |
12 changes: 12 additions & 0 deletions
12
zinc/src/sbt-test/source-dependencies/trait-trait-212/changes/A1.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package gg | ||
package table | ||
|
||
trait A { | ||
def transform: Unit = { | ||
// the use site is updated | ||
buildNonemptyObjects(0, 1) | ||
} | ||
|
||
// add extra parameter here | ||
def buildNonemptyObjects(a: Int, b: Int): Unit = () | ||
} |
10 changes: 10 additions & 0 deletions
10
zinc/src/sbt-test/source-dependencies/trait-trait-212/mirtest/A.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package gg | ||
package table | ||
|
||
trait A { | ||
def transform: Unit = { | ||
buildNonemptyObjects(0) | ||
} | ||
|
||
def buildNonemptyObjects(a: Int): Unit = () | ||
} |
9 changes: 9 additions & 0 deletions
9
zinc/src/sbt-test/source-dependencies/trait-trait-212/mirtest/B.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package xx | ||
|
||
import gg.table._ | ||
|
||
trait C extends B { | ||
} | ||
|
||
trait B extends A { | ||
} |
9 changes: 0 additions & 9 deletions
9
zinc/src/sbt-test/source-dependencies/trait-trait-212/mirtest/EvaluatorSpecs.scala
This file was deleted.
Oops, something went wrong.
4 changes: 2 additions & 2 deletions
4
zinc/src/sbt-test/source-dependencies/trait-trait-212/mirtest/Hello.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
package xx | ||
|
||
object Hello extends App { | ||
val consumer = new StringLibSpecs | ||
val consumer = new Foo | ||
consumer.transform | ||
} | ||
|
||
class StringLibSpecs extends EvaluatorSpecification | ||
class Foo extends C |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
> mirtest/run | ||
|
||
## After copying the Good implementation, we should be able to run successfully. | ||
$ copy-file changes/SliceTransforms1.scala mirtest/SliceTransforms.scala | ||
$ copy-file changes/A1.scala mirtest/A.scala | ||
> mirtest/run |
File renamed without changes.