Skip to content

Have IsBindableVisitor consider conditional conformances #39294

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

slavapestov
Copy link
Contributor

This PR fixes a validation test failure in #38977.

jckarter and others added 4 commits September 13, 2021 22:55
In order to correctly handle upper bound requirements arising from conditional conformances,
we need to be able to ingest requirements introduced by conditional conformance extensions,
which means recursively visiting the parent is no longer possible to do cleanly. It is
simpler to substitute the generic type's entire generic context as a whole.
The upper bound on a nominal type's generic argument can have more requirements imposed on it than those written in the original nominal type's generic signature, depending on the requirements imposed on the nominal type itself, thanks to conditional conformances. IsBindableVisitor failed to take this into account when visiting the bindings of generic type arguments. This could cause Type::isBindableTo to provide a false positive for a substituted type that doesn't satisfy conditional conformances, but more importantly, SIL type lowering uses the same visitor to extract an upper bound generic signature for substituted SIL function types, and if it doesn't preserve requirements from conditional conformances on nominal types in that signature, it can end up building an incorrect substitution map. Fix this by passing down the upper bound from generic arguments even when visiting nominal types, and using those upper bounds to check for conditional conformances that add requirements to generic arguments while visiting them.
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

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