Sema: Fix a couple of issues related to variance of protocol 'Self' #41757
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
… for generic context We don't want to pass in the outer generic signature here. The base type's constraint type is written in terms of archetypes, and we run generic signature queries against it with types appearing in the protocol member. Since the protocol member has Self at depth 0, index 0, prepending the outer generic signature to the opened existential signature would produce incorrect results.
slavapestov
requested review from
hamishknight,
CodaFi and
AnthonyLatsis
and removed request for
hamishknight
Comment on lines
+589
to
+596
| // Make sure this also works in a generic context! | ||
| struct G<X, Y, Z> { | ||
| func doIt() { | ||
| let exist: any UnfulfillableGenericRequirements | ||
|
|
||
| exist.method8(false) | ||
| // expected-error@-1 {{member 'method8' cannot be used on value of type 'any UnfulfillableGenericRequirements'; consider using a generic constraint instead}} | ||
| } |
There was a problem hiding this comment.
Oh, if only these declarations could just be anonymous.
…elated case There are three kinds of invariant 'Self' uses here: - 'Self' appears as the left hand side of a same-type requirement between type parameters - 'Self' appears as a structural component of the right hand side of a concrete type requirement - 'Self' appears as a structural component of the right hand side of a superclass requirement My previous fix only handled the first case. Generalize it to handle all three. Fixes rdar://problem/74944514.
slavapestov
force-pushed
the
protocol-self-reference-fixes
branch
from
318ea19 to
599bb79
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
AnthonyLatsis
approved these changes
Mar 9, 2022
There was a problem hiding this comment.
I'm wondering if the new variance check in the conformance checker, findExistentialSelfReferences() and Sema's doesMemberHaveUnfulfillableConstraintsWithExistentialBase() can all be unified somehow.
Unifying the first and last sounds simple enough, I could see to that in a follow-up. I think the type signature check should have its own entry point: apparently there are places like this one where only generic requirements are of interest (and vice versa?).
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 first one is a fix for a regression from #41677. The second one is rdar://problem/74944514, which is a follow-up to #41545.
I'm wondering if the new variance check in the conformance checker, findExistentialSelfReferences() and Sema's doesMemberHaveUnfulfillableConstraintsWithExistentialBase() can all be unified somehow.