Skip to content

[Refactoring] Replace lifted breaks/returns with placeholder for async transform #37189

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 4 commits into from
May 5, 2021

Conversation

hamishknight
Copy link
Contributor

@hamishknight hamishknight commented Apr 30, 2021

If we're lifting them outside of the control flow structure they're dealing with, turn them into placeholders, as they will no longer perform the control flow the user is expecting.

This handles:

  • Return statements at the top-level of the callback.
  • Break statements in switches that we re-write.

In addition, fix a bug where we'd inadvertently transform an unrelated switch statement in the callback.

Resolves rdar://74014897.

@hamishknight hamishknight requested a review from bnbarham April 30, 2021 22:51
And have it automatically track to the end of the
token, as that's the behavior we seem to always
want.
We were missing a `return` here to ignore any
switch statements that don't have anything to do
with the error handling.
If we're lifting them outside of the control flow
structure they're dealing with, turn them into
placeholders, as they will no longer perform the
control flow the user is expecting.

This handles:
- Return statements at the top-level of the callback.
- Break statements in switches that we re-write.

Resolves rdar://74014897.
Make sure we match against an exact return or
break in these cases, rather than a placeholder.
@hamishknight hamishknight force-pushed the break-it-down-for-me branch from 9127897 to 2d75074 Compare May 4, 2021 13:30
@hamishknight hamishknight changed the title [Refactoring] Replace unhandled break/return with placeholder for async transform [Refactoring] Replace lifted breaks/returns with placeholder for async transform May 4, 2021
@swiftlang swiftlang deleted a comment from swift-ci May 4, 2021
@swiftlang swiftlang deleted a comment from xymus May 4, 2021
@hamishknight
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Looking good, thanks Hamish.

@@ -4680,6 +4686,7 @@ struct CallbackClassifier {
void classifySwitch(SwitchStmt *SS) {
if (!IsResultParam || singleSwitchSubject(SS) != ErrParam) {
CurrentBlock->addNode(SS);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Oof. Thanks for catching this and adding the 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