[Sema] Penalize conversions to Any. #7222
Merged
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
…ersion. This change is needed by a separate change I have that penalizes conversions to Any. That change exposed a problem where we ended up doing a direct lvalue-to-Any conversion without first loading as part of an lvalue-to-rvalue conversion. We should really only be doing most of these other conversions/fixes on rvalues. Unfortunately I don't know of a way to hit this without the other change, and thus don't have a test case. As a side note, this also helps some (perhaps most, but definitely not all) of the exponential type inference cases involving arrays and dictionaries. Unfortunately again, we don't currently have a testing mechanism in place to ensure we don't regress those cases, and I don't want to add a test that will completely hang every bot if it does regress, so...*sad face*...no test case for that either.
We previously penalized bindings to Any, and that resulted in inappropriately rejecting solutions where we bind a decl that is explicitly typed Any. That penalty was removed in 170dc8a, but that has led to a variety of source compatibility issues. So now, we'll reintroduce a penalty for Any-typed things, but instead of penalizing bindings (which happen both because of explicitly-typed values in the source, and implicitly due to attempting various types for a particular type variable), penalize casts, which are always present in some form in the source. Thanks go to John for the suggestion. Fixes SR-3817 (aka rdar://problem/30311052) SR-3786 (aka rdar://problem/30268529) rdar://problem/29907555
jrose-apple
reviewed
Feb 3, 2017
| @@ -407,8 +407,10 @@ enum ScoreKind { | |||
| SK_ScalarPointerConversion, | |||
| /// A conversion from an array to a pointer of matching element type. | |||
| SK_ArrayPointerConversion, | |||
| /// A conversion to an empty existential type ('Any' or '{}'). | |||
There was a problem hiding this comment.
Oh this is a partial revert of code that I pulled out a while back. I take that to mean empty protocol composition.
There was a problem hiding this comment.
Okay, but that's never been a valid spelling for an empty protocol composition. It's not important enough to reset your smoke test for, but if you do another round here can you remove it?
There was a problem hiding this comment.
Yes. I'll definitely be revisiting this area at some point in the nearish future.
|
@swift-ci Please smoke test |
DougGregor
approved these changes
Feb 3, 2017
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.
[Sema] Penalize conversions to Any.
We previously penalized bindings to Any, and that resulted in inappropriately rejecting solutions where we bind a decl that is explicitly typed Any. That penalty was removed in 170dc8a, but that has led to a variety of source compatibility issues.
So now, we'll reintroduce a penalty for Any-typed things, but instead of penalizing bindings (which happen both because of explicitly-typed values in the source, and implicitly due to attempting various types for a particular type variable), penalize casts, which are always present in some form in the source.
Thanks go to John for the suggestion.
Fixes