Merge pull request #3853 from milseman/noescape_by_default

SE-0103 Noescape by default

Author: DougGregor

benchmark/single-source/Integrate.swift

@@ -20,7 +20,7 @@ class Integrate {
 
   let fun: (Double) -> Double
 
-  init (f: (Double) -> Double) {
+  init (f: @escaping (Double) -> Double) {
     fun = f
   }
 

benchmark/utils/DriverUtils.swift

@@ -50,7 +50,7 @@ struct Test {
   let index: Int
   let f: (Int) -> ()
   var run: Bool
-  init(name: String, n: Int, f: (Int) -> ()) {
+  init(name: String, n: Int, f: @escaping (Int) -> ()) {
     self.name = name
     self.index = n
     self.f = f

include/swift/AST/DiagnosticsSema.def

@@ -1821,6 +1821,8 @@ ERROR(autoclosure_function_type,none,
       ())
 ERROR(autoclosure_function_input_nonunit,none,
       "autoclosure argument type must be '()'", ())
+
+// FIXME: drop these when we drop @noescape
 ERROR(noescape_function_type,none,
       "@noescape may only be applied to parameters of function type",
       ())
@@ -1829,6 +1831,7 @@ ERROR(noescape_implied_by_autoclosure,none,
       "redundantly specified", ())
 ERROR(noescape_conflicts_escaping_autoclosure,none,
       "@noescape conflicts with @autoclosure(escaping)", ())
+
 ERROR(escaping_function_type,none,
       "@escaping may only be applied to parameters of function type", ())
 
@@ -2271,17 +2274,20 @@ WARNING(optional_pattern_match_promotion,none,
 ERROR(type_of_metatype,none,
       "'.dynamicType' is not allowed after a type name", ())
 ERROR(invalid_noescape_use,none,
-      "@noescape %select{value|parameter}1 %0 may only be called",
+      "non-escaping %select{value|parameter}1 %0 may only be called",
       (Identifier, bool))
 NOTE(noescape_autoclosure,none,
-    "parameter %0 is implicitly @noescape because it was declared @autoclosure",
+    "parameter %0 is implicitly non-escaping because it was declared @autoclosure",
+     (Identifier))
+NOTE(noescape_parameter,none,
+    "parameter %0 is implicitly non-escaping",
      (Identifier))
 
 ERROR(closure_noescape_use,none,
-      "closure use of @noescape parameter %0 may allow it to escape",
+      "closure use of non-escaping parameter %0 may allow it to escape",
       (Identifier))
 ERROR(decl_closure_noescape_use,none,
-      "declaration closing over @noescape parameter %0 may allow it to escape",
+      "declaration closing over non-escaping parameter %0 may allow it to escape",
       (Identifier))
 
 ERROR(capture_across_type_decl,none,

include/swift/AST/PrintOptions.h

@@ -261,6 +261,12 @@ struct PrintOptions {
   /// Whether we are printing part of SIL body.
   bool PrintInSILBody = false;
 
+  /// Whether to print the types as if they appear as function parameters. This
+  /// governs whether we print a function type with an explicit @escaping. This
+  /// is also set and restored internally when visiting a type in a parameter
+  /// position.
+  bool PrintAsInParamType = false;
+
   /// Whether to use an empty line to separate two members in a single decl.
   bool EmptyLineBetweenMembers = false;
 

include/swift/AST/Types.h

@@ -2132,7 +2132,6 @@ class AnyFunctionType : public TypeBase {
     enum : uint16_t { AutoClosureMask        = 0x010 };
     enum : uint16_t { NoEscapeMask           = 0x020 };
     enum : uint16_t { ThrowsMask             = 0x040 };
-    enum : uint16_t { ExplicitlyEscapingMask = 0x080 };
 
     uint16_t Bits;
 
@@ -2153,17 +2152,15 @@ class AnyFunctionType : public TypeBase {
 
     // Constructor with no defaults.
     ExtInfo(Representation Rep,
-            bool IsAutoClosure, bool IsNoEscape, bool IsExplicitlyEscaping,
+            bool IsAutoClosure, bool IsNoEscape,
             bool Throws)
       : ExtInfo(Rep, Throws) {
       Bits |= (IsAutoClosure ? AutoClosureMask : 0);
       Bits |= (IsNoEscape ? NoEscapeMask : 0);
-      Bits |= (IsExplicitlyEscaping ? ExplicitlyEscapingMask : 0);
     }
 
     bool isAutoClosure() const { return Bits & AutoClosureMask; }
     bool isNoEscape() const { return Bits & NoEscapeMask; }
-    bool isExplicitlyEscaping() const { return Bits & ExplicitlyEscapingMask; }
     bool throws() const { return Bits & ThrowsMask; }
     Representation getRepresentation() const {
       unsigned rawRep = Bits & RepresentationMask;
@@ -2287,12 +2284,6 @@ class AnyFunctionType : public TypeBase {
     return getExtInfo().isNoEscape();
   }
 
-  /// \brief True if the parameter declaration it is attached to has explicitly
-  /// been marked with the @escaping attribute. This is a temporary measure.
-  bool isExplicitlyEscaping() const {
-    return getExtInfo().isExplicitlyEscaping();
-  }
-  
   bool throws() const {
     return getExtInfo().throws();
   }

include/swift/Serialization/ModuleFormat.h

@@ -53,7 +53,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// in source control, you should also update the comment to briefly
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
-const uint16_t VERSION_MINOR = 258; // Last change: precedencegroup
+const uint16_t VERSION_MINOR = 259; // Last change: drop explicitlyEscaping
 
 using DeclID = PointerEmbeddedInt<unsigned, 31>;
 using DeclIDField = BCFixed<31>;
@@ -590,7 +590,6 @@ namespace decls_block {
     FunctionTypeRepresentationField, // representation
     BCFixed<1>,  // auto-closure?
     BCFixed<1>,  // noescape?
-    BCFixed<1>,  // explicitlyEscaping?
     BCFixed<1>   // throws?
   >;
 

lib/AST/ASTDumper.cpp

@@ -2916,8 +2916,11 @@ namespace {
       }
 
       printFlag(T->isAutoClosure(), "autoclosure");
-      printFlag(T->isNoEscape(), "noescape");
-      printFlag(T->isExplicitlyEscaping(), "escaping");
+
+      // Dump out either @noescape or @escaping
+      printFlag(T->isNoEscape(), "@noescape");
+      printFlag(!T->isNoEscape(), "@escaping");
+
       printFlag(T->throws(), "throws");
 
       printRec("input", T->getInput());

lib/AST/ASTPrinter.cpp

@@ -2535,7 +2535,11 @@ void PrintAST::visitVarDecl(VarDecl *decl) {
 }
 
 void PrintAST::visitParamDecl(ParamDecl *decl) {
-  return visitVarDecl(decl);
+  // Set and restore in-parameter-position printing of types
+  auto prior = Options.PrintAsInParamType;
+  Options.PrintAsInParamType = true;
+  visitVarDecl(decl);
+  Options.PrintAsInParamType = prior;
 }
 
 void PrintAST::printOneParameter(const ParamDecl *param, bool Curried,
@@ -2589,7 +2593,11 @@ void PrintAST::printOneParameter(const ParamDecl *param, bool Curried,
       TheTypeLoc.setType(BGT->getGenericArgs()[0]);
   }
 
+  // Set and restore in-parameter-position printing of types
+  auto prior = Options.PrintAsInParamType;
+  Options.PrintAsInParamType = true;
   printTypeLoc(TheTypeLoc);
+  Options.PrintAsInParamType = prior;
 
   if (param->isVariadic())
     Printer << "...";
@@ -3313,6 +3321,10 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
   const PrintOptions &Options;
   Optional<std::vector<GenericParamList *>> UnwrappedGenericParams;
 
+  /// Whether we are printing something in a function parameter position, and
+  /// thus want to print @escaping if it escapes.
+  bool inParameterPrinting;
+
   void printDeclContext(DeclContext *DC) {
     switch (DC->getContextKind()) {
     case DeclContextKind::Module: {
@@ -3480,7 +3492,8 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
 
 public:
   TypePrinter(ASTPrinter &Printer, const PrintOptions &PO)
-      : Printer(Printer), Options(PO) {}
+      : Printer(Printer), Options(PO),
+        inParameterPrinting(Options.PrintAsInParamType) {}
 
   void visit(Type T) {
     Printer.printTypePre(TypeLoc::withoutLoc(T));
@@ -3745,11 +3758,10 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
         Printer << "@autoclosure ";
       else
         Printer << "@autoclosure(escaping) ";
-    } else if (info.isNoEscape()) {
-      // autoclosure implies noescape.
-      Printer << "@noescape ";
-    } else if (info.isExplicitlyEscaping()) {
-      Printer << "@escaping ";
+    } else if (inParameterPrinting) {
+      if (!info.isNoEscape()) {
+        Printer << "@escaping ";
+      }
     }
 
     if (Options.PrintFunctionRepresentationAttrs) {
@@ -3841,8 +3853,17 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
 
     if (needsParens)
       Printer << "(";
-    
+
+    // Set in-parameter-position printing to print our parameters, then unset it
+    // for the return type (in case it is also a function), and restore at the
+    // end.
+    auto prior = inParameterPrinting;
+    inParameterPrinting = true;
     visit(inputType);
+    inParameterPrinting = false;
+    SWIFT_DEFER {
+      inParameterPrinting = prior;
+    };
 
     if (needsParens)
       Printer << ")";

lib/Sema/CSApply.cpp

@@ -5587,7 +5587,6 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
         swift::AnyFunctionType::ExtInfo newEI(fromEI.getRepresentation(),
                                         toEI.isAutoClosure(),
                                         toEI.isNoEscape() | fromEI.isNoEscape(),
-                                        toEI.isExplicitlyEscaping() | fromEI.isExplicitlyEscaping(),
                                         toEI.throws() & fromEI.throws());
         auto newToType = FunctionType::get(fromFunc->getInput(),
                                            fromFunc->getResult(), newEI);

lib/Sema/MiscDiagnostics.cpp

@@ -464,7 +464,16 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
 
       TC.diagnose(DRE->getStartLoc(), diag::invalid_noescape_use,
                   DRE->getDecl()->getName(), isa<ParamDecl>(DRE->getDecl()));
-      if (AFT->isAutoClosure())
+
+      // If we're a parameter, emit a helpful fixit to add @escaping
+      auto paramDecl = dyn_cast<ParamDecl>(DRE->getDecl());
+      auto isAutoClosure = AFT->isAutoClosure();
+      if (paramDecl && !isAutoClosure) {
+        TC.diagnose(paramDecl->getStartLoc(), diag::noescape_parameter,
+                    paramDecl->getName())
+            .fixItInsert(paramDecl->getTypeLoc().getLoc(), "@escaping ");
+      } else if (isAutoClosure)
+        // TODO: add in a fixit for autoclosure
         TC.diagnose(DRE->getDecl()->getLoc(), diag::noescape_autoclosure,
                     DRE->getDecl()->getName());
     }

lib/Sema/TypeCheckAttr.cpp

@@ -1628,10 +1628,6 @@ void TypeChecker::checkNoEscapeAttr(ParamDecl *PD, NoEscapeAttr *attr) {
     return;
   }
 
-  // Just stop if we've already applied this attribute.
-  if (FTy->isNoEscape())
-    return;
-
   // This range can be implicit e.g. if we're in the middle of diagnosing
   // @autoclosure.
   auto attrRemovalRange = attr->getRangeWithAt();
@@ -1645,6 +1641,10 @@ void TypeChecker::checkNoEscapeAttr(ParamDecl *PD, NoEscapeAttr *attr) {
     .fixItRemove(attrRemovalRange) 
     .fixItInsert(PD->getTypeLoc().getSourceRange().Start, "@noescape ");
 
+  // Stop if we've already applied this attribute.
+  if (FTy->isNoEscape())
+    return;
+
   // Change the type to include the noescape bit.
   PD->overwriteType(FunctionType::get(FTy->getInput(), FTy->getResult(),
                                       FTy->getExtInfo().withNoEscape(true)));

lib/Sema/TypeCheckCaptures.cpp

@@ -154,12 +154,22 @@ class FindCapturedVars : public ASTWalker {
       // Otherwise, diagnose this as an invalid capture.
       bool isDecl = AFR.getAbstractFunctionDecl() != nullptr;
 
-      TC.diagnose(Loc, isDecl ? diag::decl_closure_noescape_use :
-                  diag::closure_noescape_use, VD->getName());
-
-      if (VD->getType()->castTo<AnyFunctionType>()->isAutoClosure())
-        TC.diagnose(VD->getLoc(), diag::noescape_autoclosure,
-                    VD->getName());
+      TC.diagnose(Loc, isDecl ? diag::decl_closure_noescape_use
+                              : diag::closure_noescape_use,
+                  VD->getName());
+
+      // If we're a parameter, emit a helpful fixit to add @escaping
+      auto paramDecl = dyn_cast<ParamDecl>(VD);
+      bool isAutoClosure =
+          VD->getType()->castTo<AnyFunctionType>()->isAutoClosure();
+      if (paramDecl && !isAutoClosure) {
+        TC.diagnose(paramDecl->getStartLoc(), diag::noescape_parameter,
+                    paramDecl->getName())
+            .fixItInsert(paramDecl->getTypeLoc().getLoc(), "@escaping ");
+      } else if (isAutoClosure) {
+        // TODO: add in a fixit for autoclosure
+        TC.diagnose(VD->getLoc(), diag::noescape_autoclosure, VD->getName());
+      }
     }
   }
 

lib/Sema/TypeCheckType.cpp

@@ -1579,13 +1579,25 @@ Type TypeChecker::resolveType(TypeRepr *TyR, DeclContext *DC,
   return result;
 }
 
+/// Whether the given DC is a noescape-by-default context, i.e. not a property
+/// setter
+static bool isDefaultNoEscapeContext(const DeclContext *DC) {
+  auto funcDecl = dyn_cast<FuncDecl>(DC);
+  return !funcDecl || !funcDecl->isSetter();
+}
+
 Type TypeResolver::resolveType(TypeRepr *repr, TypeResolutionOptions options) {
   assert(repr && "Cannot validate null TypeReprs!");
 
   // If we know the type representation is invalid, just return an
   // error type.
   if (repr->isInvalid()) return ErrorType::get(TC.Context);
 
+  // Remember whether this is a function parameter.
+  bool isFunctionParam =
+    options.contains(TR_FunctionInput) ||
+    options.contains(TR_ImmediateFunctionInput);
+
   // Strip the "is function input" bits unless this is a type that knows about
   // them.
   if (!isa<InOutTypeRepr>(repr) && !isa<TupleTypeRepr>(repr) &&
@@ -1611,8 +1623,14 @@ Type TypeResolver::resolveType(TypeRepr *repr, TypeResolutionOptions options) {
                                     UnsatisfiedDependency);
 
   case TypeReprKind::Function:
-    if (!(options & TR_SILType))
-      return resolveASTFunctionType(cast<FunctionTypeRepr>(repr), options);
+    if (!(options & TR_SILType)) {
+      // Default non-escaping for closure parameters
+      auto info = AnyFunctionType::ExtInfo().withNoEscape(
+          isFunctionParam &&
+          isDefaultNoEscapeContext(DC));
+      return resolveASTFunctionType(cast<FunctionTypeRepr>(repr), options,
+                                    info);
+    }
     return resolveSILFunctionType(cast<FunctionTypeRepr>(repr), options);
 
   case TypeReprKind::Array:
@@ -1870,11 +1888,22 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
           .fixItReplace(resultRange, "Never");
     }
 
+    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);
+      }
+    }
+
     // Resolve the function type directly with these attributes.
     FunctionType::ExtInfo extInfo(rep,
                                   attrs.has(TAK_autoclosure),
-                                  attrs.has(TAK_noescape),
-                                  attrs.has(TAK_escaping),
+                                  defaultNoEscape | attrs.has(TAK_noescape),
                                   fnRepr->throws());
 
     ty = resolveASTFunctionType(fnRepr, options, extInfo);

lib/Serialization/Deserialization.cpp

@@ -3493,13 +3493,12 @@ Type ModuleFile::getType(TypeID TID) {
     TypeID inputID;
     TypeID resultID;
     uint8_t rawRepresentation;
-    bool autoClosure, noescape, explicitlyEscaping, throws;
+    bool autoClosure, noescape, throws;
 
     decls_block::FunctionTypeLayout::readRecord(scratch, inputID, resultID,
                                                 rawRepresentation,
                                                 autoClosure,
                                                 noescape,
-                                                explicitlyEscaping,
                                                 throws);
     auto representation = getActualFunctionTypeRepresentation(rawRepresentation);
     if (!representation.hasValue()) {
@@ -3509,7 +3508,7 @@ Type ModuleFile::getType(TypeID TID) {
 
     auto Info = FunctionType::ExtInfo(*representation,
                                autoClosure, noescape,
-                               explicitlyEscaping, throws);
+                               throws);
 
     typeOrOffset = FunctionType::get(getType(inputID), getType(resultID),
                                      Info);