Skip to content

AssociatedTypeInference: Allow Self as a fixed type witness #32799

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
Aug 6, 2020

Conversation

AnthonyLatsis
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis commented Jul 9, 2020

To gain more control over what we reject in computeFixedTypeWitness, pull the nested type out of the Self archetype rather than asking the generic signature for a concrete type.

Type concreteType = genericSig->getConcreteType(dependentType);
if (!concreteType) continue;
const auto nestedType =
contextSelf->getNestedType(assocType->getName())->mapTypeOutOfContext();
Copy link
Contributor

Choose a reason for hiding this comment

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

How about mapTypeIntoContet(assocType->getDeclaredInterfaceType())

Copy link
Contributor

Choose a reason for hiding this comment

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

... oops, I missed the mapTypeOutOfContext(). I think this is easier to accomplish using genericSig->getCanonicalTypeInContext()

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Jul 10, 2020

Choose a reason for hiding this comment

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

Currently, we only look through protocols that inherit the protocol of the given associated type, but I was going to remove that limitation eventually, because a fixed type witness can come from an unrelated protocol with a matching associated type:

protocol P {
  associatedtype A
}
protocol P2: P where A == Int {}
protocol Q {
  associatedtype A
}
struct Foo: Q, P2 {} // error while checking conformance to Q

The problem with getCanonicalTypeInContext is that it fails when given a foreign dependent member, so I decided to build on the name of the associated type to avoid reimplementing this later.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if you use DependentMemberType::get() to get the associated type-less form, using just an identifier? It would still avoid mapping in and out of context.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool, thanks!

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test

@AnthonyLatsis
Copy link
Collaborator Author

@slavapestov This is ready to go as well, I suppose?

@AnthonyLatsis
Copy link
Collaborator Author

@slavapestov ping

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