Skip to content

Sema: Allow non-final classes to satisfy properties and subscripts with covariant Self #34005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 25, 2020

Conversation

AnthonyLatsis
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis commented Sep 20, 2020

Hopefully, whatever complications were impeding this in 926e371 are long gone.

Bonus: now we can use protocols with such requirements as types (iff the requirement was the reason we previously couldn't).

@AnthonyLatsis
Copy link
Collaborator Author

Should I be adding some execution tests?

Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Runtime tests would be nice to make sure the generated witness thunk is valid and recovers the correct Self type.

This ensures that we erase opened archetypes before closing an existential result
@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test

@AnthonyLatsis AnthonyLatsis changed the base branch from master to main September 23, 2020 22:37
Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've extended some silgen tests to cover subscripts and properties as well. My understanding of the output is rather sketchy, but there is one thing that looks suspicious to my eye.

Comment on lines +97 to +108
// CHECK: [[PCOPY_ADDR:%[0-9]+]] = open_existential_addr immutable_access [[P]] : $*P to $*@opened([[N:".*"]]) P
// CHECK: [[P_RESULT:%[0-9]+]] = alloc_stack $P
// CHECK: [[PCOPY_ADDR_1:%[0-9]+]] = alloc_stack $@opened([[N]]) P
// CHECK: copy_addr [[PCOPY_ADDR]] to [initialization] [[PCOPY_ADDR_1]] : $*@opened([[N]]) P
// CHECK: [[P_P_GETTER:%[0-9]+]] = witness_method $@opened([[N]]) P, #P.p!getter : {{.*}}, [[PCOPY_ADDR]]{{.*}} : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: [[P_RESULT_ADDR2:%[0-9]+]] = init_existential_addr [[P_RESULT]] : $*P, $@opened([[N]]) P
// CHECK: apply [[P_P_GETTER]]<@opened([[N]]) P>([[P_RESULT_ADDR2]], [[PCOPY_ADDR_1]]) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: destroy_addr [[PCOPY_ADDR_1]] : $*@opened([[N]]) P
// CHECK: destroy_addr [[P_RESULT]] : $*P
// CHECK: dealloc_stack [[PCOPY_ADDR_1]] : $*@opened([[N]]) P
// CHECK: dealloc_stack [[P_RESULT]] : $*P
_ = p.p
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this output happens to be sane, why do we need PCOPY_ADDR_1 in addition to PCOPY_ADDR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's just SILGen inserting an unnecessary copy. It does that sometimes.

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants