-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Sema: Pass substitution options to swift::checkTypeWitness #33581
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
Sema: Pass substitution options to swift::checkTypeWitness #33581
Conversation
...to propagate tentative type witnesses when validating a solution to associated type inference
|
@swift-ci please smoke test |
|
|
||
| superclass = superclass.subst(subMap); | ||
| // Replace type parameters with other known or tentative type witnesses. | ||
| superclass = superclass.subst( |
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 looks the same as calling superclass.subst(subMap, options). Is it different somehow because the type witness callback ends up calling the conformance lookup callback, and the SubstitutionMap one doesn't work?
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.
Almost. subst will use LookUpConformanceInSubstitutionMap when passed a substitution map, which fails for SubstitutionMap::getProtocolSubstitutions when the referenced associated type happens to originate from an inherited protocol. Am I doing the right thing, or ought we to remap associated types?
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 P inherits from Q, then a SubstitutionMap obtained by calling getProtocolSubstitutionMap() with a conformance to P should be able to resolve associated types of Q on the Self type. Does this not work?
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.
Actually, now that I think about it, it's not correct to use SubstitutionMap::getProtocolSubstitutions() here. The superclass is not written in terms of the protocol's generic signature, <T : 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.
What happens if you remove this call to subst()?
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 P inherits from Q, then a SubstitutionMap obtained by calling getProtocolSubstitutionMap() with a conformance to P should be able to resolve associated types of Q on the Self type. Does this not work?
The catch is that we're checking the conformance to Q in this case. Here's an example:
class G<T> {}
protocol P {
associatedtype X = Never
}
protocol Q: P {
associatedtype Y: G<X> = G<X>
}
struct Conformer: Q {}Actually, now that I think about it, it's not correct to use SubstitutionMap::getProtocolSubstitutions() here. The superclass is not written in terms of the protocol's generic signature, .
Is it not correct even for something spelled inside a protocol? It seems like substitution correctness is guaranteed through checkTypeWitness and a well-formed requirement signature.
What happens if you remove this call to subst()?
Tentative type witnesses are not considered, so X in G<X> will only get replaced with nested archetype.
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.
We could pass SubstOptions all the way down to GenericEnvironment::mapTypeIntoContext, but I was planning to make this a structural check anyways.
| LookUpConformanceInModule(dc->getParentModule()), options); | ||
|
|
||
| if (superclass->hasTypeParameter()) | ||
| superclass = dc->mapTypeIntoContext(superclass); |
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 looks like this part was missing before, so we would always call isExactSuperclassOf() with a contextual type vs an interface type, which would fail. I think that might a few bugs I've seen; I'll re-screen them after this lands
| } | ||
|
|
||
| // FIXME: Type witness resolution success is order-dependent. | ||
| // FIXME: resolveTypeWitnessViaLookup must not happen independently in the |
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 could also delay the full superclass check until we check the requirement signature at the end, after inferring all type witnesses. checkTypeWitness() could perform a structural check on the superclass relationship between ClassDecls only.
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.
Good idea, I'll write this down for upcoming improvements.
|
@swift-ci please smoke test macOS |
...to propagate tentative type witnesses when validating a solution to associated type inference.
Only abstract type witnesses can currently take advantage of this.