Inherited trait definitions do not seem to be handled gracefully by Zinc #417
Assignees
Labels
Milestone
Comments
jvican
changed the title
|
I was able to reproduce this on the first trial. problemand a bunch others caused by notesHere's https://github.com/quasar-analytics/quasar/commit/66c97fd9ed3e36eec1a058e5b9a546c80ab6309b. In trait ObjectConcatHelpers extends ConcatHelpers {
....
- def buildNonemptyObjects(leftFields: Map[ColumnRef, Column], rightFields: Map[ColumnRef, Column]) = {
+ def buildNonemptyObjects(
+ leftFields: Map[ColumnRef, Column],
+ rightFields: Map[ColumnRef, Column],
+ rightSize: Int): Map[ColumnRef, Column] = {
....https://gist.github.com/eed3si9n/3a1e51a3b1c6f980814e486bfbc88ae3#file-yggdrasil-compile-compileincremental-log is the compile output from yggdrasil's Compile configuration compilation. so Now, if you look at the mimir's So Zinc knew |
|
I wonder if this is another instance of #292... The cause seems similar enough despite the error message being different. |
|
@eed3si9n Worth noting that we're on 2.11.8, not 2.11.11, due to issues with explosive state space in patmat. Traits still require codegen, so your point still stands. |
|
The laziness introduced by |
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 1, 2017
|
Not quite minimal but now I have a scripted test that causes Note that I needed two subprojects to reproduce this. |
|
Added some debug logs, and got some interesting results: The "previous" is missing inheritance relationship between |
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 7, 2017
### 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. This fact is acknowledged in the existence of `LocalDependencyByInheritance`, but we still have old assumptions scattered around the xsbt-dependency implementation. ### what this changes This change removes two if expressions that was used to filter out dependency info coming from the same source. ### notes 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`. This meant that neither `xx.StringLibSpecs` was invalidated, and resulting to a runtime error. Fixes sbt#417
|
Please review #424. We'll ship sbt 1.0.x hotfix once the fix is in. |
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 8, 2017
### 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. This fact is acknowledged in the existence of `LocalDependencyByInheritance`, but we still have old assumptions scattered around the xsbt-dependency implementation. ### what this changes This change changes two if expressions that was used to filter out dependency info coming from the same source. ### notes 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`. This meant that neither `xx.StringLibSpecs` was invalidated, and resulting to a runtime error. Fixes sbt#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 8, 2017
### 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. This fact is acknowledged in the existence of `LocalDependencyByInheritance`, but we still have old assumptions scattered around the xsbt-dependency implementation. ### what this changes This change changes two if expressions that was used to filter out dependency info coming from the same source. ### notes 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`. This meant that neither `xx.StringLibSpecs` was invalidated, and resulting to a runtime error. Fixes sbt#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 8, 2017
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. This fact is acknowledged in the existence of `LocalDependencyByInheritance`, 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`. This meant that neither `xx.StringLibSpecs` was invalidated, and resulting to a runtime error. Fixes sbt#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 9, 2017
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`. This meant that neither `xx.StringLibSpecs` was invalidated, and resulting to a runtime error. Fixes sbt#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 9, 2017
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#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 10, 2017
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.SliceTransforms` was changed, causing `xx.EvaluatorTestSupport` to invalidate. However, because of the missing same-source inheritance, it did not invalidate `xx.EvaluatorSpecification`. Calling `transform` method on a new `xx.EvaluatorSpecification` causes runtime error. Fixes sbt#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 10, 2017
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.SliceTransforms` was changed, causing `xx.EvaluatorTestSupport` to invalidate. However, because of the missing same-source inheritance, it did not invalidate `xx.EvaluatorSpecification`, and neither does `xx.StringLibSpecs`. Calling `transform` method on a new `xx.StringLibSpecs ` causes runtime error. Fixes sbt#417
jvican
pushed a commit
to eed3si9n/zinc
that referenced
this issue
Oct 10, 2017
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.SliceTransforms` was changed, causing `xx.EvaluatorTestSupport` to invalidate. However, because of the missing same-source inheritance, it did not invalidate `xx.EvaluatorSpecification`, and neither does `xx.StringLibSpecs`. Calling `transform` method on a new `xx.StringLibSpecs ` causes runtime error. Fixes sbt#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 11, 2017
### 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.EvaluatorTestSupport -> gg.table.SliceTransforms [debug] [inv] xx.StringLibSpecs -> xx.EvaluatorSpecification [debug] [inv] ] [debug] [inv] DependencyByMemberRef Relation [ [debug] [inv] xx.EvaluatorTestSupport -> gg.table.SliceTransforms [debug] [inv] xx.Hello -> gg.table.SliceTransforms [debug] [inv] xx.StringLibSpecs -> xx.EvaluatorSpecification [debug] [inv] ] .... Caused by: java.lang.AbstractMethodError: xx.StringLibSpecs.buildNonemptyObjects(II)V ``` First, we see that `xx.EvaluatorSpecification -> xx.EvaluatorTestSupport DependencyByInheritance` relationship is missing. Second, the error message seen is `java.lang.AbstractMethodError` happening on `xx.StringLibSpecs`. ### 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.SliceTransforms` was changed, 2. causing `xx.EvaluatorTestSupport` to invalidate. 3. However, because of the missing same-source inheritance, it did not invalidate `xx.EvaluatorSpecification`. 4. This meant that neither `xx.StringLibSpecs` was invalidated. 5. Calling transform method on a new `xx.StringLibSpecs` causes runtime error. By tracking same-source inheritance, we will now correctly invalidate `xx.EvaluatorSpecification` and `xx.StringLibSpecs`. 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 2.12 because 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 sbt#417
jvican
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 11, 2017
jvican
pushed a commit
to eed3si9n/zinc
that referenced
this issue
Oct 11, 2017
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. ``` [info] Compiling 1 Scala source to ... .... [debug] [inv] internalDependencies: [debug] [inv] DependencyByInheritance Relation [ [debug] [inv] xx.EvaluatorTestSupport -> gg.table.SliceTransforms [debug] [inv] xx.StringLibSpecs -> xx.EvaluatorSpecification [debug] [inv] ] [debug] [inv] DependencyByMemberRef Relation [ [debug] [inv] xx.EvaluatorTestSupport -> gg.table.SliceTransforms [debug] [inv] xx.Hello -> gg.table.SliceTransforms [debug] [inv] xx.StringLibSpecs -> xx.EvaluatorSpecification [debug] [inv] ] .... Caused by: java.lang.AbstractMethodError: xx.StringLibSpecs.buildNonemptyObjects(II)V ``` First, we see that `xx.EvaluatorSpecification -> xx.EvaluatorTestSupport DependencyByInheritance` relationship is missing. Second, the error message seen is `java.lang.AbstractMethodError` happening on `xx.StringLibSpecs`. 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.SliceTransforms` was changed, 2. causing `xx.EvaluatorTestSupport` to invalidate. 3. However, because of the missing same-source inheritance, it did not invalidate `xx.EvaluatorSpecification`. 4. This meant that neither `xx.StringLibSpecs` was invalidated. 5. Calling transform method on a new `xx.StringLibSpecs` causes runtime error. By tracking same-source inheritance, we will now correctly invalidate `xx.EvaluatorSpecification` and `xx.StringLibSpecs`. 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." No. The simple trait-trait inheritance reproduction alone will not cause problem in 2.12 because 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 sbt#417
eed3si9n
pushed a commit
to eed3si9n/zinc
that referenced
this issue
Oct 12, 2017
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 12, 2017
### 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.EvaluatorTestSupport -> gg.table.SliceTransforms [debug] [inv] xx.StringLibSpecs -> xx.EvaluatorSpecification [debug] [inv] ] [debug] [inv] DependencyByMemberRef Relation [ [debug] [inv] xx.EvaluatorTestSupport -> gg.table.SliceTransforms [debug] [inv] xx.Hello -> gg.table.SliceTransforms [debug] [inv] xx.StringLibSpecs -> xx.EvaluatorSpecification [debug] [inv] ] .... Caused by: java.lang.AbstractMethodError: xx.StringLibSpecs.buildNonemptyObjects(II)V ``` First, we see that `xx.EvaluatorSpecification -> xx.EvaluatorTestSupport DependencyByInheritance` relationship is missing. Second, the error message seen is `java.lang.AbstractMethodError` happening on `xx.StringLibSpecs`. ### 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.SliceTransforms` was changed, 2. causing `xx.EvaluatorTestSupport` to invalidate. 3. However, because of the missing same-source inheritance, it did not invalidate `xx.EvaluatorSpecification`. 4. This meant that neither `xx.StringLibSpecs` was invalidated. 5. Calling transform method on a new `xx.StringLibSpecs` causes runtime error. By tracking same-source inheritance, we will now correctly invalidate `xx.EvaluatorSpecification` and `xx.StringLibSpecs`. 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 sbt#417
eed3si9n
added a commit
to eed3si9n/zinc
that referenced
this issue
Oct 12, 2017
### 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 sbt#417
|
Fixed in #424. |
dwijnand
added a commit
to dwijnand/zinc
that referenced
this issue
Oct 24, 2017
* 1.0.x: Fix sbt#127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt#417 Add trait-trait-212 for Scala 2.12.3 Fix source-dependencies/sealed Import statement no longer needed Move mima exclusions to its own file Move REPL related xsbti Java to correct module make LoggedReporter fields lazy sbt.version=1.0.2 use neo-sbt-scalafmt Fix ConsoleInterface binding things properly^2
dwijnand
added a commit
to dwijnand/zinc
that referenced
this issue
Nov 22, 2017
* 1.0.x: (25 commits) Add yourkit acknoledgement in the README Add header to cached hashing spec Add headers to missing files Fix sbt#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix sbt#442: Name hash of value class should include underlying type source-dependencies/value-class-underlying: fix test Ignore null in generic lambda tparams Improve and make scripted parallel Fix sbt#436: Remove annoying log4j scripted exception Fix sbt#127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt#417 Add trait-trait-212 for Scala 2.12.3 Fix source-dependencies/sealed Import statement no longer needed Move mima exclusions to its own file ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala The ClassToAPI conflict is due to: * sbt#393 (a 1.x PR), conflicting with * sbt#446 (a 1.0.x PR). The MixedAnalyzingCompiler conflict is due to: * sbt#427 (a 1.x PR), conflicting with * sbt#452 (a 1.0.x PR).
dwijnand
added a commit
to dwijnand/zinc
that referenced
this issue
Nov 23, 2017
* 1.0.x: (28 commits) Split compiler bridge tests to another subproject Implement compiler bridge for 2.13.0-M2 Add yourkit acknoledgement in the README "sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0 Add header to cached hashing spec Add headers to missing files Fix sbt#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix sbt#442: Name hash of value class should include underlying type source-dependencies/value-class-underlying: fix test Ignore null in generic lambda tparams Improve and make scripted parallel Fix sbt#436: Remove annoying log4j scripted exception Fix sbt#127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt#417 Add trait-trait-212 for Scala 2.12.3 ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala project/build.properties zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala The ClassToAPI conflict is due to: * sbt#393 (a 1.x PR), conflicting with * sbt#446 (a 1.0.x PR). The build.properties conflict is due to different PRs bumping sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (sbt#413, sbt#418, sbt#453). The MixedAnalyzingCompiler conflict is due to: * sbt#427 (a 1.x PR), conflicting with * sbt#452 (a 1.0.x PR).
eed3si9n
pushed a commit
to scala/compiler-interface
that referenced
this issue
Apr 23, 2019
* 1.0.x: (28 commits) Split compiler bridge tests to another subproject Implement compiler bridge for 2.13.0-M2 Add yourkit acknoledgement in the README "sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0 Add header to cached hashing spec Add headers to missing files Fix #332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix #442: Name hash of value class should include underlying type source-dependencies/value-class-underlying: fix test Ignore null in generic lambda tparams Improve and make scripted parallel Fix #436: Remove annoying log4j scripted exception Fix #127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt/zinc#417 Add trait-trait-212 for Scala 2.12.3 ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala project/build.properties zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala The ClassToAPI conflict is due to: * sbt/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#446 (a 1.0.x PR). The build.properties conflict is due to different PRs bumping sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (#413, #418, #453). The MixedAnalyzingCompiler conflict is due to: * sbt/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x PR).
dwijnand
added a commit
to dwijnand/sbt
that referenced
this issue
Apr 25, 2019
* 1.0.x: (28 commits) Split compiler bridge tests to another subproject Implement compiler bridge for 2.13.0-M2 Add yourkit acknoledgement in the README "sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0 Add header to cached hashing spec Add headers to missing files Fix sbt#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix sbt#442: Name hash of value class should include underlying type source-dependencies/value-class-underlying: fix test Ignore null in generic lambda tparams Improve and make scripted parallel Fix sbt#436: Remove annoying log4j scripted exception Fix sbt#127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt/zinc#417 Add trait-trait-212 for Scala 2.12.3 ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala project/build.properties zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala The ClassToAPI conflict is due to: * sbt/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#446 (a 1.0.x PR). The build.properties conflict is due to different PRs bumping sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (sbt#413, sbt#418, sbt#453). The MixedAnalyzingCompiler conflict is due to: * sbt/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x PR).
eed3si9n
added a commit
to eed3si9n/scala
that referenced
this issue
May 14, 2019
### 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 sbt/zinc#417
eed3si9n
pushed a commit
to eed3si9n/scala
that referenced
this issue
May 14, 2019
* 1.0.x: (28 commits) Split compiler bridge tests to another subproject Implement compiler bridge for 2.13.0-M2 Add yourkit acknoledgement in the README "sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0 Add header to cached hashing spec Add headers to missing files Fix scala#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix scala#442: Name hash of value class should include underlying type source-dependencies/value-class-underlying: fix test Ignore null in generic lambda tparams Improve and make scripted parallel Fix scala#436: Remove annoying log4j scripted exception Fix scala#127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt/zinc#417 Add trait-trait-212 for Scala 2.12.3 ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala project/build.properties zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala The ClassToAPI conflict is due to: * sbt/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#446 (a 1.0.x PR). The build.properties conflict is due to different PRs bumping sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (scala#413, scala#418, scala#453). The MixedAnalyzingCompiler conflict is due to: * sbt/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x PR).
eed3si9n
added
the
area/under_compilation
lrytz
pushed a commit
to lrytz/scala
that referenced
this issue
Nov 5, 2019
### 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 sbt/zinc#417 Rewritten from sbt/zinc@05482d1
lrytz
pushed a commit
to lrytz/scala
that referenced
this issue
Nov 5, 2019
* 1.0.x: (28 commits) Split compiler bridge tests to another subproject Implement compiler bridge for 2.13.0-M2 Add yourkit acknoledgement in the README "sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0 Add header to cached hashing spec Add headers to missing files Fix scala#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix scala#442: Name hash of value class should include underlying type source-dependencies/value-class-underlying: fix test Ignore null in generic lambda tparams Improve and make scripted parallel Fix scala#436: Remove annoying log4j scripted exception Fix scala#127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt/zinc#417 Add trait-trait-212 for Scala 2.12.3 ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala project/build.properties zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala The ClassToAPI conflict is due to: * sbt/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#446 (a 1.0.x PR). The build.properties conflict is due to different PRs bumping sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (scala#413, scala#418, scala#453). The MixedAnalyzingCompiler conflict is due to: * sbt/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x PR). Rewritten from sbt/zinc@e245d95
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See sbt/sbt#3586 for the original report.
Ok, I don't have a nice and minimal test case for you guys, unfortunately, but I do have a reproducer. It seems like this issue is broadly related to the cake pattern, which is used heavily in some older parts of the quasar codebase (sadface…). Changing upstream functions (in inherited traits) will cause some downstream compilation units to explode at runtime with
AbstractMethodErrordue to Zinc improperly believing the files in question did not need to be invalidated.This happens intermittently in local development (my steps below point to a changeset which will reproduce the issue) and 100% deterministically in CI. The reason it is happening deterministically in CI is because we cache our target directories for PR builds in Travis (as discussed with @jvican) in order to bring down our horrendously-long compilation times. This worked almost flawlessly with SBT 0.13.x for the last eight months (we only needed to cache flush twice), which is quite impressive! However, on SBT 1.0, we can't even get a single PR build to work, as the slower matrix builds will get the new caches from the faster builds, and themselves run into mutual-invalidation issues. This is kind of an edge case, but it does demonstrate how much less reliable current Zinc is from its state in 0.13.
steps
a6b7a91010faf8d06ef678653db53bafdaceaa5esbt test:compile66c97fd9ed3e36eec1a058e5b9a546c80ab6309bsbt mimir/testnotes
sbt version: 1.0.2
The text was updated successfully, but these errors were encountered: