Skip to content

[CSApply] Fixed type-checked subexpression output bug for multi-state… #59260

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 2 commits into from
Jun 18, 2022

Conversation

amritpan
Copy link
Member

@amritpan amritpan commented Jun 3, 2022

…ment closures

Previously the type-checked subexpressions of a multi-statement closure were being printed with only partial bindings. This fix ensures that subexpressions are printed with all of their type-checked bindings.

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! Note that there is a slight semantic difference here as well - rewriteTarget is called for each syntactic element in the body but applySolution is called only once which means that processing is going to be more incremental in this case too.

@xedin
Copy link
Contributor

xedin commented Jun 3, 2022

@swift-ci please test

@xedin
Copy link
Contributor

xedin commented Jun 3, 2022

@swift-ci please test source compatibility

@xedin
Copy link
Contributor

xedin commented Jun 3, 2022

@amritpan Linux failures are what I was afraid of - looks putting processDelayed in rewriteTarget makes it too eager, we'd have to find another way to do this.

@amritpan amritpan force-pushed the multi-statement-closure-output branch from 6dbaaa9 to 3ad8638 Compare June 10, 2022 23:54
@amritpan amritpan closed this Jun 16, 2022
@amritpan amritpan force-pushed the multi-statement-closure-output branch from 3ad8638 to 9a0978a Compare June 16, 2022 22:32
@amritpan amritpan reopened this Jun 17, 2022
@amritpan amritpan force-pushed the multi-statement-closure-output branch from 6dbaaa9 to 3a38d7a Compare June 17, 2022 02:04
@amritpan amritpan force-pushed the multi-statement-closure-output branch from 380ff81 to 38d6ea8 Compare June 17, 2022 23:09
…e-checked to show their application solution status, as applicable for tap expressions and multi-statement closures.
@amritpan amritpan force-pushed the multi-statement-closure-output branch from 38d6ea8 to d7266d6 Compare June 18, 2022 01:01
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.

Looks great, thank you!

@xedin
Copy link
Contributor

xedin commented Jun 18, 2022

@swift-ci please test

@xedin xedin merged commit a391043 into swiftlang:main Jun 18, 2022
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