Merge pull request #18655 from slavapestov/legacy-tuple-cleanup

More legacy tuple cleanup

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -974,9 +974,6 @@ ERROR(wrong_argument_labels,none,
       "incorrect argument label%select{|s}0 in %select{call|subscript}3 "
       "(have '%1', expected '%2')",
       (bool, StringRef, StringRef, bool))
-ERROR(extra_named_single_element_tuple,none,
-      "cannot treat single-element named tuple as a scalar; use '.%0' to "
-      "access its element", (StringRef))
 ERROR(argument_out_of_order_named_named,none,
       "argument %0 must precede argument %1", (Identifier, Identifier))
 ERROR(argument_out_of_order_named_unnamed,none,

include/swift/AST/Types.h

@@ -1876,16 +1876,6 @@ class TupleType final : public TypeBase, public llvm::FoldingSetNode,
   /// return the element index, otherwise return -1.
   int getNamedElementId(Identifier I) const;
 
-  /// getElementForScalarInit - If a tuple of this type can be initialized with
-  /// a scalar, return the element number that the scalar is assigned to.  If
-  /// not, return -1.
-  int getElementForScalarInit() const;
-  
-  /// If this tuple has a varargs element to it, return the base type of the
-  /// varargs element (i.e., if it is "Int...", this returns Int, not [Int]).
-  /// Otherwise, this returns Type().
-  Type getVarArgsBaseType() const;
-  
   /// Returns true if this tuple has inout elements.
   bool hasInOutElement() const {
     return static_cast<bool>(Bits.TupleType.HasInOutElement);

lib/AST/Type.cpp

@@ -2422,45 +2422,6 @@ int TupleType::getNamedElementId(Identifier I) const {
   return -1;
 }
 
-/// getElementForScalarInit - If a tuple of this type can be initialized with a
-/// scalar, return the field number that the scalar is assigned to.  If not,
-/// return -1.
-int TupleType::getElementForScalarInit() const {
-  if (Bits.TupleType.Count == 0) return -1;
-  
-  int FieldWithoutDefault = -1;
-  for (unsigned i = 0, e = Bits.TupleType.Count; i != e; ++i) {
-    // If we already saw a non-vararg field missing a default value, then we
-    // cannot assign a scalar to this tuple.
-    if (FieldWithoutDefault != -1) {
-      // Vararg fields are okay; they'll just end up being empty.
-      if (getTrailingObjects<TupleTypeElt>()[i].isVararg())
-        continue;
-    
-      return -1;
-    }
-    
-    // Otherwise, remember this field number.
-    FieldWithoutDefault = i;    
-  }
-  
-  // If all the elements have default values, the scalar initializes the first
-  // value in the tuple.
-  return FieldWithoutDefault == -1 ? 0 : FieldWithoutDefault;
-}
-
-/// If this tuple has a varargs element to it, return the base type of the
-/// varargs element (i.e., if it is "Int...", this returns Int, not [Int]).
-/// Otherwise, this returns Type().
-Type TupleType::getVarArgsBaseType() const {
-  for (unsigned i = 0, e = Bits.TupleType.Count; i != e; ++i) {
-    if (getTrailingObjects<TupleTypeElt>()[i].isVararg())
-      return getTrailingObjects<TupleTypeElt>()[i].getVarargBaseTy();
-  }
-  
-  return Type();
-}
-
 
 ArchetypeType::ArchetypeType(
   const ASTContext &Ctx,

lib/Sema/CSApply.cpp

@@ -6606,19 +6606,7 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
                                         /*isImplicit*/ true));
     }
 
-    // If we're actually turning this into an lvalue tuple element, don't
-    // load.
-    bool performLoad = true;
-    if (auto toTuple = toType->getAs<TupleType>()) {
-      int scalarIdx = toTuple->getElementForScalarInit();
-      if (scalarIdx >= 0 &&
-          toTuple->getElement(scalarIdx).isInOut())
-        performLoad = false;
-    }
-
-    if (performLoad) {
-      return coerceToType(addImplicitLoadExpr(cs, expr), toType, locator);
-    }
+    return coerceToType(addImplicitLoadExpr(cs, expr), toType, locator);
   }
 
   // Coercions to tuple type.

lib/Sema/CSDiag.cpp

