[CSSimplify] Add tailored diagnostics for same-shape mismatches related to arguments

Suggest to drop tuples, synthesized or remove extraneous arguments
based on pack shape mismatches.

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -5232,9 +5232,6 @@ bool MissingArgumentsFailure::diagnoseInvalidTupleDestructuring() const {
   if (!locator->isLastElement<LocatorPathElt::ApplyArgument>())
     return false;
 
-  if (SynthesizedArgs.size() < 2)
-    return false;
-
   auto *args = getArgumentListFor(locator);
   if (!args)
     return false;
@@ -5251,11 +5248,16 @@ bool MissingArgumentsFailure::diagnoseInvalidTupleDestructuring() const {
   if (!decl)
     return false;
 
+  auto *funcType =
+      resolveType(selectedOverload->openedType)->getAs<FunctionType>();
+  if (!funcType)
+    return false;
+
   auto name = decl->getBaseName();
   auto diagnostic =
       emitDiagnostic(diag::cannot_convert_single_tuple_into_multiple_arguments,
                      decl->getDescriptiveKind(), name, name.isSpecial(),
-                     SynthesizedArgs.size(), isa<TupleExpr>(argExpr));
+                     funcType->getNumParams(), isa<TupleExpr>(argExpr));
 
   // If argument is a literal tuple, let's suggest removal of parentheses.
   if (auto *TE = dyn_cast<TupleExpr>(argExpr)) {

lib/Sema/CSSimplify.cpp

@@ -1793,7 +1793,13 @@ static ConstraintSystem::TypeMatchResult matchCallArguments(
         auto *argPack = PackType::get(cs.getASTContext(), argTypes);
         auto *argPackExpansion = PackExpansionType::get(argPack, argPack);
 
-        cs.addConstraint(subKind, argPackExpansion, paramTy, loc);
+        auto firstArgIdx =
+            argTypes.empty() ? paramIdx : parameterBindings[paramIdx].front();
+
+        cs.addConstraint(
+            subKind, argPackExpansion, paramTy,
+            locator.withPathElement(LocatorPathElt::ApplyArgToParam(
+                firstArgIdx, paramIdx, param.getParameterFlags())));
         continue;
       }
     }
@@ -5673,6 +5679,18 @@ bool ConstraintSystem::repairFailures(
     if (hasFixFor(loc, FixKind::RemoveExtraneousArguments))
       return true;
 
+    // If parameter is a pack, let's see if we have already recorded
+    // either synthesized or extraneous argument fixes.
+    if (rhs->is<PackType>()) {
+      ArrayRef tmpPath(path);
+
+      // Path would end with `ApplyArgument`.
+      auto *argsLoc = getConstraintLocator(anchor, tmpPath.drop_back());
+      if (hasFixFor(argsLoc, FixKind::RemoveExtraneousArguments) ||
+          hasFixFor(argsLoc, FixKind::AddMissingArguments))
+        return true;
+    }
+
     // If the argument couldn't be found, this could be a default value
     // type mismatch.
     if (!simplifyLocatorToAnchor(loc)) {
@@ -7271,9 +7289,16 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
     case TypeKind::Pack: {
       auto tmpPackLoc = locator.withPathElement(LocatorPathElt::PackType(type1));
       auto packLoc = tmpPackLoc.withPathElement(LocatorPathElt::PackType(type2));
-      return matchPackTypes(cast<PackType>(desugar1),
-                            cast<PackType>(desugar2),
-                            kind, subflags, packLoc);
+
+      auto result =
+          matchPackTypes(cast<PackType>(desugar1), cast<PackType>(desugar2),
+                         kind, subflags, packLoc);
+
+      // Let `repairFailures` attempt to "fix" this.
+      if (shouldAttemptFixes() && result.isFailure())
+        break;
+
+      return result;
     }
     case TypeKind::PackExpansion: {
       auto expansion1 = cast<PackExpansionType>(desugar1);
@@ -13273,16 +13298,87 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifySameShapeConstraint(
     return SolutionKind::Solved;
 
   if (shouldAttemptFixes()) {
+    // If there are placeholders involved shape mismatches are most
+    // likely just a symptom of some other issue i.e. type mismatch.
     if (type1->hasPlaceholder() || type2->hasPlaceholder())
       return SolutionKind::Solved;
 
+    auto recordShapeFix = [&](ConstraintFix *fix,
+                              unsigned impact) -> SolutionKind {
+      return recordFix(fix, impact) ? SolutionKind::Error
+                                    : SolutionKind::Solved;
+    };
+
+    // Let's check whether we can produce a tailored fix for argument/parameter
+    // mismatches.
+    if (locator.endsWith<LocatorPathElt::PackShape>()) {
+      SmallVector<LocatorPathElt> path;
+      auto anchor = locator.getLocatorParts(path);
+
+      // Drop `PackShape`
+      path.pop_back();
+
+      // Tailed diagnostics for argument/parameter mismatches - there
+      // are either missing or extra arguments.
+      if (path.size() > 0 &&
+          path[path.size() - 1].is<LocatorPathElt::ApplyArgToParam>()) {
+        auto &ctx = getASTContext();
+
+        auto *loc = getConstraintLocator(anchor, path);
+        auto argLoc =
+            loc->castLastElementTo<LocatorPathElt::ApplyArgToParam>();
+
+        auto numArgs = shape1->castTo<PackType>()->getNumElements();
+        auto numParams = shape2->castTo<PackType>()->getNumElements();
+
+        // Drops `ApplyArgToParam` and left with `ApplyArgument`.
+        path.pop_back();
+
+        auto *argListLoc = getConstraintLocator(anchor, path);
+
+        // Missing arguments.
+        if (numParams > numArgs) {
+          SmallVector<SynthesizedArg> synthesizedArgs;
+          for (unsigned i = 0, n = numParams - numArgs; i != n; ++i) {
+            auto eltTy = shape2->castTo<PackType>()->getElementType(i);
+            synthesizedArgs.push_back(SynthesizedArg{
+                argLoc.getParamIdx(), AnyFunctionType::Param(eltTy)});
+          }
+
+          return recordShapeFix(
+              AddMissingArguments::create(*this, synthesizedArgs, argListLoc),
+              /*impact=*/2 * synthesizedArgs.size());
+        } else {
+          auto argIdx = argLoc.getArgIdx() + numParams;
+          SmallVector<std::pair<unsigned, AnyFunctionType::Param>, 4>
+              extraneousArgs;
+
+          for (unsigned i = 0, n = numArgs - numParams; i != n; ++i) {
+            extraneousArgs.push_back(
+                {argIdx + i, AnyFunctionType::Param(ctx.TheEmptyTupleType)});
+          }
+
+          auto overload = findSelectedOverloadFor(getCalleeLocator(argListLoc));
+          if (!overload)
+            return SolutionKind::Error;
+
+          return recordShapeFix(
+              RemoveExtraneousArguments::create(
+                  *this, overload->openedType->castTo<FunctionType>(),
+                  extraneousArgs, argListLoc),
+              /*impact=*/2 * extraneousArgs.size());
+        }
+      }
+    }
+
     unsigned impact = 1;
     if (locator.endsWith<LocatorPathElt::AnyRequirement>())
       impact = assessRequirementFailureImpact(*this, shape1, locator);
 
-    auto *fix = SkipSameShapeRequirement::create(*this, type1, type2,
-                                                 getConstraintLocator(locator));
-    return recordFix(fix, impact) ? SolutionKind::Error : SolutionKind::Solved;
+    return recordShapeFix(
+        SkipSameShapeRequirement::create(*this, type1, type2,
+                                         getConstraintLocator(locator)),
+        impact);
   }
 
   return SolutionKind::Error;

test/Constraints/pack-expansion-expressions.swift

@@ -332,8 +332,8 @@ func test_pack_expansions_with_closures() {
 // rdar://107151854 - crash on invalid due to specialized pack expansion
 func test_pack_expansion_specialization() {
   struct Data<each T> {
-    init(_: repeat each T) {} // expected-note 2 {{'init(_:)' declared here}}
-    init(vals: repeat each T) {} // expected-note 2 {{'init(vals:)' declared here}}
+    init(_: repeat each T) {} // expected-note 3 {{'init(_:)' declared here}}
+    init(vals: repeat each T) {} // expected-note {{'init(vals:)' declared here}}
   }
 
   _ = Data<Int>() // expected-error {{missing argument for parameter #1 in call}}

test/Constraints/variadic_generic_constraints.swift

@@ -58,14 +58,14 @@ takesParallelSequences(t: Array<String>(), Set<Int>(), u: Array<Int>(), Set<Stri
 // Same-shape requirements
 
 func zip<each T, each U>(t: repeat each T, u: repeat each U) -> (repeat (each T, each U)) {}
+// expected-note@-1 {{'zip(t:u:)' declared here}}
 
 let _ = zip()  // ok
 let _ = zip(t: 1, u: "hi")  // ok
 let _ = zip(t: 1, 2, u: "hi", "hello")  // ok
 let _ = zip(t: 1, 2, 3, u: "hi", "hello", "greetings")  // ok
-
-// FIXME: Bad diagnostic
-let _ = zip(t: 1, u: "hi", "hello", "greetings")  // expected-error {{type of expression is ambiguous without more context}}
+let _ = zip(t: 1, u: "hi", "hello", "greetings")  // expected-error {{extra arguments at positions #3, #4 in call}}
+// expected-error@-1 {{global function 'zip(t:u:)' requires the type packs 'Pack{Int}' and 'Pack{String, String, String}' have the same shape}}
 
 func goodCallToZip<each T, each U>(t: repeat each T, u: repeat each U) where (repeat (each T, each U)): Any {
   _ = zip(t: repeat each t, u: repeat each u)