Skip to content

[5.5][Refactoring] Support refactoring calls to async if a variable or function is used as completion handler #37381

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

ahoppen
Copy link
Member

@ahoppen ahoppen commented May 12, 2021

Cherry-pick of #37314 into release/5.5

  • Explanation: Async refactoring previously failed if we were using a variable as a completion handler. For example, we couldn’t refactor wrapperAroundSimple to async.
func simple(completion: (String) -> Void) {}
func simple() async -> String {}

func wrapperAroundSimple(completionHandler: (String) -> Void) {
  simple(completion: completionHandler)
}

Now we generate

func wrapperAroundSimple() async {
  return await simple()
}

Similarly, we previously failed if a function was used as completion handler

func printResultOfSimple() {
  simple(completion: print)
}

We now generate

func printResultOfSimple() async {
  let result = await simple()
  print(result)
}
  • Scope: Async refactoring (add async alternative, convert to async, convert call to async)
  • Risk: Low (this only affects async refactoring)
  • Testing: Added new test cases to the refactoring test suite
  • Issue: rdar://77460524
  • Reviewer: Ben Barham (@bnbarham)

ahoppen added 7 commits May 12, 2021 09:56
This will later allow us to reuse parts of `LegacyAlternativeBodyCreator` from `AsyncConverter` when refactoring calls to an async alternative if they pass a variable as the completion handler.
… possible

The code moved from `LegacyAlternativeBodyCreator` was using constant strings a lot. Use `tok::` instead to match the style of `AsyncConverter`.
…ction is used as completion handler

Previously, we only supported  refactoring a function to call the async alternative if a closure was used for the callback parameter. With this change, we also support calling a arbitrary function (or variable with function type) that is passed to the completion handler argument.

The implementation basically re-uses the code we already have to create the legacy function’s body (which calls the newly created async version and then forwards the arguments to the legacy completion handler).

To describe the completion handler that the result is being forwarded to, I’m also using `AsyncHandlerDesc`, but since the completion handler may be a variable, it doesn’t necessarily have an `Index` within a function decl that declares it. Because of this, I split the `AsyncHandlerDesc` up into a context-free `AsyncHandlerDesc` (without an `Index`) and `AsyncHandlerParamDesc` (which includes the `Index`). It turns out that `AsyncHandlerDesc` is sufficient in most places.

Resolves rdar://77460524
…eter is not optional

If the parameter is not an `Optional`, add a placeholder instead that the user can fill with a sensible default value.
When a function’s completion handler is being passed to another function and the function that the completion handler is being declared in is converted to async, replace the call to the completion handler by a `return` statement.

For example:
```swift
func foo(completion: (String) -> Void) {
  bar(completion)
}
```

becomes
```swift
func foo() async -> String {
  return await bar()
}
```

Previously, we were calling the completion handler, which no longer exists in the newly created `async` function.
…ecks in `variable_as_callback.swift`

These were a legacy from when the tests lived in `basic.swift`. They are no longer needed.
@ahoppen ahoppen added the r5.5 label May 12, 2021
@ahoppen ahoppen requested a review from a team as a code owner May 12, 2021 08:04
@ahoppen
Copy link
Member Author

ahoppen commented May 12, 2021

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented May 12, 2021

@swift-ci Please nominate

@airspeedswift airspeedswift merged commit 9fc3e6a into swiftlang:release/5.5 May 12, 2021
@ahoppen ahoppen deleted the pr-5.5/refactor-to-async-variable-callback branch May 17, 2021 17:09
@AnthonyLatsis AnthonyLatsis added swift 5.5 🍒 release cherry pick Flag: Release branch cherry picks labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants