Skip to content

GenericSignatureImpl, #31712: Plug remaining relevant methods with ty… #31927

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 1 commit into from
May 21, 2020

Conversation

AnthonyLatsis
Copy link
Collaborator

…pe param. assertions

Follow-up to #31712

@AnthonyLatsis AnthonyLatsis requested a review from CodaFi May 20, 2020 20:51
@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please test macOS

@@ -428,7 +428,7 @@ Type GenericSignatureImpl::getSuperclassBound(Type type) const {
/// required to conform.
GenericSignature::RequiredProtocols
GenericSignatureImpl::getRequiredProtocols(Type type) const {
if (!type->isTypeParameter()) return { };
assert(type->isTypeParameter() && "Expected a type parameter");
Copy link
Contributor

Choose a reason for hiding this comment

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

It would also be nice to assert if resolveEquivalenceClass() returns an unresolved type or concrete type, instead of returning an empty array, but I don't know what the fallout from that might be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you think calling this on a same-type-to-concrete type parameter and getting back an empty array might backfire on us?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would expect that any code doing so is already incorrect, because we can't hope to return a consistent result in this case. Should it be empty, or reflect any existing (possibly redundant) conformance constraints?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see. There is a similar issue with getSuperclassBound, and others, I guess. For example, in checkTypeWitness, we always want an explicit bound, not one implied by a conformance constraint, to avoid checking both inheritance and conformance (a syntactic conformance, even if it's broken, should always be enough for the candidate to pass).

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - b25c466

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please test source compatibility

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please clean test

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please clean test source compatibility

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please test source compatibility

@AnthonyLatsis
Copy link
Collaborator Author

Debug builder failure unrelated

@AnthonyLatsis AnthonyLatsis merged commit 24fa750 into swiftlang:master May 21, 2020
@AnthonyLatsis AnthonyLatsis deleted the assert-type-param-3 branch May 21, 2020 19:21
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.

4 participants