@@ -3482,9 +3482,24 @@ static Optional<unsigned> getElementForScalarInitOfArg(
 
   auto getElementForScalarInitSimple =
       [](const TupleType *tupleTy) -> Optional<unsigned> {
-    int index = tupleTy->getElementForScalarInit();
-    if (index < 0) return None;
-    return index;    
+    Optional<unsigned> result = None;
+    for (unsigned i = 0, e = tupleTy->getNumElements(); i != e; ++i) {
+      // If we already saw a non-vararg field, then we have more than
+      // one candidate field.
+      if (result.hasValue()) {
+        // Vararg fields are okay; they'll just end up being empty.
+        if (tupleTy->getElement(i).isVararg())
+          continue;
+
+        // Give up.
+        return None;
+      }
+
+      // Otherwise, remember this field number.
+      result = i;
+    }
+
+    return result;
   };
 
   // If there aren't any candidates, we're done.
@@ -8281,7 +8296,16 @@ bool FailureDiagnosis::visitTupleExpr(TupleExpr *TE) {
   }
 
   if (!variadicArgs.empty()) {
-    auto varargsTy = contextualTT->getVarArgsBaseType();
+    Type varargsTy;
+    for (auto &elt : contextualTT->getElements()) {
+      if (elt.isVararg()) {
+        varargsTy = elt.getVarargBaseTy();
+        break;
+      }
+    }
+
+    assert(varargsTy);
+
     for (unsigned i = 0, e = variadicArgs.size(); i != e; ++i) {
       unsigned inArgNo = variadicArgs[i];
 

lib/Sema/CSSolver.cpp

@@ -314,21 +314,6 @@ static SmallVector<Type, 4>
 enumerateDirectSupertypes(TypeChecker &tc, Type type) {
   SmallVector<Type, 4> result;
 
-  if (auto tupleTy = type->getAs<TupleType>()) {
-    // A tuple that can be constructed from a scalar has a value of that
-    // scalar type as its supertype.
-    // FIXME: There is a way more general property here, where we can drop
-    // one label from the tuple, maintaining the rest.
-    int scalarIdx = tupleTy->getElementForScalarInit();
-    if (scalarIdx >= 0) {
-      auto &elt = tupleTy->getElement(scalarIdx);
-      if (elt.isVararg()) // FIXME: Should we keep the name?
-        result.push_back(elt.getVarargBaseTy());
-      else if (elt.hasName())
-        result.push_back(elt.getType());
-    }
-  }
-
   if (type->mayHaveSuperclass()) {
     // FIXME: Can also weaken to the set of protocol constraints, but only
     // if there are any protocols that the type conforms to but the superclass
@@ -729,16 +714,6 @@ bool ConstraintSystem::tryTypeVariableBindings(
       }
 
       if (binding.Kind == AllowedBindingKind::Subtypes) {
-        if (auto tupleTy = type->getAs<TupleType>()) {
-          int scalarIdx = tupleTy->getElementForScalarInit();
-          if (scalarIdx >= 0) {
-            auto eltType = tupleTy->getElementType(scalarIdx);
-            if (exploredTypes.insert(eltType->getCanonicalType()).second)
-              newBindings.push_back(
-                  {eltType, binding.Kind, binding.BindingSource});
-          }
-        }
-
         // If we were unsuccessful solving for T?, try solving for T.
         if (auto objTy = type->getOptionalObjectType()) {
           if (exploredTypes.insert(objTy->getCanonicalType()).second) {

lib/Sema/MiscDiagnostics.cpp

@@ -1768,39 +1768,16 @@ bool swift::diagnoseArgumentLabelError(ASTContext &ctx,
   auto &diags = ctx.Diags;
   auto tuple = dyn_cast<TupleExpr>(expr);
   if (!tuple) {
-    llvm::SmallString<16> str;
-    // If the diagnostic is local, flush it before returning.
-    // This makes sure it's emitted before 'str' is destroyed.
-    SWIFT_DEFER { diagOpt.reset(); };
-
     if (newNames[0].empty()) {
-      // This is probably a conversion from a value of labeled tuple type to
-      // a scalar.
-      // FIXME: We want this issue to disappear completely when single-element
-      // labeled tuples go away.
-      if (auto tupleTy = expr->getType()->getRValueType()->getAs<TupleType>()) {
-        int scalarFieldIdx = tupleTy->getElementForScalarInit();
-        if (scalarFieldIdx >= 0) {
-          auto &field = tupleTy->getElement(scalarFieldIdx);
-          if (field.hasName()) {
-            str = ".";
-            str += field.getName().str();
-            if (!existingDiag) {
-              diagOpt.emplace(
-                          diags.diagnose(expr->getStartLoc(),
-                                         diag::extra_named_single_element_tuple,
-                                         field.getName().str()));
-            }
-            getDiag().fixItInsertAfter(expr->getEndLoc(), str);
-            return true;
-          }
-        }
-      }
-
       // We don't know what to do with this.
       return false;
     }
 
+    llvm::SmallString<16> str;
+    // If the diagnostic is local, flush it before returning.
+    // This makes sure it's emitted before 'str' is destroyed.
+    SWIFT_DEFER { diagOpt.reset(); };
+
     // This is a scalar-to-tuple conversion. Add the name. We "know"
     // that we're inside a ParenExpr, because ParenExprs are required
     // by the syntax and locator resolution looks through on level of

lib/Sema/TypeCheckError.cpp

@@ -674,8 +674,7 @@ class ApplyClassifier {
         return classifyShuffleRethrowsArgument(shuffle, paramTupleType);
       }
 
-      int scalarElt = paramTupleType->getElementForScalarInit();
-      if (scalarElt < 0) {
+      if (paramTupleType->getNumElements() != 1) {
         // Otherwise, we're passing an opaque tuple expression, and we
         // should treat it as contributing to 'rethrows' if the original
         // parameter type included a throwing function type.
@@ -684,7 +683,10 @@ class ApplyClassifier {
                                     PotentialReason::forRethrowsArgument(arg));
       }
 
-      paramType = paramTupleType->getElementType(scalarElt);
+      // FIXME: There's a case where we can end up with an ApplyExpr that
+      // has a single-element-tuple argument type, but the argument is just
+      // a ClosureExpr and not a TupleExpr/TupleShuffleExpr.
+      paramType = paramTupleType->getElementType(0);
     }
 
     // Otherwise, if the original parameter type was not a throwing

lib/Sema/TypeCheckPattern.cpp

@@ -1646,7 +1646,9 @@ bool TypeChecker::coerceParameterListToType(ParameterList *P, ClosureExpr *CE,
   auto hasParenSugar = [](ArrayRef<AnyFunctionType::Param> params) -> bool {
     if (params.size() == 1) {
       const auto &param = params.front();
-      return !param.hasLabel() && !param.isVariadic();
+      return (!param.hasLabel() &&
+              !param.isVariadic() &&
+              !param.isInOut());
     }
 
     return false;
@@ -1661,36 +1663,28 @@ bool TypeChecker::coerceParameterListToType(ParameterList *P, ClosureExpr *CE,
     return type;
   };
 
-  // Check if parameter list only contains one single tuple.
-  // If it is, then parameter type would be sugared ParenType
-  // with a single underlying TupleType. In that case, check if
-  // the closure argument is also one to avoid the tuple splat
-  // from happening.
+  // If the closure is called with a single argument of tuple type
+  // but the closure body expects multiple parameters, explode the
+  // tuple.
+  //
+  // FIXME: This looks like the wrong place for this; the constraint
+  // solver should have inserted an explicit conversion already.
+  //
+  // The only reason we can get away with this, I think, is that
+  // at the SIL level, recursive tuple expansion lowers
+  // ((T, U)) -> () and (T, U) -> () to the same function type,
+  // and SILGen doesn't enforce AST invariaints very strictly.
   if (!hadError && hasParenSugar(params)) {
-    auto underlyingTy = params.front().getType();
-
-    if (underlyingTy->is<TupleType>() &&
-        !underlyingTy->castTo<TupleType>()->getVarArgsBaseType()) {
+    auto underlyingTy = params.front().getPlainType();
+    if (underlyingTy->is<TupleType>()) {
+      // If we're actually expecting a single parameter, handle it normally.
       if (P->size() == 1)
         return handleParameter(P->get(0), underlyingTy, /*mutable*/false);
-    }
 
-    // pass (strip paren sugar)
-    params.clear();
-    FunctionType::decomposeInput(underlyingTy, params);
-  }
-
-  // The context type must be a tuple.
-  if (hasParenSugar(params) && !hadError) {
-    const auto &param = params.front();
-    if (P->size() == 1) {
-      assert(P->size() == params.size());
-      return handleParameter(P->get(0), getType(param),
-                             /*mutable*/ param.isInOut());
+      // Otherwise, explode the tuple.
+      params.clear();
+      FunctionType::decomposeInput(underlyingTy, params);
     }
-    diagnose(P->getStartLoc(), diag::tuple_pattern_in_non_tuple_context,
-             param.getType());
-    hadError = true;
   }
 
   // The number of elements must match exactly.