Skip to content

[CS] Connect arg-to-param constraint to function builder #33005

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 3 commits into from
Jul 21, 2020

Conversation

hamishknight
Copy link
Contributor

Previously we could inadvertently split the constraint system without realizing that a function builder with a generic argument may allow the closure body to reference a type variable that connects it to the enclosing expression.

Fix this issue by checking for an unresolved closure argument and forming an unresolved argument conversion constraint that includes any type variables from the function builder type.

Resolves SR-13183
Resolves rdar://problem/65695054

Factor out the callee locator computation and
function builder opening logic.
We used to apply builder transforms to regular
functions without using a constraint system,
which is what the assertion is primarily geared
towards. However we now use a constraint system
for such cases, with the lack of a constraint
system only being used to check if we can even
apply the transform in the first place.
Previously we could inadvertently split the
constraint system without realizing that a function
builder with a generic argument may allow the
closure body to reference a type variable that
connects it to the enclosing expression.

Fix this issue by checking for an unresolved
closure argument and forming an unresolved argument
conversion constraint that includes any type
variables from the function builder type.

Resolves SR-13183
Resolves rdar://problem/65695054
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@swift-ci please test source compatibility

@hamishknight hamishknight requested a review from xedin July 20, 2020 18:50
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

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