Fix #3945: Fix eta expansion for partially applied methods #3981
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
biboudis
reviewed
Feb 13, 2018
There was a problem hiding this comment.
If I remove the explicit type annotation from int in q I get:
9 | val p: (String => String) => (String) => (Int) => String = int
| ^
| found: String => Nothing => String => Int => String
| required: String => String => String => Int => String
|After tracing the constraint set a bit I see that the constraint A <: String is indeed added. But the type is approximated from below and is instantiated with Nothing. WDYT?
Take bindingTree into account when deciding which type variables to instantiate before an implicit search. The additions to i3945.scala fail without this change. Also two other tweaks.
|
Test output for failed tests is missing. I just see |
Looking into it |
odersky
added a commit
to dotty-staging/dotty
that referenced
this pull request
Mar 1, 2018
This is an altogether unsatisfying scenario. In scala#4032 there's an unconstrained type variable that should be interpolated and it's a coin flip whether we instantiate it to the lower or upper bound. A completely unrelated change in scala#3981 meant that we instantiated the variable to the upper instead of the lower bound which caused the program to fail. We now fix it by adding another condition which is also more or less arbitrary: if neither lower bound nor upper bound is defined, we now prefer again the lower bound. But all of this is quite arbitrary. The fact is that we do cut off some parts of the search space in arbitrary ways and programs have come on rely on the specific arbitrary way in which we do it.
odersky
added a commit
to dotty-staging/dotty
that referenced
this pull request
Mar 2, 2018
Intrepolation is a pretty delicate scenario. It's difficult to even say what is right and what is wrong. In scala#4032 there's an unconstrained type variable that should be interpolated and it's a coin flip whether we instantiate it to the lower or upper bound. A completely unrelated change in scala#3981 meant that we instantiated the variable to the upper instead of the lower bound which caused the program to fail. The failure was because the type variable appeared covariantly in the lower bound of some other type variable. So maximizing the type first type variable overconstrained the second. We avoid this problem now by computing the variance of the type variable that's eliminated in the rest of the constraint. We take this into account to instantiate the variable so that it narrows the overall constraint the least, defaulting again to lower bound if there is not best direction. Since the test is expensive (linear in the constraint size), we do this only if the variable's lower bound is unconstrained. We could do it for all non-occurring type variables, but have to see how that would affect performance.
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.
This was a tricky one to fix.