"fix" a regression in the 5.2 merge #3004
Merged
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.
This adds a test that newly fails on the 5.2 branch, and a fix, in two commits.
The test passes upstream and seems fine, so I think this is a regression in the merge. But I don't think the fix here (restoring a piece of the previous implementation of checking
Pexp_constraint
s in typecore) is correct. I'm just posting this so @mshinwell can use it to make more progress and so I can point to it for discussion.What is happening is that
loosen_arrow_modes
intype_argument
is updating the scope of the type, and the fact that the previous implementation was throwing away the old type meant we ignored that. That seems wrong, instead, I thinktype_argument
shouldn't need to use the equation for mode checking in this example, and so this program typechecks in 5.1 for the wrong reason.Will discuss with modes people.