Avoid junk produced by Constraint#replace. #678
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
There were two instances where a constraint undergoing a replace would still refer to poly params that are no longer bound after the replace. 1. In an ordering the replaced parameters was n ot removed from the bounds of the others. 2. When a parameter refers to the replaced parameter in a type, (not a TypeBounds), the replaced parameter was not replaced. We now have checking in place that in globally committable typer states, TypeVars are not instantiated to PolyParams and (configurable) that constraints of such typer states are always closed. Fixes scala#670.
| def removeFromOrdering(po: ParamOrdering) = { | ||
| def removeFromBoundss(key: PolyType, bndss: Array[List[PolyParam]]): Array[List[PolyParam]] = { | ||
| val bndss1 = bndss.map(bnds => bnds.filterConserve(_.binder ne pt)) | ||
| if ((0 until bndss.length).forall(i => bndss(i) eq bndss1(i))) bndss else bndss1 |
There was a problem hiding this comment.
Maybe:
var removed = false
val bndss1 = bndss.map(bnds => bnds.filterConserve(x => (x.binder ne pt) || { removed = true; false }))
if (removed) bndss1 else bndssThere was a problem hiding this comment.
I settled for:
val bndss1 = bndss.map(_.filterConserve(_.binder ne pt))
if (bndss.corresponds(bndss1)(_ eq _)) bndss else bndss1
|
The commit comment has a few lines longer than the git convention, makes it hard to read in the GitHub UI. |
Uncomment two lines that were commented out by accident.
Find a nicer way to express the same logic.
The operation on SimpleMap is eager. As suggested by @retronym we should find a name different from the lazy Map#mapValues operation.
odersky
added a commit
that referenced
this pull request
Jun 25, 2015
Avoid junk produced by Constraint#replace.
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.
There were two instances where a constraint undergoing a replace would still refer
to poly params that are no longer bound after the replace.
replaced parameter was not replaced.
We now have checking in place that in globally committable typer states, TypeVars are not instantiated
to PolyParams and (configurable) that constraints of such typer states are always closed.
Fixes #670. Review by @retronym