Skip to content

[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

Merged

Conversation

rjmccall
Copy link
Contributor

@rjmccall rjmccall commented Dec 6, 2018

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.

@rjmccall rjmccall requested a review from a team as a code owner December 6, 2018 03:14
@rjmccall rjmccall requested a review from DougGregor December 6, 2018 03:15
@rjmccall
Copy link
Contributor Author

rjmccall commented Dec 6, 2018

@swift-ci Please test.

@rjmccall rjmccall changed the title Allow Error to conform to itself [5.0] Allow Error to conform to itself Dec 6, 2018
@AnnaZaks AnnaZaks merged commit 1a3637c into swiftlang:swift-5.0-branch Dec 7, 2018
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.

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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
Copy link
Contributor

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];
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 selfParam.subst(subs) is more robust

auto origWitnessFTy = getWitnessFunctionType(SGM, witness, witnessKind);
auto witnessFTy = origWitnessFTy;
if (!witnessSubs.empty())
witnessFTy = origWitnessFTy->substGenericArgs(SGM.M, witnessSubs);
Copy link
Contributor

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

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.

3 participants