-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #2948: Use symbol's info when mapping inherited denotations #2970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
Approximating type maps now work with bounds instead of NoTypes. This gives more precision (in particular for type applications), and also makes it possible to combine approximation with other mappings. As a side-effect we provide the hooks for preventing constructing illegal types C#A where C is non-singleton and A is abstract by setting variance to 0 for the prefix of an abstract type selection. There's one test case that fails: One of the types in dependent-exractors.scala does not check out anymore. This has likely to do with the loss of precision incurred by the maps. Exact cause remains to be tracked down.
A Range, if it survives, will always lead to a situation where the upper bound appears in a covariant position in the result, and the lower bound appears in a contravariant position. Hence, when we apply a type map to the argument in a range we should take this into account. Fixes a previous failure in t2435.scala and dependent-extractors.scala.
Supersedes old scheme of dealing with unstable prefixes in non-variant positions.
It's no longer needed.
- Use range instead of Range in AsSeenFromMap#apply. We needed Range before because we did an incorrect variance computation for NamedTypes. - Refine derivedSelect
This puts avoid on firmer ground.
In a commonly used derived... operation, deal with the common case that the types have not chanegd first.
The test case shows that it is inadmissible to combine the infos of inherited denotations into a new denotation. The problem here is that a type parameter CC was instantiated in an inherited denotation to Traversable, yet the parameter was afterwards instantiated to ListBuffer. This shows that infos from inheroted denotations are useless; instead we have to go back to the inherited symbol's infos and map them with an asSeenFrom. This fix also shows that one of the reasons for abandoning scala#2947 was wrong. We cannot form denotations from parent denotations in any case, so instantiating early should be fine with the change in this commit.
@felixmulder CI seems to hang here. How can we unblock it? |
@odersky I restarted drone and the tests for your PR. |
Looks like it's only running one job concurrently. Will try to fix it now. |
Alright, should be unblocked now. Guillaume and I both jumped on this at the same time, so we ended up killing and starting docker images left and right 😂 |
Only last two commits need to be reviewed. |
Last two commits LGTM |
nicolasstucki
approved these changes
Aug 16, 2017
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.
The test case shows that it is inadmissible to combine the infos of inherited denotations
into a new denotation. The problem here is that a type parameter CC was instantiated in
an inherited denotation to Traversable, yet the parameter was afterwards instantiated to
ListBuffer. This shows that infos from inherited denotations are useless; instead we have
to go back to the inherited symbol's infos and map them with an asSeenFrom.
This fix also shows that one of the reasons for abandoning #2947 was wrong. We cannot form
denotations from parent denotations in any case, so instantiating early should be fine with
the change in this commit.
Based on #2945