Fix healAmbiguous to compareAlternatives with disambiguate = true
#21339
Conversation
This file contains 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
We now use a left-biased scheme, as follows. From 3.6 on: - A given x: X is better than a given or implicit y: Y if y can be instantiated/widened to X. - An implicit x: X is better than a given or implicit y: Y if y can be instantiated to a supertype of X. - Use owner score for givens as a tie breaker if after all other tests we still have an ambiguity. This is not transitive, so we need a separate scheme to work around that. Other change: - Drop special handling of NotGiven in prioritization. The previous logic pretended to do so, but was ineffective.
We only have transitivity between givens or between implicits. To cope with that - We tank first all implicits, giving a best implicit search result. - Then we rank all givens startign with the implicit result. If there is a given that is better than the best implicit, the best given will be chosen. Otherwise we will stick with the best implicit.
Warnings from 3.6, change in 3.7. This is one version later than originally planned.
Make the wording of a priority change warning message stable under different orders of eligibles. We now always report the previously chosen alternative first and the new one second. Note: We can still get ambiguities by fallging different pairs of alternatives depending on initial order.
odersky
reviewed
Aug 6, 2024
EugeneFlesselle
force-pushed
the
given-healAmbiguous
branch
2 times, most recently
from
be388ca to
dcf90e1
Compare
On the final result, compared with all the ambiguous candidates we are trying to recover from. We should still use `disambiguate = false` when filtering the `pending` candidates for the purpose of warnings, as in the other cases. Before the changes, it was possible for an ambiguous SearchFailure to be healed by a candidate which was considered better (possibly only) under a prioritization scheme different from the current one. As an optimization, we can avoid redoing compareAlternatives in versions which could have only used the new prioritization scheme to begin with. Also restores behaviour avoiding false positive warnings. Specifically, in cases where we could report a change in prioritization, despite having not yet done `tryImplicit` on the alternative, i.e. it was only compared as part of an early filtering See scala#21045 for related changes
EugeneFlesselle
force-pushed
the
given-healAmbiguous
branch
from
dcf90e1 to
0361991
Compare
Gedochao
added
the
backport:nominated
odersky
approved these changes
Aug 7, 2024
|
@EugeneFlesselle Please merge unless there's something else you want to add. |
This was referenced Aug 8, 2024
WojciechMazur
added
backport:accepted
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.
Based on #21328
Opened here to have it run the CI