Skip to content

Sema: Teach rethrows checking about 'nil' default arguments #36737

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 3 commits into from
Apr 4, 2021

Conversation

slavapestov
Copy link
Contributor

In Swift 5.4, this worked:

func foo(_: (() throws -> ())? = nil) rethrows {}
foo()  // no 'try' needed

However, this was an accident, because this was also accepted:

func foo(_: (() throws -> ())? = { throw ... }) rethrows {}
foo()  // 'try' *should* be required here

This got fixed at some point recently, but since people rely on the
old case working for 'nil', let's add back a check for a 'nil'
default parameter.

Fixes rdar://problem/76169080.

@slavapestov slavapestov requested a review from hborla April 3, 2021 03:44
…d::NilLiteral

We support a special DefaultArgumentKind::NilLiteral, but it was only
used for synthesized and imported default arguments as well.

Let's also use it for parsed default arguments that are exactly 'nil',
so that rethrows checking can special-case them.
In Swift 5.4, this worked:

  func foo(_: (() throws -> ())? = nil) rethrows {}
  foo()  // no 'try' needed

However, this was an accident, because this was also accepted:

  func foo(_: (() throws -> ())? = { throw ... }) rethrows {}
  foo()  // 'try' *should* be required here

This got fixed at some point recently, but since people rely on the
old case working for 'nil', let's add back a check for a 'nil'
default parameter.

Fixes rdar://problem/76169080.
@slavapestov slavapestov force-pushed the nil-default-arg-rethrows branch from 2af1a5f to a06bd00 Compare April 3, 2021 03:57
@slavapestov slavapestov force-pushed the nil-default-arg-rethrows branch from a06bd00 to ef2995b Compare April 3, 2021 03:59
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@slavapestov
Copy link
Contributor Author

swiftlang/sourcekit-lsp#384
@swift-ci Please smoke test macOS

@slavapestov slavapestov merged commit 228112f into swiftlang:main Apr 4, 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