Sema: Refactor logic for excluding arrays / optionals out of closure

Author: sl

lib/Sema/CSFix.h

@@ -167,7 +167,8 @@ enum class FixKind : uint8_t {
   /// associated with single declaration.
   ExplicitlySpecifyGenericArguments,
 
-  /// Skip any unhandled constructs that occur within a closure argument that matches up with a
+  /// Skip any unhandled constructs that occur within a closure argument that
+  /// matches up with a
   /// parameter that has a function builder.
   SkipUnhandledConstructInFunctionBuilder,
 };

lib/Sema/CSSimplify.cpp

@@ -1598,23 +1598,24 @@ static ConstraintSystem::TypeMatchResult matchDeepTypeArguments(
     ConstraintSystem &cs, ConstraintSystem::TypeMatchOptions subflags,
     ArrayRef<Type> args1, ArrayRef<Type> args2,
     ConstraintLocatorBuilder locator,
-    llvm::function_ref<bool(unsigned)> recordMismatch = [](unsigned) { return false; }) {
+    llvm::function_ref<void(unsigned)> recordMismatch = [](unsigned) {}) {
   if (args1.size() != args2.size()) {
     return cs.getTypeMatchFailure(locator);
   }
 
+  auto allMatch = cs.getTypeMatchSuccess();
   for (unsigned i = 0, n = args1.size(); i != n; ++i) {
     auto result = cs.matchTypes(
         args1[i], args2[i], ConstraintKind::Bind, subflags,
         locator.withPathElement(LocatorPathElt::getGenericArgument(i)));
 
     if (result.isFailure()) {
-      if (!cs.shouldAttemptFixes() || !recordMismatch(i))
-        return result;
+      recordMismatch(i);
+      allMatch = result;
     }
   }
 
-  return cs.getTypeMatchSuccess();
+  return allMatch;
 }
 
 ConstraintSystem::TypeMatchResult
@@ -1670,46 +1671,48 @@ ConstraintSystem::matchDeepEqualityTypes(Type type1, Type type2,
       return result;
   }
 
-  bool forRequirement = false;
-  if (auto last = locator.last()) {
-    forRequirement =
-        last->isTypeParameterRequirement() || last->isConditionalRequirement();
-  }
-  // Optionals and arrays have a lot of special diagnostics and only one
-  // generic argument so if we' re dealing with one, don't produce generic
-  // arguments mismatch fixes.
-  // TODO(diagnostics): Move Optional and Array diagnostics over to the
-  // new framework.
-  bool isOptional = bound1->getDecl()->isOptionalDecl();
-  bool isArray = isArrayType(bound1).hasValue();
-
   auto args1 = bound1->getGenericArgs();
   auto args2 = bound2->getGenericArgs();
 
-  SmallVector<unsigned, 4> failedArguments;
   // Match up the generic arguments, exactly.
-  auto result =
-      matchDeepTypeArguments(*this, subflags, args1, args2, locator,
-                             [&](unsigned position) {
-                                if (forRequirement || isOptional || isArray)
-                                  return false;
-                                
-                                failedArguments.push_back(position);
-                                return true;
-                             });
-  if (!failedArguments.empty()) {
-    auto *fix = GenericArgumentsMismatch::create(
-        *this, bound1, bound2, failedArguments, getConstraintLocator(locator));
-    // Increase the solution's score for each mismtach this fixes.
-    for (unsigned i = 0; i < failedArguments.size(); ++i) {
-      increaseScore(SK_Fix);
+
+  if (shouldAttemptFixes()) {
+    bool forRequirement = false;
+    if (auto last = locator.last()) {
+      forRequirement = last->isTypeParameterRequirement() ||
+                       last->isConditionalRequirement();
     }
 
+    // Optionals and arrays have a lot of special diagnostics and only one
+    // generic argument so if we' re dealing with one, don't produce generic
+    // arguments mismatch fixes.
+    // TODO(diagnostics): Move Optional and Array diagnostics over to the
+    // new framework.
+    bool isOptional = bound1->getDecl()->isOptionalDecl();
+    bool isArray = isArrayType(bound1).hasValue();
+
+    if (forRequirement || isOptional || isArray)
+      return matchDeepTypeArguments(*this, subflags, args1, args2, locator);
+
+    SmallVector<unsigned, 4> mismatches;
+    auto result = matchDeepTypeArguments(
+        *this, subflags, args1, args2, locator,
+        [&mismatches](unsigned position) { mismatches.push_back(position); });
+
+    if (mismatches.empty())
+      return result;
+
+    auto *fix = GenericArgumentsMismatch::create(
+        *this, bound1, bound2, mismatches, getConstraintLocator(locator));
+
     if (!recordFix(fix)) {
+      // Increase the solution's score for each mismtach this fixes.
+      increaseScore(SK_Fix, mismatches.size());
       return getTypeMatchSuccess();
     }
+    return result;
   }
-  return result;
+  return matchDeepTypeArguments(*this, subflags, args1, args2, locator);
 }
 
 ConstraintSystem::TypeMatchResult
@@ -2150,12 +2153,12 @@ bool ConstraintSystem::repairFailures(
   }
 
   auto hasConversionOrRestriction = [&](ConversionRestrictionKind kind) {
-    return llvm::any_of(
-        conversionsOrFixes, [kind](const RestrictionOrFix correction) {
-          if (auto restriction = correction.getRestriction())
-            return restriction == kind;
-          return false;
-        });
+    return llvm::any_of(conversionsOrFixes,
+                        [kind](const RestrictionOrFix correction) {
+                          if (auto restriction = correction.getRestriction())
+                            return restriction == kind;
+                          return false;
+                        });
   };
 
   auto &elt = path.back();

test/decl/protocol/protocol_with_superclass.swift

@@ -43,7 +43,7 @@ extension ProtoRefinesClass {
     let _: BaseProto & Generic<Int> = self
     let _: BaseProto & Concrete = self
 
-    let _: Generic<String> = self;
+    let _: Generic<String> = self
     // expected-error@-1 {{cannot assign value of type 'Generic<Int>' to type 'Generic<String>'}}
   }
 }