Sema: Variadic parameters are always @escaping and cannot be @autoclosure

A variadic parameter of function type must be @escaping -- we cannot
reason about an array of non-escaping closures, so this was a safety
hole.

Also, attempting to define an @autoclosure variadic did not produce a
diagnostic, but would fail later on if you actually tried to do
anything with it. Let's ban this completely.

Both changes are source breaking, but impact is limited to code that
was already only marginally valid.

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -906,10 +906,6 @@ ERROR(attribute_requires_operator_identifier,none,
 ERROR(attribute_requires_single_argument,none,
       "'%0' requires a function with one argument", (StringRef))
 
-ERROR(inout_cant_be_variadic,none,
-      "inout arguments cannot be variadic", ())
-ERROR(inout_only_parameter,none,
-      "'inout' is only valid in parameter lists", ())
 ERROR(var_parameter_not_allowed,none,
       "parameters may not have the 'var' specifier", ())
 
@@ -1831,10 +1827,12 @@ ERROR(property_behavior_invalid_parameter,none,
 // Type Check Attributes
 //------------------------------------------------------------------------------
 
-ERROR(attr_only_only_one_decl_kind,none,
+ERROR(attr_only_one_decl_kind,none,
       "%0 may only be used on '%1' declarations", (DeclAttribute,StringRef))
-ERROR(attr_only_only_on_parameters,none,
+ERROR(attr_only_on_parameters,none,
       "%0 may only be used on parameters", (StringRef))
+ERROR(attr_not_on_variadic_parameters,none,
+      "%0 may not be used on variadic parameters", (StringRef))
 
 
 ERROR(override_final,none,

lib/Sema/TypeCheckAttr.cpp

@@ -681,7 +681,7 @@ void TypeChecker::checkDeclAttributesEarly(Decl *D) {
     }
 
     if (!OnlyKind.empty())
-      Checker.diagnoseAndRemoveAttr(attr, diag::attr_only_only_one_decl_kind,
+      Checker.diagnoseAndRemoveAttr(attr, diag::attr_only_one_decl_kind,
                                     attr, OnlyKind);
     else if (attr->isDeclModifier())
       Checker.diagnoseAndRemoveAttr(attr, diag::invalid_decl_modifier, attr);
@@ -1577,7 +1577,7 @@ void TypeChecker::checkTypeModifyingDeclAttributes(VarDecl *var) {
       checkAutoClosureAttr(pd, attr);
     else {
       AttributeEarlyChecker Checker(*this, var);
-      Checker.diagnoseAndRemoveAttr(attr, diag::attr_only_only_one_decl_kind,
+      Checker.diagnoseAndRemoveAttr(attr, diag::attr_only_one_decl_kind,
                                     attr, "parameter");
     }
   }
@@ -1586,7 +1586,7 @@ void TypeChecker::checkTypeModifyingDeclAttributes(VarDecl *var) {
       checkNoEscapeAttr(pd, attr);
     else {
       AttributeEarlyChecker Checker(*this, var);
-      Checker.diagnoseAndRemoveAttr(attr, diag::attr_only_only_one_decl_kind,
+      Checker.diagnoseAndRemoveAttr(attr, diag::attr_only_one_decl_kind,
                                     attr, "parameter");
     }
   }

lib/Sema/TypeCheckPattern.cpp

@@ -739,21 +739,21 @@ static bool validateParameterType(ParamDecl *decl, DeclContext *DC,
                                   TypeChecker &TC) {
   if (auto ty = decl->getTypeLoc().getType())
     return ty->is<ErrorType>();
-  
+
+  // If the element is a variadic parameter, resolve the parameter type as if
+  // it were in non-parameter position, since we want functions to be
+  // @escaping in this case.
+  auto elementOptions = (options |
+                         (decl->isVariadic() ? TR_VariadicFunctionInput
+                                             : TR_FunctionInput));
   bool hadError = TC.validateType(decl->getTypeLoc(), DC,
-                                  options|TR_FunctionInput, resolver);
+                                  elementOptions, resolver);
 
   Type Ty = decl->getTypeLoc().getType();
   if (decl->isVariadic() && !hadError) {
-    // If isn't legal to declare something both inout and variadic.
-    if (Ty->is<InOutType>()) {
-      TC.diagnose(decl->getStartLoc(), diag::inout_cant_be_variadic);
+    Ty = TC.getArraySliceType(decl->getStartLoc(), Ty);
+    if (Ty.isNull()) {
       hadError = true;
-    } else {
-      Ty = TC.getArraySliceType(decl->getStartLoc(), Ty);
-      if (Ty.isNull()) {
-        hadError = true;
-      }
     }
     decl->getTypeLoc().setType(Ty);
   }

lib/Sema/TypeCheckType.cpp

@@ -1779,6 +1779,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
   bool isFunctionParam =
     options.contains(TR_FunctionInput) ||
     options.contains(TR_ImmediateFunctionInput);
+  bool isVariadicFunctionParam =
+    options.contains(TR_VariadicFunctionInput);
 
   // The type we're working with, in case we want to build it differently
   // based on the attributes we see.
@@ -1959,7 +1961,9 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
     // @autoclosure is only valid on parameters.
     if (!isFunctionParam && attrs.has(TAK_autoclosure)) {
       TC.diagnose(attrs.getLoc(TAK_autoclosure),
-                  diag::attr_only_only_on_parameters, "@autoclosure");
+                  isVariadicFunctionParam
+                      ? diag::attr_not_on_variadic_parameters
+                      : diag::attr_only_on_parameters, "@autoclosure");
       attrs.clearAttribute(TAK_autoclosure);
     }
 
@@ -2390,7 +2394,11 @@ Type TypeResolver::resolveInOutType(InOutTypeRepr *repr,
   // inout is only valid for function parameters.
   if (!(options & TR_FunctionInput) &&
       !(options & TR_ImmediateFunctionInput)) {
-    TC.diagnose(repr->getInOutLoc(), diag::inout_only_parameter);
+    TC.diagnose(repr->getInOutLoc(),
+                (options & TR_VariadicFunctionInput)
+                    ? diag::attr_not_on_variadic_parameters
+                    : diag::attr_only_on_parameters,
+                "'inout'");
     repr->setInvalid();
     return ErrorType::get(Context);
   }
@@ -2510,38 +2518,58 @@ Type TypeResolver::resolveTupleType(TupleTypeRepr *repr,
     if (options & TR_ImmediateFunctionInput)
       elementOptions |= TR_FunctionInput;
   }
-  
+
+  bool complained = false;
+
+  // Variadic tuples are not permitted.
+  if (repr->hasEllipsis() &&
+      !(options & TR_ImmediateFunctionInput)) {
+    TC.diagnose(repr->getEllipsisLoc(), diag::tuple_ellipsis);
+    repr->removeEllipsis();
+    complained = true;
+  }
+
   for (auto tyR : repr->getElements()) {
     NamedTypeRepr *namedTyR = dyn_cast<NamedTypeRepr>(tyR);
-    if (namedTyR && !(options & TR_ImmediateFunctionInput)) {
-      Type ty = resolveType(namedTyR->getTypeRepr(), elementOptions);
-      if (!ty || ty->is<ErrorType>()) return ty;
+    Type ty;
+    Identifier name;
+    bool variadic = false;
 
-      elements.push_back(TupleTypeElt(ty, namedTyR->getName()));
-    } else {
+    // If the element has a label, stash the label and get the underlying type.
+    if (namedTyR) {
       // FIXME: Preserve and serialize parameter names in function types, maybe
       // with a new sugar type.
-      Type ty = resolveType(namedTyR ? namedTyR->getTypeRepr() : tyR,
-                            elementOptions);
-      if (!ty || ty->is<ErrorType>()) return ty;
+      if (!(options & TR_ImmediateFunctionInput))
+        name = namedTyR->getName();
 
-      elements.push_back(TupleTypeElt(ty));
+      tyR = namedTyR->getTypeRepr();
     }
-  }
 
-  // Tuple representations are limited outside of function inputs.
-  if (!(options & TR_ImmediateFunctionInput)) {
-    bool complained = false;
+    // If the element is a variadic parameter, resolve the parameter type as if
+    // it were in non-parameter position, since we want functions to be
+    // @escaping in this case.
+    auto thisElementOptions = elementOptions;
+    if (repr->hasEllipsis() &&
+        elements.size() == repr->getEllipsisIndex()) {
+      thisElementOptions = withoutContext(elementOptions);
+      thisElementOptions |= TR_VariadicFunctionInput;
+      variadic = true;
+    }
+
+    ty = resolveType(tyR, thisElementOptions);
+    if (!ty || ty->is<ErrorType>()) return ty;
+
+    // If the element is a variadic parameter, the underlying type is actually
+    // an ArraySlice of the element type.
+    if (variadic)
+      ty = TC.getArraySliceType(repr->getEllipsisLoc(), ty);
 
-    // Variadic tuples are not permitted.
-    if (repr->hasEllipsis()) {
-      TC.diagnose(repr->getEllipsisLoc(), diag::tuple_ellipsis);
-      repr->removeEllipsis();
-      complained = true;
-    } 
+    elements.push_back(TupleTypeElt(ty, name, variadic));
+  }
 
-    // Single-element labeled tuples are not permitted outside of declarations
-    // or SIL, either.
+  // Single-element labeled tuples are not permitted outside of declarations
+  // or SIL, either.
+  if (!(options & TR_ImmediateFunctionInput)) {
     if (elements.size() == 1 && elements[0].hasName()
         && !(options & TR_SILType)
         && !(options & TR_EnumCase)) {
@@ -2557,14 +2585,6 @@ Type TypeResolver::resolveTupleType(TupleTypeRepr *repr,
     }
   }
 
-  if (repr->hasEllipsis()) {
-    auto &element = elements[repr->getEllipsisIndex()];
-    Type baseTy = element.getType();
-    Type fullTy = TC.getArraySliceType(repr->getEllipsisLoc(), baseTy);
-    Identifier name = element.getName();
-    element = TupleTypeElt(fullTy, name, true);
-  }
-
   return TupleType::get(elements, Context);
 }
 

lib/Sema/TypeChecker.h

@@ -327,58 +327,61 @@ enum TypeResolutionFlags : unsigned {
   /// Whether this is the immediate input type to a function type,
   TR_ImmediateFunctionInput = 0x40,
 
+  /// Whether this is a variadic function input.
+  TR_VariadicFunctionInput = 0x100,
+
   /// Whether we are in the result type of a function body that is
   /// known to produce dynamic Self.
-  TR_DynamicSelfResult = 0x100,
+  TR_DynamicSelfResult = 0x200,
 
   /// Whether this is a resolution based on a non-inferred type pattern.
-  TR_FromNonInferredPattern = 0x200,
+  TR_FromNonInferredPattern = 0x400,
 
   /// Whether we are the variable type in a for/in statement.
-  TR_EnumerationVariable = 0x400,
+  TR_EnumerationVariable = 0x800,
 
   /// Whether we are looking only in the generic signature of the context
   /// we're searching, rather than the entire context.
-  TR_GenericSignature = 0x800,
+  TR_GenericSignature = 0x1000,
 
   /// Whether this type is the referent of a global type alias.
-  TR_GlobalTypeAlias = 0x1000,
+  TR_GlobalTypeAlias = 0x2000,
 
   /// Whether this type is the value carried in an enum case.
-  TR_EnumCase = 0x2000,
+  TR_EnumCase = 0x4000,
 
   /// Whether this type is being used in an expression or local declaration.
   ///
   /// This affects what sort of dependencies are recorded when resolving the
   /// type.
-  TR_InExpression = 0x4000,
+  TR_InExpression = 0x8000,
 
   /// Whether this type resolution is guaranteed not to affect downstream files.
-  TR_KnownNonCascadingDependency = 0x8000,
+  TR_KnownNonCascadingDependency = 0x10000,
 
   /// Whether we should allow references to unavailable types.
-  TR_AllowUnavailable = 0x10000,
+  TR_AllowUnavailable = 0x20000,
 
   /// Whether this is the payload subpattern of an enum pattern.
-  TR_EnumPatternPayload = 0x20000,
+  TR_EnumPatternPayload = 0x40000,
 
   /// Whether we are binding an extension declaration, which limits
   /// the lookup.
-  TR_ExtensionBinding = 0x40000,
+  TR_ExtensionBinding = 0x80000,
 
   /// Whether we are in the inheritance clause of a nominal type declaration
   /// or extension.
-  TR_InheritanceClause = 0x80000,
+  TR_InheritanceClause = 0x100000,
 
   /// Whether we should resolve only the structure of the resulting
   /// type rather than its complete semantic properties.
-  TR_ResolveStructure = 0x100000,
+  TR_ResolveStructure = 0x200000,
 
   /// Whether this is the type of an editor placeholder.
-  TR_EditorPlaceholder = 0x200000,
+  TR_EditorPlaceholder = 0x400000,
 
   /// Whether we are in a type argument for an optional
-  TR_ImmediateOptionalTypeArgument = 0x400000,
+  TR_ImmediateOptionalTypeArgument = 0x800000,
 };
 
 /// Option set describing how type resolution should work.
@@ -394,6 +397,7 @@ static inline TypeResolutionOptions
 withoutContext(TypeResolutionOptions options) {
   options -= TR_ImmediateFunctionInput;
   options -= TR_FunctionInput;
+  options -= TR_VariadicFunctionInput;
   options -= TR_EnumCase;
   options -= TR_ImmediateOptionalTypeArgument;
   return options;

test/attr/attr_autoclosure.swift

@@ -156,3 +156,7 @@ func callAutoclosureWithNoEscape_3(_ fn: @autoclosure () -> Int) {
   takesAutoclosure(fn()) // ok
 }
 
+// expected-error @+1 {{@autoclosure may not be used on variadic parameters}}
+func variadicAutoclosure(_ fn: @autoclosure () -> ()...) {
+  for _ in fn {}
+}

test/attr/attr_escaping.swift

@@ -127,3 +127,18 @@ func testModuloOptionalness() {
     deepOptionalClosure = fn // expected-error{{assigning non-escaping parameter 'fn' to an @escaping closure}}
   }
 }
+
+// Check that functions in vararg position are @escaping
+func takesEscapingFunction(fn: @escaping () -> ()) {}
+func takesArrayOfFunctions(array: [() -> ()]) {}
+
+func takesVarargsOfFunctions(fns: () -> ()...) {
+  takesArrayOfFunctions(array: fns)
+  for fn in fns {
+    takesEscapingFunction(fn: fn)
+  }
+}
+
+func takesNoEscapeFunction(fn: () -> ()) { // expected-note {{parameter 'fn' is implicitly non-escaping}}
+  takesVarargsOfFunctions(fns: fn) // expected-error {{passing non-escaping parameter 'fn' to function expecting an @escaping closure}}
+}

test/attr/attr_inout.swift

@@ -4,10 +4,10 @@ func f(x : inout Int) { } // okay
 
 func h(_ : inout Int) -> (inout Int) -> (inout Int) -> Int { }
 
-func ff(x: (inout Int, inout Float)) { } //  expected-error {{'inout' is only valid in parameter lists}}  
+func ff(x: (inout Int, inout Float)) { } //  expected-error {{'inout' may only be used on parameters}}
 
 enum inout_carrier {
-  case carry(inout Int) // expected-error {{'inout' is only valid in parameter lists}}
+  case carry(inout Int) // expected-error {{'inout' may only be used on parameters}}
 }
 
 func deprecated(inout x: Int) {} // expected-error {{'inout' before a parameter name is not allowed, place it before the parameter type instead}}

test/decl/func/vararg.swift

@@ -19,7 +19,7 @@ f3({ print($0) })
 func f4(_ a: Int..., b: Int) { }
 
 // rdar://16008564
-func inoutVariadic(_ i: inout Int...) {  // expected-error {{inout arguments cannot be variadic}}
+func inoutVariadic(_ i: inout Int...) {  // expected-error {{'inout' may not be used on variadic parameters}}
 }
 
 // rdar://19722429

test/type/types.swift

@@ -44,9 +44,9 @@ var h10 : Int?.Type?.Type
 
 var i = Int?(42)
 
-var bad_io : (Int) -> (inout Int, Int)  // expected-error {{'inout' is only valid in parameter lists}}
+var bad_io : (Int) -> (inout Int, Int)  // expected-error {{'inout' may only be used on parameters}}
 
-func bad_io2(_ a: (inout Int, Int)) {}    // expected-error {{'inout' is only valid in parameter lists}}
+func bad_io2(_ a: (inout Int, Int)) {}    // expected-error {{'inout' may only be used on parameters}}
 
 // <rdar://problem/15588967> Array type sugar default construction syntax doesn't work
 func test_array_construct<T>(_: T) {
@@ -148,12 +148,12 @@ let dictWithTuple = [String: (age:Int, count:Int)]()
 let bb2 = [Int!](repeating: nil, count: 2)
 
 // <rdar://problem/21560309> inout allowed on function return type
-func r21560309<U>(_ body: (_: inout Int) -> inout U) {}  // expected-error {{'inout' is only valid in parameter lists}}
+func r21560309<U>(_ body: (_: inout Int) -> inout U) {}  // expected-error {{'inout' may only be used on parameters}}
 r21560309 { x in x }
 
 // <rdar://problem/21949448> Accepts-invalid: 'inout' shouldn't be allowed on stored properties
 class r21949448 {
-  var myArray: inout [Int] = []   // expected-error {{'inout' is only valid in parameter lists}}
+  var myArray: inout [Int] = []   // expected-error {{'inout' may only be used on parameters}}
 }
 
 // SE-0066 - Standardize function type argument syntax to require parentheses