SIL: Fix some corner cases with tuple type lowering

1) It's possible to materialize a tuple value with an @escaping or
@autoclosure element in it.

I don't think this causes any bad behavior in 4.2 because these
flags have no semantic effect after the type checker, but now
I'm adding an assertion that will fire when such types are
serialized, so let's make sure it doesn't happen by explicitly
clearing out these flags when lowering tuples types.

2) It's also possible to materialize a tuple with a single vararg
element. Again, this was not a problem in 4.2, but with the above
change to start clearing tuple flags, we now end up in a
situation where the lowered type is not a tuple, because
TupleType::get() returns a ParenType if the tuple has one
element that is not vararg (which it no longer is, because we
just cleared all the flags).

Fix the second problem by treating one-element vararg tuples just
like tuples with inout, __shared and __owned elements, that is,
by always exploding them when they appear at the top level of a
function parameter list, ensuring we never try to materialize
a value whose type is the entire tuple type.

These problems all stem from the fact that lowering a function type
with the opaque abstraction pattern treats the top level argument
list as a single tuple argument. Once that is fixed, much of the
above will simplify down to assertions.

Author: slavapestov

include/swift/SIL/TypeLowering.h

@@ -41,6 +41,29 @@ namespace swift {
 
 namespace Lowering {
 
+/// Should this tuple type always be expanded into its elements, even
+/// when emitted against an opaque abstraction pattern?
+///
+/// FIXME: Remove this once function signature lowering always explodes
+/// the top-level argument list.
+inline bool shouldExpandTupleType(TupleType *type) {
+  // Tuples with inout, __shared and __owned elements cannot be lowered
+  // to SIL types.
+  if (type->hasElementWithOwnership())
+    return true;
+
+  // A one-element tuple with a vararg element is essentially
+  // equivalent to the element itself, and we also can't lower it, since
+  // that would strip off the vararg-ness and produce a non-tuple type.
+  if (type->getNumElements() == 1 &&
+      type->getElement(0).isVararg()) {
+    return true;
+  }
+
+  // Everything else is OK.
+  return false;
+}
+
 /// The default convention for handling the callee object on thick
 /// callees.
 const ParameterConvention DefaultThickCalleeConvention =

lib/SIL/SILFunctionType.cpp

@@ -607,7 +607,7 @@ class DestructureInputs {
       // a valid target for substitution, don't expand it.
       if (!tty ||
           (pattern.isTypeParameter() &&
-           !tty->hasElementWithOwnership())) {
+           !shouldExpandTupleType(tty))) {
         visit(paramFlags.getValueOwnership(), /*forSelf=*/false, pattern, ty,
               silRepresentation);
         return;

lib/SIL/TypeLowering.cpp

@@ -1335,6 +1335,10 @@ void TypeConverter::insert(TypeKey k, const TypeLowering *tl) {
 static CanTupleType getLoweredTupleType(TypeConverter &tc,
                                         AbstractionPattern origType,
                                         CanTupleType substType) {
+  // We can't lower InOutType, and we can't lower an unlabeled one
+  // element vararg tuple either, because lowering strips off flags,
+  // which would end up producing a ParenType.
+  assert(!shouldExpandTupleType(substType));
   assert(origType.matchesTuple(substType));
 
   // Does the lowered tuple type differ from the substituted type in
@@ -1347,27 +1351,32 @@ static CanTupleType getLoweredTupleType(TypeConverter &tc,
     auto origEltType = origType.getTupleElementType(i);
     auto substEltType = substType.getElementType(i);
 
-    assert(!isa<LValueType>(substEltType) &&
-           "lvalue types cannot exist in function signatures");
-    assert(!isa<InOutType>(substEltType) &&
-           "inout cannot appear in tuple element type here");
-    
-    // If the original type was an archetype, use that archetype as
-    // the original type of the element --- the actual archetype
-    // doesn't matter, just the abstraction pattern.
+    auto &substElt = substType->getElement(i);
+
+    // Make sure we don't have something non-materializable.
+    auto Flags = substElt.getParameterFlags();
+    assert(Flags.getValueOwnership() == ValueOwnership::Default);
+
     SILType silType = tc.getLoweredType(origEltType, substEltType);
     CanType loweredSubstEltType = silType.getASTType();
 
-    changed = (changed || substEltType != loweredSubstEltType);
+    changed = (changed || substEltType != loweredSubstEltType ||
+               !Flags.isNone());
 
-    auto &substElt = substType->getElement(i);
-    loweredElts.push_back(substElt.getWithType(loweredSubstEltType));
+    // Note: we drop any parameter flags such as @escaping, @autoclosure and
+    // varargs.
+    //
+    // FIXME: Replace this with an assertion that the original tuple element
+    // did not have any flags.
+    loweredElts.emplace_back(loweredSubstEltType,
+                             substElt.getName(),
+                             ParameterTypeFlags());
   }
 
   if (!changed) return substType;
 
-  // Because we're transforming an existing tuple, the weird corner
-  // case where TupleType::get does not return a TupleType can't happen.
+  // The cast should succeed, because if we end up with a one-element
+  // tuple type here, it must have a label.
   return cast<TupleType>(CanType(TupleType::get(loweredElts, tc.Context)));
 }
 

lib/SILGen/SILGenApply.cpp

@@ -2448,10 +2448,12 @@ class ArgEmitter {
     //
     // FIXME: Once -swift-version 3 code generation goes away, we
     // can simplify this.
-    if (isUnmaterializableTupleType(substArgType)) {
-      assert(origParamType.isTypeParameter());
-      emitExpanded(std::move(arg), origParamType);
-      return;
+    if (auto substTupleType = dyn_cast<TupleType>(substArgType)) {
+      if (shouldExpandTupleType(substTupleType)) {
+        assert(origParamType.isTypeParameter());
+        emitExpanded(std::move(arg), origParamType);
+        return;
+      }
     }
 
     // Okay, everything else will be passed as a single value, one
@@ -2538,13 +2540,6 @@ class ArgEmitter {
     return (dest->isValid() ? dest : nullptr);
   }
 
-  bool isUnmaterializableTupleType(CanType type) {
-    if (auto tuple = dyn_cast<TupleType>(type))
-      if (tuple->hasElementWithOwnership())
-        return true;
-    return false;
-  }
-
   /// Emit an argument as an expanded tuple.
   void emitExpanded(ArgumentSource &&arg, AbstractionPattern origParamType) {
     assert(!arg.isLValue() && "argument is l-value but parameter is tuple?");

lib/SILGen/SILGenPoly.cpp

@@ -872,8 +872,9 @@ namespace {
         }
       }
 
-      // Special-case: tuples containing inouts, __shared or __owned.
-      if (inputTupleType && inputTupleType->hasElementWithOwnership()) {
+      // Special-case: tuples containing inouts, __shared or __owned,
+      // and one-element vararg tuples.
+      if (inputTupleType && shouldExpandTupleType(inputTupleType)) {
         // Non-materializable tuple types cannot be bound as generic
         // arguments, so none of the remaining transformations apply.
         // Instead, the outermost tuple layer is exploded, even when

test/SILGen/tuple_attribute_reabstraction.swift

@@ -0,0 +1,24 @@
+// RUN: %target-swift-emit-silgen -enable-sil-ownership %s | %FileCheck %s
+
+public struct G<T> {
+  var t: T
+
+  public init(t: T) { self.t = t }
+}
+
+public func takesAutoclosureAndEscaping(_: @autoclosure () -> (), _: @escaping () -> ()) {}
+public func takesVarargs(_: Int...) {}
+
+public func f() {
+  _ = G(t: takesAutoclosureAndEscaping)
+  _ = G(t: takesVarargs)
+}
+
+// We shouldn't have @autoclosure and @escaping attributes in the lowered tuple type:
+
+// CHECK-LABEL: sil shared [transparent] [serializable] [reabstraction_thunk] @$SIg_Ieg_Iegyg_ytytIgnr__ytytIegnr_tytIegnr_TR : $@convention(thin) (@in_guaranteed (@noescape @callee_guaranteed (@in_guaranteed ()) -> @out (), @callee_guaranteed (@in_guaranteed ()) -> @out ()), @guaranteed @callee_guaranteed (@noescape @callee_guaranteed () -> (), @guaranteed @callee_guaranteed () -> ()) -> ()) -> @out ()
+
+// The one-element vararg tuple ([Int]...) should be exploded and not treated as opaque,
+// even though its materializable:
+
+// CHECK-LABEL: sil shared [transparent] [serializable] [reabstraction_thunk] @$SSaySiGIegg_AAytIegnr_TR : $@convention(thin) (@in_guaranteed Array<Int>, @guaranteed @callee_guaranteed (@guaranteed Array<Int>) -> ()) -> @out ()