-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Conversation
Should I be adding some execution tests? |
…th covariant Self
0a8ca5b
to
103a821
Compare
There was a problem hiding this 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
@swift-ci please smoke test |
9ab98ba
to
9f58968
Compare
There was a problem hiding this 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.
// 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 |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
@swift-ci please smoke test |
@swift-ci please smoke test Windows |
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).