Merge pull request #4031 from milseman/noescape_by_default

noescape by default: deprecate @NoEscape and @autoclosure(escaping)

Author: milseman

CHANGELOG.md

@@ -2,6 +2,14 @@ Note: This is in reverse chronological order, so newer entries are added to the
 
 Swift 3.0
 ---------
+
+* [SE-103](https://github.com/apple/swift-evolution/blob/master/proposals/0103-make-noescape-default.md)
+
+  Closure parameters are non-escaping by default, rather than explicitly being
+  annotated `@noescape`. Use `@escaping` to say that a closure parameter may
+  escape. `@autoclosure(escaping)` is now spelled `@autoclosure @escaping`.
+  `@noescape` and `@autoclosure(escaping)` are deprecated.
+
 * [SE-0115](https://github.com/apple/swift-evolution/blob/master/proposals/0115-literal-syntax-protocols.md)
 
   To clarify the role of `*LiteralConvertible` protocols, they have 

include/swift/AST/DiagnosticsParse.def

@@ -1215,6 +1215,12 @@ ERROR(attr_noescape_conflicts_escaping_autoclosure,none,
 ERROR(attr_noescape_implied_by_autoclosure,none,
       "@noescape is implied by @autoclosure and should not be "
       "redundantly specified", ())
+WARNING(attr_autoclosure_escaping_deprecated,none,
+        "@autoclosure(escaping) is deprecated; use @autoclosure @escaping instead",
+        ())
+WARNING(attr_noescape_deprecated,none,
+        "@noescape is the default and is deprecated",
+        ())
 
 // convention
 ERROR(convention_attribute_expected_lparen,none,

include/swift/AST/DiagnosticsSema.def

@@ -2239,7 +2239,7 @@ ERROR(closure_implicit_capture_mutating_self,none,
       ())
 ERROR(nested_function_with_implicit_capture_argument,none,
       "nested function with %select{an |}0implicitly captured inout "
-      "parameter%select{|s}0 can only be used as a @noescape argument", (bool))
+      "parameter%select{|s}0 can only be used as a non-escaping argument", (bool))
 ERROR(nested_function_escaping_inout_capture,none,
       "nested function cannot capture inout parameter and escape", ())
 

lib/AST/ASTDumper.cpp

@@ -2917,7 +2917,6 @@ namespace {
       printFlag(T->isAutoClosure(), "autoclosure");
 
       // Dump out either @noescape or @escaping
-      printFlag(T->isNoEscape(), "@noescape");
       printFlag(!T->isNoEscape(), "@escaping");
 
       printFlag(T->throws(), "throws");

lib/AST/ASTPrinter.cpp

@@ -3742,16 +3742,11 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
   void printFunctionExtInfo(AnyFunctionType::ExtInfo info) {
     if (Options.SkipAttributes)
       return;
-    if (info.isAutoClosure()) {
-      if (info.isNoEscape())
-        Printer << "@autoclosure ";
-      else
-        Printer << "@autoclosure(escaping) ";
-    } else if (inParameterPrinting) {
-      if (!info.isNoEscape()) {
-        Printer << "@escaping ";
-      }
-    }
+
+    if (info.isAutoClosure())
+      Printer << "@autoclosure ";
+    if (inParameterPrinting && !info.isNoEscape())
+      Printer << "@escaping ";
 
     if (Options.PrintFunctionRepresentationAttrs) {
       // TODO: coalesce into a single convention attribute.

lib/AST/TypeRepr.cpp

@@ -276,13 +276,9 @@ void AttributedTypeRepr::printAttrs(llvm::raw_ostream &OS) const {
 void AttributedTypeRepr::printAttrs(ASTPrinter &Printer) const {
   const TypeAttributes &Attrs = getAttrs();
 
-  switch (Attrs.has(TAK_autoclosure)*2 + Attrs.has(TAK_noescape)) {
-  case 0: break;  // Nothing specified.
-  case 1: Printer << "@noescape "; break;
-  case 2: Printer << "@autoclosure(escaping) "; break;
-  case 3: Printer << "@autoclosure "; break;
-  }
-  if (Attrs.has(TAK_escaping))     Printer << "@escaping ";
+  if (Attrs.has(TAK_autoclosure)) Printer << "@autoclosure ";
+  if (Attrs.has(TAK_escaping))    Printer << "@escaping ";
+
   if (Attrs.has(TAK_thin))         Printer << "@thin ";
   if (Attrs.has(TAK_thick))        Printer << "@thick ";
   if (Attrs.convention.hasValue()) {

lib/FrontendTool/FrontendTool.cpp

@@ -629,8 +629,11 @@ class JSONFixitWriter : public DiagnosticConsumer {
         Info.ID == diag::noescape_autoclosure.ID ||
         Info.ID == diag::where_inside_brackets.ID ||
         Info.ID == diag::selector_construction_suggest.ID ||
-        Info.ID == diag::selector_literal_deprecated_suggest.ID)
+        Info.ID == diag::selector_literal_deprecated_suggest.ID ||
+        Info.ID == diag::attr_noescape_deprecated.ID ||
+        Info.ID == diag::attr_autoclosure_escaping_deprecated.ID)
       return true;
+
     return false;
   }
 

lib/Parse/ParseDecl.cpp

@@ -1444,6 +1444,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
   SourceLoc Loc = consumeToken();
 
   bool isAutoclosureEscaping = false;
+  SourceLoc autoclosureEscapingRightParenLoc;
   StringRef conventionName;
 
   // Handle @autoclosure(escaping)
@@ -1463,7 +1464,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
     if (isAutoclosureEscaping) {
       consumeToken(tok::l_paren);
       consumeToken(tok::identifier);
-      consumeToken(tok::r_paren);
+      autoclosureEscapingRightParenLoc = consumeToken(tok::r_paren);
     }
   } else if (attr == TAK_convention) {
     SourceLoc LPLoc;
@@ -1512,27 +1513,35 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
     // Handle @autoclosure(escaping)
     if (isAutoclosureEscaping) {
       // @noescape @autoclosure(escaping) makes no sense.
-      if (Attributes.has(TAK_noescape))
+      if (Attributes.has(TAK_noescape)) {
         diagnose(Loc, diag::attr_noescape_conflicts_escaping_autoclosure);
-    } else {
-      if (Attributes.has(TAK_noescape))
-        diagnose(Loc, diag::attr_noescape_implied_by_autoclosure);
-      Attributes.setAttr(TAK_noescape, Loc);
+      } else {
+        diagnose(Loc, diag::attr_autoclosure_escaping_deprecated)
+            .fixItReplace({Loc, autoclosureEscapingRightParenLoc},
+                          "@autoclosure @escaping ");
+      }
+      Attributes.setAttr(TAK_escaping, Loc);
+    } else if (Attributes.has(TAK_noescape)) {
+      diagnose(Loc, diag::attr_noescape_implied_by_autoclosure);
     }
     break;
 
   case TAK_noescape:
-    // You can't specify @noescape and @autoclosure(escaping) together, and
-    // @noescape after @autoclosure is redundant.
-    if (Attributes.has(TAK_autoclosure)) {
-      diagnose(Loc, diag::attr_noescape_conflicts_escaping_autoclosure);
-      return false;
-    }
     // You can't specify @noescape and @escaping together.
     if (Attributes.has(TAK_escaping)) {
       diagnose(Loc, diag::attr_escaping_conflicts_noescape);
       return false;
     }
+
+    // @noescape after @autoclosure is redundant.
+    if (Attributes.has(TAK_autoclosure)) {
+      diagnose(Loc, diag::attr_noescape_implied_by_autoclosure);
+    }
+
+    // @noescape is deprecated and no longer used
+    diagnose(Loc, diag::attr_noescape_deprecated)
+      .fixItRemove({Attributes.AtLoc,Loc});
+
     break;
   case TAK_escaping:
     // You can't specify @noescape and @escaping together.

lib/Sema/TypeCheckAttr.cpp

@@ -1609,7 +1609,7 @@ void TypeChecker::checkAutoClosureAttr(ParamDecl *PD, AutoClosureAttr *attr) {
     return;
 
   // This decl attribute has been moved to being a type attribute.
-  auto text = attr->isEscaping() ? "@autoclosure(escaping) " : "@autoclosure ";
+  auto text = attr->isEscaping() ? "@autoclosure @escaping " : "@autoclosure ";
   diagnose(attr->getLocation(), diag::attr_decl_attr_now_on_type,
            "@autoclosure")
     .fixItRemove(attr->getRangeWithAt())

lib/Sema/TypeCheckType.cpp

@@ -1915,15 +1915,13 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
     }
 
     bool defaultNoEscape = false;
-    // TODO: Get rid of the need for checking autoclosure, by refactoring
-    // special autoclosure knowledge to just as "isEscaping" or similar.
-    if (isFunctionParam && !attrs.has(TAK_autoclosure)) {
-      // Closure params default to non-escaping
-      if (attrs.has(TAK_noescape)) {
-        // FIXME: diagnostic to tell user this is redundant and drop it
-      } else if (!attrs.has(TAK_escaping)) {
-        defaultNoEscape = isDefaultNoEscapeContext(DC);
-      }
+    if (isFunctionParam && !attrs.has(TAK_escaping)) {
+      defaultNoEscape = isDefaultNoEscapeContext(DC);
+    }
+
+    if (isFunctionParam && attrs.has(TAK_noescape) &&
+        isDefaultNoEscapeContext(DC)) {
+      // FIXME: diagnostic to tell user this is redundant and drop it
     }
 
     // Resolve the function type directly with these attributes.

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift.gyb

@@ -60,7 +60,7 @@ public func withInvalidOrderings(_ body: (@escaping (Int, Int) -> Bool) -> Void)
 
 internal func _mapInPlace<C : MutableCollection>(
   _ elements: inout C,
-  _ transform: @noescape (C.Iterator.Element) -> C.Iterator.Element
+  _ transform: (C.Iterator.Element) -> C.Iterator.Element
 ) where C.Indices.Iterator.Element == C.Index {
   for i in elements.indices {
     elements[i] = transform(elements[i])

stdlib/private/StdlibCollectionUnittest/LoggingWrappers.swift.gyb

@@ -224,28 +224,28 @@ public struct ${Self}<
   }
 
   public func map<T>(
-    _ transform: @noescape (Base.Iterator.Element) throws -> T
+    _ transform: (Base.Iterator.Element) throws -> T
   ) rethrows -> [T] {
     Log.map[selfType] += 1
     return try base.map(transform)
   }
 
   public func filter(
-    _ isIncluded: @noescape (Base.Iterator.Element) throws -> Bool
+    _ isIncluded: (Base.Iterator.Element) throws -> Bool
   ) rethrows -> [Base.Iterator.Element] {
     Log.filter[selfType] += 1
     return try base.filter(isIncluded)
   }
 
   public func forEach(
-    _ body: @noescape (Base.Iterator.Element) throws -> Void
+    _ body: (Base.Iterator.Element) throws -> Void
   ) rethrows {
     Log.forEach[selfType] += 1
     try base.forEach(body)
   }
 
   public func first(
-    where predicate: @noescape (Base.Iterator.Element) throws -> Bool
+    where predicate: (Base.Iterator.Element) throws -> Bool
   ) rethrows -> Base.Iterator.Element? {
     Log.first[selfType] += 1
     return try base.first(where: predicate)
@@ -276,7 +276,7 @@ public struct ${Self}<
   public func split(
     maxSplits: Int = Int.max,
     omittingEmptySubsequences: Bool = true,
-    whereSeparator isSeparator: @noescape (Base.Iterator.Element) throws -> Bool
+    whereSeparator isSeparator: (Base.Iterator.Element) throws -> Bool
   ) rethrows -> [SubSequence] {
     Log.split[selfType] += 1
     return try base.split(
@@ -296,7 +296,7 @@ public struct ${Self}<
   /// `preprocess` on `self` and return its result.  Otherwise, return
   /// `nil`.
   public func _preprocessingPass<R>(
-    _ preprocess: @noescape () throws -> R
+    _ preprocess: () throws -> R
   ) rethrows -> R? {
     Log._preprocessingPass[selfType] += 1
     return try base._preprocessingPass(preprocess)
@@ -443,14 +443,14 @@ public struct ${Self}<
 
 %     if Kind == 'MutableCollection':
   public mutating func partition(
-    by belongsInSecondPartition: @noescape (Iterator.Element) throws -> Bool
+    by belongsInSecondPartition: (Iterator.Element) throws -> Bool
   ) rethrows -> Index {
     Log.partitionBy[selfType] += 1
     return try base.partition(by: belongsInSecondPartition)
   }
 
   public mutating func _withUnsafeMutableBufferPointerIfSupported<R>(
-    _ body: @noescape (UnsafeMutablePointer<Iterator.Element>, Int) throws -> R
+    _ body: (UnsafeMutablePointer<Iterator.Element>, Int) throws -> R
   ) rethrows -> R? {
     Log._withUnsafeMutableBufferPointerIfSupported[selfType] += 1
     let result = try base._withUnsafeMutableBufferPointerIfSupported(body)
@@ -650,7 +650,7 @@ public struct ${Self}<
   }
 
   public mutating func _withUnsafeMutableBufferPointerIfSupported<R>(
-    _ body: @noescape (UnsafeMutablePointer<Iterator.Element>, Int) throws -> R
+    _ body: (UnsafeMutablePointer<Iterator.Element>, Int) throws -> R
   ) rethrows -> R? {
     Log._withUnsafeMutableBufferPointerIfSupported[selfType] += 1
     let result = try base._withUnsafeMutableBufferPointerIfSupported(body)

stdlib/private/StdlibUnittest/RaceTest.swift

@@ -47,7 +47,7 @@ import Glibc
 #if _runtime(_ObjC)
 import ObjectiveC
 #else
-func autoreleasepool(invoking code: @noescape () -> Void) {
+func autoreleasepool(invoking code: () -> Void) {
   // Native runtime does not have autorelease pools.  Execute the code
   // directly.
   code()

stdlib/private/StdlibUnittestFoundationExtras/StdlibUnittestFoundationExtras.swift

@@ -24,7 +24,7 @@ extension NSLocale {
 
 public func withOverriddenLocaleCurrentLocale<Result>(
   _ temporaryLocale: NSLocale,
-  _ body: @noescape () -> Result
+  _ body: () -> Result
 ) -> Result {
   let oldMethod = class_getClassMethod(
     NSLocale.self, #selector(getter: NSLocale.current))
@@ -48,7 +48,7 @@ public func withOverriddenLocaleCurrentLocale<Result>(
 
 public func withOverriddenLocaleCurrentLocale<Result>(
   _ temporaryLocaleIdentifier: String,
-  _ body: @noescape () -> Result
+  _ body: () -> Result
 ) -> Result {
   precondition(
     NSLocale.availableLocaleIdentifiers.contains(temporaryLocaleIdentifier),
@@ -65,7 +65,7 @@ public func withOverriddenLocaleCurrentLocale<Result>(
 /// return-autoreleased optimization.)
 @inline(never)
 public func autoreleasepoolIfUnoptimizedReturnAutoreleased(
-  invoking body: @noescape () -> Void
+  invoking body: () -> Void
 ) {
 #if arch(i386) && (os(iOS) || os(watchOS))
   autoreleasepool(invoking: body)

stdlib/public/SDK/Dispatch/Block.swift

@@ -43,9 +43,9 @@ public class DispatchWorkItem {
 			qos.qosClass.rawValue.rawValue, Int32(qos.relativePriority), block)
 	}
 
-	// Used by DispatchQueue.synchronously<T> to provide a @noescape path through
+	// Used by DispatchQueue.synchronously<T> to provide a path through
 	// dispatch_block_t, as we know the lifetime of the block in question.
-	internal init(flags: DispatchWorkItemFlags = [], noescapeBlock: @noescape () -> ()) {
+	internal init(flags: DispatchWorkItemFlags = [], noescapeBlock: () -> ()) {
 		_block = _swift_dispatch_block_create_noescape(flags.rawValue, noescapeBlock)
 	}
 

stdlib/public/SDK/Dispatch/Data.swift

@@ -70,7 +70,7 @@ public struct DispatchData : RandomAccessCollection, _ObjectiveCBridgeable {
 	}
 
 	public func withUnsafeBytes<Result, ContentType>(
-		body: @noescape (UnsafePointer<ContentType>) throws -> Result) rethrows -> Result
+		body: (UnsafePointer<ContentType>) throws -> Result) rethrows -> Result
 	{
 		var ptr: UnsafeRawPointer? = nil
 		var size = 0
@@ -82,7 +82,7 @@ public struct DispatchData : RandomAccessCollection, _ObjectiveCBridgeable {
 	}
 
 	public func enumerateBytes(
-		block: @noescape (_ buffer: UnsafeBufferPointer<UInt8>, _ byteIndex: Int, _ stop: inout Bool) -> Void) 
+		block: (_ buffer: UnsafeBufferPointer<UInt8>, _ byteIndex: Int, _ stop: inout Bool) -> Void) 
 	{
 		_swift_dispatch_data_apply(__wrapped) { (_, offset: Int, ptr: UnsafeRawPointer, size: Int) in
             let bytePtr = ptr.bindMemory(to: UInt8.self, capacity: size)

stdlib/public/SDK/Dispatch/Private.swift

@@ -139,7 +139,7 @@ public func dispatch_io_set_interval(_ channel: DispatchIO, _ interval: UInt64,
 }
 
 @available(*, unavailable, renamed:"DispatchQueue.apply(attributes:iterations:execute:)")
-public func dispatch_apply(_ iterations: Int, _ queue: DispatchQueue, _ block: @noescape (Int) -> Void) 
+public func dispatch_apply(_ iterations: Int, _ queue: DispatchQueue, _ block: (Int) -> Void) 
 {
 	fatalError()
 }
@@ -205,7 +205,7 @@ public func dispatch_barrier_async(_ queue: DispatchQueue, _ block: () -> Void)
 }
 
 @available(*, unavailable, renamed:"DispatchQueue.sync(self:flags:execute:)")
-public func dispatch_barrier_sync(_ queue: DispatchQueue, _ block: @noescape () -> Void)
+public func dispatch_barrier_sync(_ queue: DispatchQueue, _ block: () -> Void)
 {
 	fatalError()
 }