-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Refactoring] Fix invalid legacy body for Result<Void, ...> handler #38855
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
Conversation
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really should try and only use refactor-check-compiles
tests, which would have caught the issue…
Good point, I didn't even notice that we weren't. I assume the majority of these were added before it existed? I'll convert these now in a separate commit (and put the unrelated _ change with it). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
e3f99c5
to
3ab7862
Compare
When adding an async alternative for a function with a `(Result<Void, ...>) -> Void` completion handler, the legacy body was adding a call to the handler with `.success(result)`. We don't assign to `result` for `Void` handlers, so pass `()` instead. Resolves rdar://81829392
3ab7862
to
46a4b24
Compare
Updated all the tests in convert_function.swift to compile as well. Changed them all to -convert-to-async since none are checking the legacy body. Also noticed that we already had the |
Update the async refactoring tests in `convert_function.swift` to compile the refactored code (where possible) to check for errors. This would have prevented the issue with the refactored `Result<Void, ...`> handler producing erroneous code.
46a4b24
to
9fc758a
Compare
@swift-ci please test |
When adding an async alternative for a function with a
(Result<Void, ...>) -> Void
completion handler, the legacy body was adding a call tothe handler with
.success(result)
. We don't assign toresult
forVoid handlers, so pass
()
instead.Resolves rdar://81829392