Merge pull request #4878 from slavapestov/fix-variadic-escaping-autoclosure-3.0

Sema: Variadic parameters are always @escaping and cannot be @autoclo…

Author: milseman

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