Skip to content

[CSClosure] Support empty return when closure result is optional Void #40475

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
Dec 10, 2021

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Dec 8, 2021

Source compatibility workaround.

func test<T>(_: () -> T?) {
  ...
}

A multi-statement closure passed to test that has an optional
Void result type inferred from the body allows:

  • empty return(s);
  • to skip return nil or return () at the end.

Implicit return () has to be inserted as the last element
of the body if there is none. This wasn't needed before SE-0326
because result type was (incorrectly) inferred as Void due to
the body being skipped.

Resolves: rdar://85840941

xedin added 2 commits December 8, 2021 11:58
…ication

It's possible that solution application either modifies or replaces the
`BraceStmt` that represents closure body, so we need to make sure that
body is always updated.
…oid`

Source compatibility workaround.

func test<T>(_: () -> T?) {
  ...
}

A multi-statement closure passed to `test` that has an optional
`Void` result type inferred from the body allows:
  - empty `return`(s);
  - to skip `return nil` or `return ()` at the end.

Implicit `return ()` has to be inserted as the last element
of the body if there is none. This wasn't needed before SE-0326
because result type was (incorrectly) inferred as `Void` due to
the body being skipped.

Resolves: rdar://85840941
@xedin xedin requested a review from hborla December 8, 2021 21:58
Copy link
Member

@hborla hborla left a comment

Choose a reason for hiding this comment

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

We should consider adding a warning for this, but otherwise LGTM

@xedin
Copy link
Contributor Author

xedin commented Dec 9, 2021

@swift-ci please smoke test

@xedin xedin merged commit 0e370db into swiftlang:main Dec 10, 2021
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