Fix #1401: Make sure all references are forwarded #1407
Merged
Conversation
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
|
Review by @liufengyun ? |
- increase page width - print scopes more legibly under -verbose
Faced with recursive dependencies through self types, we might have to apply `normalizeToClassRefs` to a class P with a parent that is not yet initialized (witnessed by P's parents being Nil). In that case we should still execute forwardRefs on P, but we have to wait in a suspension until P is initialized. This avoids the problem raised in scala#1401. I am still not quite sure why forwardRefs is needed, but it seems that asSeenFrom alone is not enough to track the dependencies in this case.
|
Rebased to master |
odersky
added
area:reporting
| @@ -106,7 +106,7 @@ object Scala2Unpickler { | |||
| // `denot.sourceModule.exists` provision i859.scala crashes in the backend. | |||
| denot.owner.thisType select denot.sourceModule | |||
| else selfInfo | |||
| denot.info = ClassInfo(denot.owner.thisType, denot.classSymbol, Nil, decls, ost) // first rough info to avoid CyclicReferences | |||
| denot.info = new TempClassInfo(denot.owner.thisType, denot.classSymbol, decls, ost) // first rough info to avoid CyclicReferences | |||
There was a problem hiding this comment.
It's not clear to me where the suspensions of this TempClassInfo is executed. For the TempClassInfo in Namer, there's an explicit call tempClassInfo.suspensions.foreach(_()).
There was a problem hiding this comment.
Good catch (they were never executed). I added a commit that fixes that. I could not come up with a test case specifically for classfile reading, but it seems obvious that the treatment
should be the same as in Namer.
Make treatment in Scala2Unpickler and Namer the same and factor out common functionality.
|
LGTM |
|
Thanks @odersky for fixing this tricky bug. |
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.
I minimized the problem to i1401.scala. If we swap Buffer and BufferLike, the test already succeeds without this PR. The difference between the two orders is that we are lacking some parameter forwarders in the error scenario. This PR fixes that.