Skip to content

Sema: Implicit conversion for single-expression closures of Never type (3.0) #4951

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

slavapestov
Copy link
Contributor

  • Description: When we moved from the @NoReturn attribute to the Never return type, we lost a bit of functionality. Since Never is just an ordinary type, the type checker did not allow a single-expression closure to contain a Never-returning expression unless the closure type itself returned Never. This precludes using single expression closures for callbacks in certain situations. This patch adds a new rule, similar to the existing rule for dropping the return type of a single-expression closure.
  • Scope of the issue: This has been affecting people in the wild while migrating code from Swift 2 to Swift 3.
  • Risk: Low. The change has a bit of churn to repurpose the Void-conversion code and removes a flag; a careful reading should show the changes are equivalent.
  • Tested: New test added.
  • Radar: rdar://problem/28269358
  • Reviewed by: @DougGregor

@slavapestov slavapestov added this to the Swift 3.0 milestone Sep 23, 2016
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov slavapestov changed the title Sema: Implicit conversion for single-expression closures of Never type Sema: Implicit conversion for single-expression closures of Never type (3.0) Sep 23, 2016
@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - e680e4dd851bb00194c97641ba448f1c5f4a0e5c
Test requested by - @slavapestov

This fixes a usability regression with the removal of @NoReturn
in Swift 3. Previously, it was legal to write this:

let callback: () -> Int = { fatalError() }

Now that the special @NoReturn attribute has been replaced with
a Never type, the above fails to typecheck, because the expression
now has type 'Never', and we expect a value of type 'Int'.

Getting around this behavior requires ugly workarounds to force the
parser to treat the body as a statement rather than an expression;
for example,

let callback: () -> Int = { _ = (); fatalError() }

This patch generalized single-expression closures to allow
the 'Never to T' conversion. Note that this is rather narrow
in scope -- it only applies to closure *literals*, single-expression
ones at that, not arbitrary function *values*.

In fact, it is not really a conversion at all, but more of a
desugaring rule for single-expression closures. They can now be
summarized as follows:

- If the closure literal has contextual return type T and
  the expression has Never type, the closure desugars as
  { _ = <expr> }, with no ReturnStmt.

- If the closure literal has contextual return type T for some
  non-void type T, the closure desugars as { return <expr> };
  the expression type must be convertible to T.

- If the closure literal has contextual return type Void, and
  the expression has some non-Void type T, the closure
  desugars as { _ = <expr>; return () }.

Fixes <rdar://problem/28269358> and <https://bugs.swift.org/browse/SR-2661>.
@slavapestov slavapestov force-pushed the never-returning-closure-usability-3.0 branch from e680e4d to 3b137b2 Compare September 23, 2016 07:31
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

SmallVector<ASTNode, 1> elements;
elements.push_back(singleExpr);

auto braceStmt = BraceStmt::create(tc.Context,
Copy link
Member

Choose a reason for hiding this comment

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

This kind of AST hackery feels gross, but... okay. I don't see a better localized fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just copy and pasted from the old code (and I know, that's a HORRIBLE excuse)

@tkremenek tkremenek merged commit 75355c4 into swiftlang:swift-3.0-branch Sep 23, 2016
MaxDesiatov pushed a commit that referenced this pull request Sep 7, 2023
[pull] swiftwasm from main
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.

4 participants