Private def with existential type causes spurious recompilation due to name instability #823
Comments
ghost
assigned 
|
Probably due to 864580a. The correct approach was removed because it relied on Symbol.id, which is apparently an unreliable implementation detail. The id didn't need to be stable across runs, but was just used to identify equal type parameters. I don't remember the details, but should it be desirable to restore the indexed approach, perhaps a Symbol map would work instead. |
|
Thanks for the pointer! Yes, using Symbol.id would solve that particular problem but would introduce a lot more other problems. I was thinking about introducing some kind of a map on Sbt side. I'm not sure if I was thinking about introducing a much more specialized map |
|
Not that everything isn't a side effecting operation, but in general owners know nothing of their children. It doesn't mutate the owner unless you mutate the owner (perhaps entering it into the owner's scope.) |
gkossakowski
added a commit
to gkossakowski/sbt
that referenced
this issue
Aug 2, 2013
Fix the problem with unstable names synthesized for existential
types (declared with underscore syntax) by renaming type variables
to a scheme that is guaranteed to be stable no matter where given
the existential type appears.
We can do that because type variables declared in existential types
are scoped to those types so the renaming operation is rather local.
There's a specs2 unit test covering instability of existential types.
The test is included in compiler-interface project and the build
definition has been modified to enable building and executing tests
in compiler-interface project. Some dependencies has been modified:
* compiler-interface project depends on api project for testing
(test makes us of SameAPI)
* dependency on junit has been introduced because it's needed
for `@RunWith` annotation which declares that specs2 unit
test should be ran with JUnitRunner
SameAPI has been modified to expose a method that allows us to
compare two definitions.
Also, a integration test has been added which tests scenario
mentioned in sbt#823.
This commit fixes sbt#823.
gkossakowski
added a commit
to gkossakowski/sbt
that referenced
this issue
Aug 22, 2013
Fix the problem with unstable names synthesized for existential
types (declared with underscore syntax) by renaming type variables
to a scheme that is guaranteed to be stable no matter where given
the existential type appears.
We can do that because type variables declared in existential types
are scoped to those types so the renaming operation is rather local.
There's a specs2 unit test covering instability of existential types.
The test is included in compiler-interface project and the build
definition has been modified to enable building and executing tests
in compiler-interface project. Some dependencies has been modified:
* compiler-interface project depends on api project for testing
(test makes us of SameAPI)
* dependency on junit has been introduced because it's needed
for `@RunWith` annotation which declares that specs2 unit
test should be ran with JUnitRunner
SameAPI has been modified to expose a method that allows us to
compare two definitions.
Also, a integration test has been added which tests scenario
mentioned in sbt#823.
This commit fixes sbt#823.
|
+1 I got to this pull request from https://gist.github.com/jroper/6374383. I think this is the pull request of the "Scala incremental compiler improvements"? The result of the fix looks amazing! Thanks! |
gkossakowski
added a commit
to gkossakowski/sbt
that referenced
this issue
Oct 24, 2013
Fix the problem with unstable names synthesized for existential
types (declared with underscore syntax) by renaming type variables
to a scheme that is guaranteed to be stable no matter where given
the existential type appears.
The sheme we use are De Bruijn-like indices that capture both position
of type variable declarion within single existential type and nesting
level of nested existential type. This way we properly support nested
existential types by avoiding name clashes.
In general, we can perform renamings like that because type variables
declared in existential types are scoped to those types so the renaming
operation is local.
There's a specs2 unit test covering instability of existential types.
The test is included in compiler-interface project and the build
definition has been modified to enable building and executing tests
in compiler-interface project. Some dependencies has been modified:
* compiler-interface project depends on api project for testing
(test makes us of SameAPI)
* dependency on junit has been introduced because it's needed
for `@RunWith` annotation which declares that specs2 unit
test should be ran with JUnitRunner
SameAPI has been modified to expose a method that allows us to
compare two definitions.
Also, a integration test has been added which tests scenario
mentioned in sbt#823.
This commit fixes sbt#823.
Proper fix for unstable existential names.
gkossakowski
added a commit
to gkossakowski/sbt
that referenced
this issue
Oct 29, 2013
Fix the problem with unstable names synthesized for existential
types (declared with underscore syntax) by renaming type variables
to a scheme that is guaranteed to be stable no matter where given
the existential type appears.
The sheme we use are De Bruijn-like indices that capture both position
of type variable declarion within single existential type and nesting
level of nested existential type. This way we properly support nested
existential types by avoiding name clashes.
In general, we can perform renamings like that because type variables
declared in existential types are scoped to those types so the renaming
operation is local.
There's a specs2 unit test covering instability of existential types.
The test is included in compiler-interface project and the build
definition has been modified to enable building and executing tests
in compiler-interface project. Some dependencies has been modified:
* compiler-interface project depends on api project for testing
(test makes us of SameAPI)
* dependency on junit has been introduced because it's needed
for `@RunWith` annotation which declares that specs2 unit
test should be ran with JUnitRunner
SameAPI has been modified to expose a method that allows us to
compare two definitions.
This commit also adds `ScalaCompilerForUnitTesting` class that allows
to compile a piece of Scala code and inspect information recorded
callbacks defined in `AnalysisCallback` interface. That class uses
existing ConsoleLogger for logging. I considered doing the same for
ConsoleReporter. There's LoggingReporter defined which would fit our
usecase but it's defined in compile subproject that compiler-interface
doesn't depend on so we roll our own.
Also, a integration test has been added which tests scenario
mentioned in sbt#823.
This commit fixes sbt#823.
|
I accidentally reopened this issue but clicking the wrong button on issue list. I'm very sorry for that. Closing again. |
gkossakowski
added a commit
that referenced
this issue
Mar 21, 2014
Fix the problem with unstable names synthesized for existential
types (declared with underscore syntax) by renaming type variables
to a scheme that is guaranteed to be stable no matter where given
the existential type appears.
The sheme we use are De Bruijn-like indices that capture both position
of type variable declarion within single existential type and nesting
level of nested existential type. This way we properly support nested
existential types by avoiding name clashes.
In general, we can perform renamings like that because type variables
declared in existential types are scoped to those types so the renaming
operation is local.
There's a specs2 unit test covering instability of existential types.
The test is included in compiler-interface project and the build
definition has been modified to enable building and executing tests
in compiler-interface project. Some dependencies has been modified:
* compiler-interface project depends on api project for testing
(test makes us of SameAPI)
* dependency on junit has been introduced because it's needed
for `@RunWith` annotation which declares that specs2 unit
test should be ran with JUnitRunner
SameAPI has been modified to expose a method that allows us to
compare two definitions.
This commit also adds `ScalaCompilerForUnitTesting` class that allows
to compile a piece of Scala code and inspect information recorded
callbacks defined in `AnalysisCallback` interface. That class uses
existing ConsoleLogger for logging. I considered doing the same for
ConsoleReporter. There's LoggingReporter defined which would fit our
usecase but it's defined in compile subproject that compiler-interface
doesn't depend on so we roll our own.
ScalaCompilerForUnit testing uses TestCallback from compiler-interface
subproject for recording information passed to callbacks. In order
to be able to access TestCallback from compiler-interface
subproject I had to tweak dependencies between interface and
compiler-interface so test classes from the former are visible in the
latter. I also modified the TestCallback itself to accumulate apis in
a HashMap instead of a buffer of tuples for easier lookup.
An integration test has been added which tests scenario
mentioned in #823.
This commit fixes #823.
Conflicts:
project/Util.scala
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Consider the following code structure:
Let's assume it's been compiled.
Now, let's introduce a private method to
Fooso it looks like this:If we recompile we would expect one iteration of compilation (just to recompile
Foo.scala) because the newly introduced method is private. At the moment, incremental compiler will decide to recompileBar.scalaas well because it sees changed signature offoomethod. The reason its seen as changed is name instability for existential types introduced with widlcard syntax. What happens is that Scala compiler has a global counter for ids per compilation unit and uses that counter for naming existential types. Therefore even private definition can affect a signature a public definition thus causing spurious recompilation.The text was updated successfully, but these errors were encountered: