[SR-11711] Single-expression returns for #if declarations within closures

Author: maustinstar

lib/Parse/ParseExpr.cpp

@@ -2839,39 +2839,60 @@ ParserResult<Expr> Parser::parseExprClosure() {
     closure->setParameterList(params);
     closure->setHasAnonymousClosureVars();
   }
-
+  
   // If the body consists of a single expression, turn it into a return
   // statement.
   bool hasSingleExpressionBody = false;
-  if (!missingRBrace && bodyElements.size() == 1) {
-    // If the closure's only body element is a single return statement,
-    // use that instead of creating a new wrapping return expression.
-    Expr *returnExpr = nullptr;
+  if (!missingRBrace && bodyElements.size()) {
+    auto Element = bodyElements.front();
 
-    if (bodyElements[0].is<Stmt *>()) {
-      if (auto returnStmt =
-                  dyn_cast<ReturnStmt>(bodyElements[0].get<Stmt*>())) {
-        
-        if (!returnStmt->hasResult()) {
-          
-          returnExpr = TupleExpr::createEmpty(Context,
-                                              SourceLoc(),
-                                              SourceLoc(),
-                                              /*implicit*/true);
-          
-          returnStmt->setResult(returnExpr);
+    // If the body consists of an #if declaration with a single
+    // expression active clause, turn it into a return statement.
+    bool ReturnsLastElement = false;
+    if (bodyElements.size() == 2) {
+      if (auto *D = Element.dyn_cast<Decl *>()) {
+        // Step into nested active clause.
+        while (auto *ICD = dyn_cast<IfConfigDecl>(D)) {
+          auto ACE = ICD->getActiveClauseElements();
+          if (ACE.size() == 1) {
+            ReturnsLastElement = true;
+            Element = bodyElements.back();
+            break;
+          } else if (ACE.size() == 2) {
+            if (auto *ND = ACE.front().dyn_cast<Decl *>()) {
+              D = ND;
+              continue;
+            }
+          }
+          break;
         }
-        
-        hasSingleExpressionBody = true;
       }
     }
 
-    // Otherwise, create the wrapping return.
-    if (bodyElements[0].is<Expr *>()) {
-      hasSingleExpressionBody = true;
-      returnExpr = bodyElements[0].get<Expr*>();
-      bodyElements[0] = new (Context) ReturnStmt(SourceLoc(),
-                                                 returnExpr);
+    if (ReturnsLastElement || bodyElements.size() == 1) {
+      // If the closure's only body element is a single return statement,
+      // use that instead of creating a new wrapping return expression.
+      Expr *returnExpr = nullptr;
+      
+      if (Element.is<Stmt *>()) {
+        if (auto returnStmt = dyn_cast<ReturnStmt>(Element.get<Stmt *>())) {
+          if (!returnStmt->hasResult()) {
+            returnExpr = TupleExpr::createEmpty(Context,
+                                                SourceLoc(),
+                                                SourceLoc(),
+                                                /*implicit*/true);
+            returnStmt->setResult(returnExpr);
+          }
+          hasSingleExpressionBody = true;
+        }
+      }
+      
+      // Otherwise, create the wrapping return.
+      if (Element.is<Expr *>()) {
+        hasSingleExpressionBody = true;
+        returnExpr = Element.get<Expr*>();
+        bodyElements.back() = new (Context) ReturnStmt(SourceLoc(), returnExpr);
+      }
     }
   }
 

lib/Sema/CSClosure.cpp

@@ -173,7 +173,11 @@ class ClosureConstraintApplication
   }
 
   void visitDecl(Decl *decl) {
-    llvm_unreachable("Declarations not supported");
+    
+    if (isa<IfConfigDecl>(decl))
+      return;
+    
+    llvm_unreachable("Unimplemented case for closure body");
   }
 
   ASTNode visitBraceStmt(BraceStmt *braceStmt) {

test/FixCode/fixits-omit-return.swift

@@ -12,14 +12,14 @@ let cl_fixit_addreturn: () -> String = {
 
 func ff_fixit_addreturn_ifdecl() -> String {
     #if true
-    print("entering ff_fixit_addreturn()")
+    print("entering ff_fixit_addreturn_ifdecl()")
     "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a function expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
     #endif
 }
 
 let cl_fixit_addreturn_ifdecl: () -> String = {
     #if true
-    print("entering cl_fixit_addreturn()")
+    print("entering cl_fixit_addreturn_ifdecl()")
     "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a closure expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
     #endif
 }

test/expr/closure/single_expr_ifdecl.swift

@@ -0,0 +1,235 @@
+// RUN: %target-typecheck-verify-swift
+
+func takeIntToInt(_ f: (Int) -> Int) { }
+func takeIntIntToInt(_ f: (Int, Int) -> Int) { }
+
+// Simple closures with anonymous arguments
+func simple() {
+  takeIntToInt({
+    #if true
+    $0 + 1
+    #else 
+    $0 + 2
+    #endif
+  })
+  takeIntIntToInt({
+    #if true
+    $0 + $1 + 1
+    #else 
+    $0 + $1 + 2
+    #endif
+  })
+}
+
+// Anonymous arguments with inference
+func myMap<T, U>(_ array: [T], _ f: (T) -> U) -> [U] {}
+
+func testMap(_ array: [Int]) {
+  var farray = myMap(array, {
+    #if true
+    Float($0)
+    #else
+    Float($0 + 1)
+    #endif
+  })
+  var _ : Float = farray[0]
+  let farray2 = myMap(array, { (x : Int) in
+    #if true
+    Float(x)
+    #else
+    Float(x + 1)
+    #endif
+  })
+  farray = farray2
+  _ = farray
+}
+
+// Nested single-expression closures -- <rdar://problem/20931915>
+class NestedSingleExpr {
+  private var b: Bool = false
+  private func callClosureA(_ callback: () -> Void) {}
+  private func callClosureB(_ callback: () -> Void) {}
+
+  func call() {
+    callClosureA { [weak self] in
+      #if true
+      self?.callClosureA {
+        #if true
+        self?.b = true
+        #else
+        self?.b = false
+        #endif
+      }
+      #else
+      self?.callClosureB {
+        #if true
+        self?.b = true
+        #else
+        self?.b = false
+        #endif
+      }
+      #endif
+    }
+  }
+}
+
+// Autoclosure nested inside single-expr closure should get discriminator
+// <rdar://problem/22441425> Swift compiler "INTERNAL ERROR: this diagnostic should not be produced"
+struct Expectation<T> {}
+func expect<T>(_ expression: @autoclosure () -> T) -> Expectation<T> {
+  return Expectation<T>()
+}
+func describe(_ closure: () -> ()) {}
+func f() {
+  #if true
+  describe {
+    #if false
+    _ = expect("this")
+    #else
+    _ = expect("what")
+    #endif
+  }
+  #endif
+}
+
+struct Blob {}
+
+func withBlob(block: (Blob) -> ()) {}
+
+protocol Binding {}
+extension Int: Binding {}
+extension Double: Binding {}
+extension String: Binding {}
+extension Blob: Binding {}
+
+struct Stmt {
+  @discardableResult
+  func bind(_ values: Binding?...) -> Stmt {
+    return self
+  }
+
+  @discardableResult
+  func bind(_ values: [Binding?]) -> Stmt {
+    return self
+  }
+
+  @discardableResult
+  func bind(_ values: [String: Binding?]) -> Stmt {
+    return self
+  }
+}
+
+let stmt = Stmt()
+withBlob {
+  #if true
+  stmt.bind(1, 2.0, "3", $0)
+  #endif
+}
+withBlob {
+  #if true
+  stmt.bind([1, 2.0, "3", $0])
+  #endif
+}
+withBlob {
+  #if true
+  stmt.bind(["1": 1, "2": 2.0, "3": "3", "4": $0])
+  #endif
+}
+
+// <rdar://problem/19840785>
+// We shouldn't crash on the call to 'a.dispatch' below.
+class A {
+  func dispatch(_ f : () -> Void) {
+    f()
+  }
+}
+
+class C {
+	var prop = 0
+	var a = A()
+
+	func act() {
+    a.dispatch({() -> Void in
+      #if true
+      self.prop // expected-warning {{expression of type 'Int' is unused}}
+      #endif
+    })
+  }
+}
+
+// Never-returning expressions
+func haltAndCatchFire() -> Never {
+  #if true
+  while true { }
+  #else
+  while false { }
+  #endif
+}
+let backupPlan: () -> Int = {
+  #if true
+  haltAndCatchFire()
+  #endif
+}
+func missionCritical(storage: () -> String) {}
+missionCritical(storage: {
+  #if true
+  haltAndCatchFire()
+  #endif
+})
+
+// <https://bugs.swift.org/browse/SR-4963>
+enum E { }
+func takesAnotherUninhabitedType(e: () -> E) {}
+takesAnotherUninhabitedType {
+  #if true
+  haltAndCatchFire()
+  #endif
+}
+
+// Weak capture bug caught by rdar://problem/67351438
+class Y {
+  var toggle: Bool = false
+
+  func doSomething(animated: Bool, completionHandler: (Int, Int) -> Void) { }
+}
+
+class X {
+  private(set) var someY: Y!
+
+  func doSomething() {
+    someY?.doSomething(animated: true, completionHandler: { [weak someY] _, _ in
+      #if true
+      someY?.toggle = true
+      #else
+      someY?.toggle = false
+      #endif
+    })
+  }
+}
+
+var intOrStringClosure_true = {
+  #if true
+  42
+  #else
+  "foo"
+  #endif
+}
+
+var intOrStringClosure_false = {
+  #if false
+  42
+  #else
+  "foo"
+  #endif
+}
+
+func testMultiType() {
+  
+  let a = intOrStringClosure_true()
+  _ = a as Int
+  _ = a as String // expected-error {{cannot convert value of type 'Int' to type 'String'}}
+  
+  let b = intOrStringClosure_false()
+  _ = b as Int // expected-error {{cannot convert value of type 'String' to type 'Int'}}
+  _ = b as String
+}