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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,24 @@ CHANGELOG
Swift 5.5
---------

* The determination of whether a call to a `rethrows` function can throw now considers default arguments of `Optional` type.

In Swift 5.4, such default arguments were ignored entirely by `rethrows` checking. This meant that the following example was accepted:

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

However, it also meant that the following was accepted, even though the call to `foo()` can throw and the call site is not marked with `try`:

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

The new behavior is that the first example is accepted because the default argument is syntactically written as `nil`, which is known not to throw. The second example is correctly rejected, on account of missing a `try` since the default argument *can* throw.

* [SE-0293][]:

Property wrappers can now be applied to function and closure parameters:
Expand Down
6 changes: 6 additions & 0 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ static DefaultArgumentKind getDefaultArgKind(Expr *init) {
if (!init)
return DefaultArgumentKind::None;

// Parse an as-written 'nil' expression as the special NilLiteral kind,
// which is emitted by the caller and can participate in rethrows
// checking.
if (isa<NilLiteralExpr>(init))
return DefaultArgumentKind::NilLiteral;

auto magic = dyn_cast<MagicIdentifierLiteralExpr>(init);
if (!magic)
return DefaultArgumentKind::Normal;
Expand Down
11 changes: 10 additions & 1 deletion lib/Sema/TypeCheckEffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,16 @@ class ApplyClassifier {
Classification classifyArgument(Expr *arg, Type paramType, EffectKind kind) {
arg = arg->getValueProvidingExpr();

if (isa<DefaultArgumentExpr>(arg)) {
if (auto *defaultArg = dyn_cast<DefaultArgumentExpr>(arg)) {
// Special-case a 'nil' default argument, which is known not to throw.
if (defaultArg->isCallerSide()) {
auto *callerSideArg = defaultArg->getCallerSideDefaultExpr();
if (isa<NilLiteralExpr>(callerSideArg)) {
if (callerSideArg->getType()->getOptionalObjectType())
return Classification();
}
}

return classifyArgumentByType(arg->getType(),
PotentialEffectReason::forDefaultClosure(),
kind);
Expand Down
6 changes: 3 additions & 3 deletions test/SILGen/constrained_extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ extension Array where Element == Int {

// CHECK-LABEL: sil non_abi [serialized] [ossa] @$sSa22constrained_extensionsSiRszlE12staticMethod1eS2iSg_tFZfA_ : $@convention(thin) () -> Optional<Int>
// CHECK-LABEL: sil [ossa] @$sSa22constrained_extensionsSiRszlE12staticMethod1eS2iSg_tFZ : $@convention(method) (Optional<Int>, @thin Array<Int>.Type) -> Int
public static func staticMethod(e: Element? = nil) -> Element {
public static func staticMethod(e: Element? = (nil)) -> Element {
return e!
}

Expand Down Expand Up @@ -104,7 +104,7 @@ extension Dictionary where Key == Int {
// CHECK-LABEL: sil non_abi [serialized] [ossa] @$sSD22constrained_extensionsSiRszrlE12staticMethod1k1vq_SiSg_q_SgtFZfA_ : $@convention(thin) <Key, Value where Key == Int> () -> Optional<Int>
// CHECK-LABEL: sil non_abi [serialized] [ossa] @$sSD22constrained_extensionsSiRszrlE12staticMethod1k1vq_SiSg_q_SgtFZfA0_ : $@convention(thin) <Key, Value where Key == Int> () -> @out Optional<Value>
// CHECK-LABEL: sil [ossa] @$sSD22constrained_extensionsSiRszrlE12staticMethod1k1vq_SiSg_q_SgtFZ : $@convention(method) <Key, Value where Key == Int> (Optional<Int>, @in_guaranteed Optional<Value>, @thin Dictionary<Int, Value>.Type) -> @out Value
public static func staticMethod(k: Key? = nil, v: Value? = nil) -> Value {
public static func staticMethod(k: Key? = (nil), v: Value? = (nil)) -> Value {
return v!
}

Expand Down Expand Up @@ -191,7 +191,7 @@ extension Array where Element == Int {

// CHECK-LABEL: sil hidden [ossa] @$sSa22constrained_extensionsSiRszlE6NestedV10hasDefault1eySiSg_tFfA_ : $@convention(thin) () -> Optional<Int>
// CHECK-LABEL: sil hidden [ossa] @$sSa22constrained_extensionsSiRszlE6NestedV10hasDefault1eySiSg_tF : $@convention(method) (Optional<Int>, @inout Array<Int>.Nested) -> ()
mutating func hasDefault(e: Element? = nil) {
mutating func hasDefault(e: Element? = (nil)) {
self.e = e
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/SILGen/default_arguments.swift
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func test_r18400194() {
// Don't add capture arguments to local default argument generators.
func localFunctionWithDefaultArg() {
var z = 5
func bar(_ x: Int? = nil) {
func bar(_ x: Int? = (nil)) {
z += 1
}
bar()
Expand Down
2 changes: 1 addition & 1 deletion test/SILGen/default_arguments_inherited.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// derived class generic signature.

class Puppy<T, U> {
init(t: T? = nil, u: U? = nil) {}
init(t: T? = (nil), u: U? = (nil)) {}
}

class Chipmunk : Puppy<Int, String> {}
Expand Down
12 changes: 12 additions & 0 deletions test/SILGen/default_arguments_nil.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// RUN: %target-swift-emit-silgen %s | %FileCheck %s

public func takesAnInt(_: Int? = nil) {}

// CHECK-LABEL: sil [ossa] @$s21default_arguments_nil15callsTakesAnIntyyF : $@convention(thin) () -> () {
// CHECK: [[NIL:%.*]] = enum $Optional<Int>, #Optional.none!enumelt
// CHECK: [[FN:%.*]] = function_ref @$s21default_arguments_nil10takesAnIntyySiSgF
// CHECK: apply [[FN]]([[NIL]]) : $@convention(thin) (Optional<Int>) -> ()
// CHECK: return
public func callsTakesAnInt() {
takesAnInt()
}
18 changes: 18 additions & 0 deletions test/decl/func/rethrows.swift
Original file line number Diff line number Diff line change
Expand Up @@ -628,3 +628,21 @@ func throwableDefaultRethrows(_ f: () throws -> () = { throw SomeError.Badness }
// This should always emit a diagnostic because we can statically know that default argument can throw.
throwableDefaultRethrows() // expected-error {{call can throw but is not marked with 'try'}}
// expected-note@-1 {{call is to 'rethrows' function, but a defaulted argument function can throw}}

// rdar://76169080 - rethrows -vs- Optional default arguments
func optionalRethrowsDefaultArg1(_: (() throws -> ())? = nil) rethrows {}

func callsOptionalRethrowsDefaultArg1() throws {
optionalRethrowsDefaultArg1()
optionalRethrowsDefaultArg1(nil)
try optionalRethrowsDefaultArg1 { throw SomeError.Badness }
}

func optionalRethrowsDefaultArg2(_: (() throws -> ())? = { throw SomeError.Badness }) rethrows {}

func callsOptionalRethrowsDefaultArg2() throws {
optionalRethrowsDefaultArg2() // expected-error {{call can throw but is not marked with 'try'}}
// expected-note@-1 {{call is to 'rethrows' function, but a defaulted argument function can throw}}
optionalRethrowsDefaultArg2(nil)
try optionalRethrowsDefaultArg2 { throw SomeError.Badness }
}