Skip to content

[Refactoring] Only unwrap optionals if the handler has an optional error #37235

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
May 6, 2021

Conversation

bnbarham
Copy link
Contributor

@bnbarham bnbarham commented May 4, 2021

Also added some extra tests for convert to async since it does actually vary from add async slightly.

Resolves rdar://73973459

@bnbarham bnbarham requested review from ahoppen and hamishknight May 4, 2021 05:13
@bnbarham bnbarham force-pushed the async-refactoring-mixed-optional branch from 94de88a to d1e3ace Compare May 4, 2021 06:07
@bnbarham
Copy link
Contributor Author

bnbarham commented May 4, 2021

@swift-ci please test

@bnbarham
Copy link
Contributor Author

bnbarham commented May 4, 2021

This will need to have some changes after Alex's PR

Copy link
Contributor

@hamishknight hamishknight 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!

@bnbarham bnbarham force-pushed the async-refactoring-mixed-optional branch from d1e3ace to 73d8914 Compare May 5, 2021 01:40
@bnbarham
Copy link
Contributor Author

bnbarham commented May 5, 2021

@swift-ci please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

I’ve got one small style suggestion.

Also, I think in one test case you are missing a try. Not sure if the try is not emitted or if you simply forgot to write it in the CHECK: line.

bnbarham added 2 commits May 6, 2021 10:27
Convert to async and add async alternative differ when there is no/an
invalid completion handler. In those cases, also check convert to async.
@bnbarham bnbarham force-pushed the async-refactoring-mixed-optional branch from 73d8914 to 398124c Compare May 6, 2021 00:30
@bnbarham
Copy link
Contributor Author

bnbarham commented May 6, 2021

@swift-ci please test

@bnbarham
Copy link
Contributor Author

bnbarham commented May 6, 2021

@hamishknight @ahoppen are you both fine with this one now?

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

🚢 it

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

LGTM!

@bnbarham bnbarham merged commit 8b3147c into swiftlang:main May 6, 2021
@bnbarham bnbarham deleted the async-refactoring-mixed-optional branch May 6, 2021 22:20
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.

3 participants