Merged
Conversation
Contributor
Author
|
Demoted to WIP ... it's along the right lines, but needs more work. |
Contributor
Author
|
Although the multiple stabilizer issue was real, in the actual scenario, shapeless's This is ready for review now ... |
Contributor
Author
|
/synch |
Contributor
Author
|
/rebuild |
a0b1f9d to
0643e74
Compare
Contributor
Author
|
Rebased in the hope of unwedging Jenkins. |
Contributor
|
/synch |
1 similar comment
Contributor
Author
|
/synch |
Contributor
Author
|
More synch did it! |
Contributor
Author
|
Rebased. |
scala#5999 introduces a synthetic val for the unstable LHS of an implicit method application where the method type depends on the LHS. Unfortunately this only handles the case where an expression requires a _single_ stabilizing val: if the expression involves chained method applications, each requiring a stabilizing val, then only the last of these is emitted resulting in a failure during bytecode generation, object Test { class Bar { class Foo { type Out } object Foo { implicit def foo: Foo { type Out = Bar } = ??? } def foo(implicit foo: Foo): foo.Out = ??? } (new Bar).foo.foo } results in a NoSuchElementException, java.util.NoSuchElementException: value stabilizer$1 at scala.collection.mutable.AnyRefMap$ExceptionDefault.apply(AnyRefMap.scala:425) at scala.collection.mutable.AnyRefMap$ExceptionDefault.apply(AnyRefMap.scala:424) at scala.collection.mutable.AnyRefMap.apply(AnyRefMap.scala:180) at scala.tools.nsc.backend.jvm.BCodeSkelBuilder$PlainSkelBuilder$locals$.load(BCodeSkelBuilder.scala:392) ... Prior to this commit only a single stabilizer is recorded on the expression tree which is overwritten by later ones. This is fixed here by allowing multiple stabilizers to be recorded and emitted. In addition to that, previously spurious dependencies were picked up due to insufficient dealiasing. The following, object Test { class Bar { class Foo { type Out } object Foo { implicit def foo: Foo { type Out = Bar } = ??? } def foo(implicit foo: Foo): foo.Out = ??? } (new Bar).foo.foo } triggered the preceding error despite not having a true dependency which would require a stabilizer in the first place. The test for a dependent argument now dealiases to eliminate these spurious dependencies. Finally we avoid retyping stabilizing definitions when they are emitted, fixing scala/bug#10736.
lrytz
approved these changes
Mar 7, 2018
|
I think this PR caused the following regression scala/bug#11397 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes scala/bug#10714. Also fixes scala/bug#10736.
#5999 introduces a synthetic val for the unstable LHS of an implicit method application where the method type depends on the LHS. Unfortunately this only handles the case where an expression requires a single stabilizing val: if the expression involves chained method applications, each requiring a stabilizing val, then only the last of these is emitted resulting in a failure during bytecode generation,
results in a
NoSuchElementException,Prior to this PR only a single stabilizer is recorded on the expression tree which is overwritten by later ones. This is fixed here by allowing multiple stabilizers to be recorded and emitted.
In addition to that, previously spurious dependencies were picked up due to insufficient dealiasing. The following,
triggered the preceding error despite not having a true dependency which would require a stabilizer in the first place. The test for a dependent argument now dealiases to eliminate these spurious dependencies.
Finally we avoid retyping stabilizing definitions when they are emitted, fixing scala/bug#10736.