Merge pull request #36737 from slavapestov/nil-default-arg-rethrows

Sema: Teach rethrows checking about 'nil' default arguments

Author: slavapestov

CHANGELOG.md

@@ -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:

lib/Parse/ParsePattern.cpp

@@ -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;

lib/Sema/TypeCheckEffects.cpp

@@ -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);

test/SILGen/constrained_extensions.swift

@@ -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!
   }
 
@@ -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!
   }
 
@@ -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
     }
   }

test/SILGen/default_arguments.swift

@@ -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()

test/SILGen/default_arguments_inherited.swift

@@ -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> {}

test/SILGen/default_arguments_nil.swift

@@ -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()
+}

test/decl/func/rethrows.swift

@@ -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 }
+}