-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[5.0] Allow Error to conform to itself #21073
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
[5.0] Allow Error to conform to itself #21073
Conversation
Most of the foundation for this was laid in earlier patches.
@swift-ci Please test. |
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.
Looks good, just a few cleanup suggestions
|
||
// Due to an IRGen limitation, witness tables cannot be passed from an | ||
// existential to an archetype parameter, so for now we restrict this to | ||
// @objc protocols. |
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.
This comment is out of date?
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.
It certainly needs updating, yeah. I'll do that on master.
@@ -3575,15 +3575,23 @@ SILGenFunction::emitVTableThunk(SILDeclRef derived, | |||
enum class WitnessDispatchKind { | |||
Static, | |||
Dynamic, | |||
Class | |||
Class, | |||
Witness |
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.
A comment here would be nice
getSelfTypeAndConformanceForWitness(SILDeclRef witness, SubstitutionMap subs) { | ||
auto protocol = cast<ProtocolDecl>(witness.getDecl()->getDeclContext()); | ||
auto selfParam = protocol->getProtocolSelfType()->getCanonicalType(); | ||
auto type = subs.getReplacementTypes()[0]; |
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 selfParam.subst(subs)
is more robust
auto origWitnessFTy = getWitnessFunctionType(SGM, witness, witnessKind); | ||
auto witnessFTy = origWitnessFTy; | ||
if (!witnessSubs.empty()) | ||
witnessFTy = origWitnessFTy->substGenericArgs(SGM.M, witnessSubs); |
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.
You don't have to check if the subs are non-empty here
5.0 version of #20629. Includes the patches from #20658 and #20662.
ABI-additive: the stdlib exports some extra symbols. Not specifically important for convergence, but eventually necessary in order to implement SE-0235 in Swift 5.0.