SIL: Stop imploding parameter list into a single value with opaque abstraction pattern

The constraint solver support for the Swift 3 function type behavior
has been removed, so it's no longer possible to pun the same value as
both a function taking multiple parameters and a function taking a
single tuple argument.

This means the entire parameter list is no longer a target for
substitution as a single value, so the most general form of a function
value passes each parameter indirectly instead of passing a single
tuple parameter indirectly.

Author: slavapestov

include/swift/SIL/AbstractionPattern.h

@@ -34,8 +34,7 @@ namespace clang {
 namespace swift {
 namespace Lowering {
 
-/// A pattern for the abstraction of a value.  See the large comment
-/// in SILGenPoly.cpp.
+/// A pattern for the abstraction of a value.
 ///
 /// The representation of values in Swift can vary according to how
 /// their type is abstracted: which is to say, according to the pattern
@@ -93,33 +92,21 @@ namespace Lowering {
 /// this representation when made abstract.  Unfortunately, there
 /// are a lot of obvious situations where this is sub-optimal:
 /// for example, in totally non-generic code that just passes around
-/// a value of type (Int,Int)->Bool.  It's particularly bad because
-/// Swift functions take multiple arguments as just a tuple, and that
-/// tuple is usually abstractable: e.g., '<' above could also be
-/// passed to this:
-///   func fred<T>(f : T -> Bool)
+/// a value of type (Int,Int)->Bool.
 ///
 /// 3. Permit the representation of values to vary by abstraction.
 /// Values require coercion when changing abstraction patterns.
-/// For example, the argument to 'fred' would be expected to return
-/// its Bool result directly but take a single T parameter indirectly.
+/// For example, the argument to 'bar' would be expected to return
+/// its Bool result directly but take the T and U parameters indirectly.
 /// When '<' is passed to this, what must actually be passed is a
-/// thunk that expects a tuple of type (Int,Int) to be stored at
-/// the input address.
+/// thunk that loads both indirect parameters before calling '<'.
 ///
 /// There is one major risk with (3): naively implemented, a single
 /// function value which undergoes many coercions could build up a
 /// linear number of re-abstraction thunks.  However, this can be
 /// solved dynamically by applying thunks with a runtime function that
 /// can recognize and bypass its own previous handiwork.
 ///
-/// There is one major exception to what sub-expressions in a type
-/// expression can be abstracted with type variables: a type substitution
-/// must always be materializable.  For example:
-///   func f(inout Int, Int) -> Bool
-/// 'f' cannot be passed to 'foo' above: T=inout Int is not a legal
-/// substitution.  Nor can it be passed to 'fred'.
-///
 /// In general, abstraction patterns are derived from some explicit
 /// type expression, such as the written type of a variable or
 /// parameter.  This works whenever the expression directly provides
@@ -129,24 +116,25 @@ namespace Lowering {
 /// not provide structure at the appropriate level, i.e. when that
 /// level is substituted in: when the original type is merely T.  In
 /// these cases, we must devolve to a representation which all legal
-/// substitutors will agree upon.  In general, this is the
-/// representation of the type which replaces all materializable
-/// sub-expressions with a fresh type variable.
+/// substitutors will agree upon.
+///
+/// The most general type of a function type replaces all parameters and the
+/// result with fresh, unrestricted generic parameters.
+///
+/// That is, if we have a substituted function type:
+///
+///   (UnicodeScalar, (Int, Float), Double) -> (Bool, String)
+///
+/// then its most general form is
 ///
-/// For example, when applying the substitution
-///   T=(Int,Int)->Bool
-/// values of T are abstracted as if they were of type U->V, i.e.
-/// taking one indirect parameter and returning one indirect result.
+///   (A, B, C) -> D
 ///
-/// But under the substitution
-///   T=(inout Int,Int)->Bool
-/// values of T are abstracted as if they were of type (inout U,V)->W,
-/// i.e. taking one parameter inout, another indirectly, and returning
-/// one indirect result.
+/// because there is a valid substitution
+///   A := UnicodeScalar
+///   B := (Int, Float)
+///   C := Double
+///   D := (Bool, String)
 ///
-/// An abstraction pattern is represented with an original,
-/// unsubstituted type.  The archetypes or generic parameters
-/// naturally fall at exactly the specified abstraction points.
 class AbstractionPattern {
   enum class Kind {
     /// A type reference.  OrigType is valid.

include/swift/SIL/TypeLowering.h

@@ -41,43 +41,6 @@ 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;
-}
-
-/// A version of the above for parameter lists.
-///
-/// FIXME: Should also remove this soon.
-inline bool shouldExpandParams(AnyFunctionType::CanParamArrayRef params) {
-  for (auto param : params)
-    if (param.getValueOwnership() != ValueOwnership::Default)
-      return true;
-
-  if (params.size() == 1)
-    return params[0].isVariadic();
-
-  return false;
-}
-
 /// The default convention for handling the callee object on thick
 /// callees.
 const ParameterConvention DefaultThickCalleeConvention =

lib/SIL/SILFunctionType.cpp

@@ -477,70 +477,15 @@ static bool isFormallyPassedIndirectly(SILModule &M,
   }
 }
 
-/// A visitor for turning formal input types into SILParameterInfos,
-/// matching the abstraction patterns of the original type.
-///
-/// If the original abstraction pattern is fully opaque, we must
-/// pass the function's inputs as if the original type were the most
-/// general function signature (expressed entirely in type
-/// variables) which can be substituted to equal the given
-/// signature.
-///
-/// The goal of the most general type is to be (1) unambiguous to
-/// compute from the substituted type and (2) the same for every
-/// possible generalization of that type.  For example, suppose we
-/// have a Vector<(Int,Int)->Bool>.  Obviously, we would prefer to
-/// store optimal function pointers directly in this array; and if
-/// all uses of it are ungeneralized, we'd get away with that.  But
-/// suppose the vector is passed to a function like this:
-///   func satisfiesAll<T>(v : Vector<(T,T)->Bool>, x : T, y : T) -> Bool
-/// That function will expect to be able to pull values out with the
-/// proper abstraction.  The only type we can possibly expect to agree
-/// upon is the most general form.
-///
-/// The precise way this works is that Vector's subscript operation
-/// (assuming that's how it's being accessed) has this signature:
-///   <X> Vector<X> -> Int -> X
-/// which 'satisfiesAll' is calling with this substitution:
-///   X := (T, T) -> Bool
-/// Since 'satisfiesAll' has a function type substituting for an
-/// unrestricted archetype, it expects the value returned to have the
-/// most general possible form 'A -> B', which it will need to
-/// de-generalize (by thunking) if it needs to pass it around as
-/// a '(T, T) -> Bool' value.
-///
-/// It is only this sort of direct substitution in types that forces
-/// the most general possible type to be selected; declarations will
-/// generally provide a target generalization level.  For example,
-/// in a Vector<IntPredicate>, where IntPredicate is a struct (not a
-/// tuple) with one field of type (Int, Int) -> Bool, all the
-/// function pointers will be stored ungeneralized.  Of course, such
-/// a vector couldn't be passed to 'satisfiesAll'.
-///
-/// For most types, the most general type is simply a fresh,
-/// unrestricted type variable.  But unmaterializable types are not
-/// valid results of substitutions, so this does not apply.  The
-/// most general form of an unmaterializable type preserves the
-/// basic structure of the unmaterializable components, replacing
-/// any materializable components with fresh type variables.
+/// A visitor for turning formal input types into SILParameterInfos, matching
+/// the abstraction patterns of the original type.
 ///
-/// That is, if we have a substituted function type:
-///   (UnicodeScalar, (Int, Float), Double) -> Bool
-/// then its most general form is
-///   A -> B
+/// If the original abstraction pattern is fully opaque, we must pass the
+/// function's parameters and results indirectly, as if the original type were
+/// the most general function signature (expressed entirely in generic
+/// parameters) which can be substituted to equal the given signature.
 ///
-/// because there is a valid substitution
-///   A := (UnicodeScalar, (Int, Float), Double)
-///   B := Bool
-///
-/// But if we have a substituted function type:
-///   (UnicodeScalar, (Int, Float), inout Double) -> Bool
-/// then its most general form is
-///   (A, B, inout C) -> D
-/// because the substitution
-///   X := (UnicodeScalar, (Int, Float), inout Double)
-/// is invalid substitution, ultimately because 'inout Double'
-/// is not materializable.
+/// See the comment in AbstractionPattern.h for details.
 class DestructureInputs {
   SILModule &M;
   const Conventions &Convs;
@@ -603,21 +548,6 @@ class DestructureInputs {
     // Add any leading foreign parameters.
     maybeAddForeignParameters();
 
-    // FIXME(swift3): Remove this.
-    //
-    // If the abstraction pattern is opaque and the parameter list is a valid
-    // target for substitution, implode it into a single tuple parameter.
-    if (!hasSelf) {
-      if (origType.isTypeParameter() && !shouldExpandParams(params)) {
-        CanType ty = AnyFunctionType::composeInput(M.getASTContext(), params,
-                                                   /*canonicalVararg*/true)
-                                                     ->getCanonicalType();
-        visit(ValueOwnership::Default, /*forSelf=*/false,
-              origType, ty, silRepresentation);
-        return;
-      }
-    }
-
     // Process all the non-self parameters.
     for (unsigned i = 0; i != numNonSelfParams; ++i) {
       auto ty = params[i].getParameterType();

lib/SIL/TypeLowering.cpp

@@ -1335,10 +1335,6 @@ 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
@@ -1356,23 +1352,24 @@ static CanTupleType getLoweredTupleType(TypeConverter &tc,
     // Make sure we don't have something non-materializable.
     auto Flags = substElt.getParameterFlags();
     assert(Flags.getValueOwnership() == ValueOwnership::Default);
+    assert(!Flags.isVariadic());
 
     SILType silType = tc.getLoweredType(origEltType, substEltType);
     CanType loweredSubstEltType = silType.getASTType();
 
     changed = (changed || substEltType != loweredSubstEltType ||
                !Flags.isNone());
 
-    // Note: we drop any parameter flags such as @escaping, @autoclosure and
-    // varargs.
+    // Note: we drop @escaping and @autoclosure which can still appear on
+    // materializable tuple types.
     //
     // 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;
 
   // The cast should succeed, because if we end up with a one-element

lib/SILGen/SILGenApply.cpp

@@ -1712,13 +1712,11 @@ static unsigned getFlattenedValueCount(AbstractionPattern origType,
 
   // The count is always 1 unless the substituted type is a tuple.
   auto substTuple = dyn_cast<TupleType>(substType);
-  if (!substTuple) return 1;
+  if (!substTuple)
+    return 1;
 
-  // If the original type is opaque and the substituted type is
-  // materializable, the count is 1 anyway.
-  //
-  // FIXME: Should always be materializable here.
-  if (origType.isTypeParameter() && substTuple->isMaterializable())
+  // If the original type is opaque, the count is 1 anyway.
+  if (origType.isTypeParameter())
     return 1;
 
   // Otherwise, add up the elements.