Fix substParam(s) #16082
Closed
Fix substParam(s) #16082
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
The `substParam` and `substParams` overlooked a case: If we substitute P := T and we have a type variable A with origin P then A should map to T as well. Previously, it didn't which means that substParam(s) could break the link between a type variable and the type parameter it stands for. This gives more freedoms to instantiate type variables than allowed so it can mask type errors.
|
I noticed this when working on #16042. Any attempt to do fixes to get dependencies right would break the CB. I thought first my changes were the problem, but in fact my fixes to dependencies also worked around the soundness hole addressed by this PR. And I believe that tightening of the rules is what broke the CB. So, unfortunately, it seems we will have some type errors in the CB that were previously not detected and now have to be fixed. |
|
@smarter Or can you see a legitimate reason why substituting type parameters should leave associated type variables out? The problem seems to arise in |
|
It turns out empirically that this introduces errors where there are none. |
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
substParamandsubstParamsoperations overlooked a case: If we substitute P := T and we have a type variable A with origin P then A should map to T as well. Previously, it didn't, which means that substParam(s) could break the link between a type variable and the type parameter it stands for. This gives more freedoms to instantiate type variables than allowed so it can mask type errors.