[CS] Improve diagnostics a bit for pattern mismatch

There's still plenty of more work to do here for
pattern diagnostics, including introducing a
bunch of new locator elements, and handling things
like argument list mismatches. This at least lets
us fall back to a generic mismatch diagnostic.

Author: hamishknight

include/swift/Sema/ConstraintLocator.h

@@ -318,6 +318,9 @@ class ConstraintLocator : public llvm::FoldingSetNode {
   /// otherwise \c nullptr.
   NullablePtr<Pattern> getPatternMatch() const;
 
+  /// Whether the locator in question is for a pattern match.
+  bool isForPatternMatch() const;
+
   /// Returns true if \p locator is ending with either of the following
   ///  - Member
   ///  - Member -> KeyPathDynamicMember

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -225,6 +225,10 @@ CUSTOM_LOCATOR_PATH_ELT(TernaryBranch)
 /// Performing a pattern patch.
 CUSTOM_LOCATOR_PATH_ELT(PatternMatch)
 
+/// The constraint that models the allowed implicit casts for
+/// an EnumElementPattern.
+SIMPLE_LOCATOR_PATH_ELT(EnumPatternImplicitCastMatch)
+
 /// Points to a particular attribute associated with one of
 /// the arguments e.g. `inout` or its type e.g. `@escaping`.
 ///

lib/Sema/CSDiagnostics.cpp

@@ -2715,6 +2715,19 @@ bool ContextualFailure::diagnoseAsError() {
     return false;
   }
 
+  case ConstraintLocator::EnumPatternImplicitCastMatch: {
+    // In this case, the types are reversed, as we are checking whether we
+    // can convert the pattern type to the context type.
+    std::swap(fromType, toType);
+    diagnostic = diag::cannot_match_value_with_pattern;
+    break;
+  }
+
+  case ConstraintLocator::PatternMatch: {
+    diagnostic = diag::cannot_match_value_with_pattern;
+    break;
+  }
+
   default:
     return false;
   }
@@ -2927,9 +2940,11 @@ bool ContextualFailure::diagnoseConversionToNil() const {
 void ContextualFailure::tryFixIts(InFlightDiagnostic &diagnostic) const {
   auto *locator = getLocator();
   // Can't apply any of the fix-its below if this failure
-  // is related to `inout` argument.
-  if (locator->isLastElement<LocatorPathElt::LValueConversion>())
+  // is related to `inout` argument, or a pattern mismatch.
+  if (locator->isLastElement<LocatorPathElt::LValueConversion>() ||
+      locator->isForPatternMatch()) {
     return;
+  }
 
   if (trySequenceSubsequenceFixIts(diagnostic))
     return;

lib/Sema/CSGen.cpp

@@ -2783,6 +2783,9 @@ namespace {
         if (dyn_cast_or_null<TuplePattern>(enumPattern->getSubPattern()))
           functionRefKind = FunctionRefKind::Compound;
 
+        auto patternLocator =
+            locator.withPathElement(LocatorPathElt::PatternMatch(pattern));
+
         if (enumPattern->getParentType() || enumPattern->getParentTypeRepr()) {
           // Resolve the parent type.
           const auto parentType = [&] {
@@ -2810,25 +2813,23 @@ namespace {
 
           // Perform member lookup into the parent's metatype.
           Type parentMetaType = MetatypeType::get(parentType);
-          CS.addValueMemberConstraint(
-              parentMetaType, enumPattern->getName(), memberType, CurDC,
-              functionRefKind, {},
-              CS.getConstraintLocator(locator,
-                                      LocatorPathElt::PatternMatch(pattern)));
+          CS.addValueMemberConstraint(parentMetaType, enumPattern->getName(),
+                                      memberType, CurDC, functionRefKind, {},
+                                      patternLocator);
 
           // Parent type needs to be convertible to the pattern type; this
           // accounts for cases where the pattern type is existential.
           CS.addConstraint(
               ConstraintKind::Conversion, parentType, patternType,
-              locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
+              patternLocator.withPathElement(
+                  ConstraintLocator::EnumPatternImplicitCastMatch));
 
           baseType = parentType;
         } else {
           // Use the pattern type for member lookup.
           CS.addUnresolvedValueMemberConstraint(
               MetatypeType::get(patternType), enumPattern->getName(),
-              memberType, CurDC, functionRefKind,
-              locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
+              memberType, CurDC, functionRefKind, patternLocator);
 
           baseType = patternType;
         }
@@ -2865,13 +2866,11 @@ namespace {
           // NOTE: The order here is important! Pattern matching equality is
           // not symmetric (we need to fix that either by using a different
           // constraint, or actually making it symmetric).
-          CS.addConstraint(
-              ConstraintKind::Equal, functionType, memberType,
-              locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
+          CS.addConstraint(ConstraintKind::Equal, functionType, memberType,
+                           patternLocator);
 
-          CS.addConstraint(
-              ConstraintKind::Conversion, outputType, baseType,
-              locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
+          CS.addConstraint(ConstraintKind::Conversion, outputType, baseType,
+                           patternLocator);
         }
 
         return setType(patternType);

lib/Sema/CSSimplify.cpp

@@ -6360,8 +6360,20 @@ bool ConstraintSystem::repairFailures(
     break;
   }
 
+  case ConstraintLocator::EnumPatternImplicitCastMatch: {
+    // If either type is a placeholder, consider this fixed.
+    if (lhs->isPlaceholder() || rhs->isPlaceholder())
+      return true;
+
+    conversionsOrFixes.push_back(ContextualMismatch::create(
+        *this, lhs, rhs, getConstraintLocator(locator)));
+    break;
+  }
+
   case ConstraintLocator::PatternMatch: {
     auto *pattern = elt.castTo<LocatorPathElt::PatternMatch>().getPattern();
+
+    // TODO: We ought to introduce a new locator element for this.
     bool isMemberMatch =
         lhs->is<FunctionType>() && isa<EnumElementPattern>(pattern);
 
@@ -6376,13 +6388,12 @@ bool ConstraintSystem::repairFailures(
     if (lhs->isPlaceholder() || rhs->isPlaceholder())
       return true;
 
-    // If member reference didn't match expected pattern,
-    // let's consider that a contextual mismatch.
     if (isMemberMatch) {
       recordAnyTypeVarAsPotentialHole(lhs);
       recordAnyTypeVarAsPotentialHole(rhs);
       conversionsOrFixes.push_back(AllowAssociatedValueMismatch::create(
           *this, lhs, rhs, getConstraintLocator(locator)));
+      break;
     }
 
     // `weak` declaration with an explicit non-optional type e.g.
@@ -6404,6 +6415,9 @@ bool ConstraintSystem::repairFailures(
       }
     }
 
+    conversionsOrFixes.push_back(ContextualMismatch::create(
+        *this, lhs, rhs, getConstraintLocator(locator)));
+
     break;
   }
 

lib/Sema/ConstraintLocator.cpp

@@ -90,6 +90,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
   case ConstraintLocator::ImplicitCallAsFunction:
   case ConstraintLocator::TernaryBranch:
   case ConstraintLocator::PatternMatch:
+  case ConstraintLocator::EnumPatternImplicitCastMatch:
   case ConstraintLocator::ArgumentAttribute:
   case ConstraintLocator::UnresolvedMemberChainResult:
   case ConstraintLocator::PlaceholderType:
@@ -403,6 +404,10 @@ void LocatorPathElt::dump(raw_ostream &out) const {
     out << "pattern match";
     break;
 
+  case ConstraintLocator::EnumPatternImplicitCastMatch:
+    out << "enum pattern implicit cast match";
+    break;
+
   case ConstraintLocator::ArgumentAttribute: {
     using AttrLoc = LocatorPathElt::ArgumentAttribute;
 
@@ -677,6 +682,10 @@ NullablePtr<Pattern> ConstraintLocator::getPatternMatch() const {
   return matchElt->getPattern();
 }
 
+bool ConstraintLocator::isForPatternMatch() const {
+  return getPatternMatch() != nullptr;
+}
+
 bool ConstraintLocator::isMemberRef() const {
   if (isLastElement<LocatorPathElt::Member>()) {
     return true;

lib/Sema/ConstraintSystem.cpp

@@ -5824,6 +5824,11 @@ void constraints::simplifyLocator(ASTNode &anchor,
       continue;
     }
 
+    case ConstraintLocator::EnumPatternImplicitCastMatch: {
+      path = path.slice(1);
+      continue;
+    }
+
     case ConstraintLocator::PackType:
     case ConstraintLocator::ParentType:
     case ConstraintLocator::KeyPathType:

test/Constraints/rdar107709341.swift

@@ -2,10 +2,9 @@
 
 // rdar://107709341 – Make sure we don't crash.
 func foo(_ x: Int) {
-  // FIXME: We ought to have a better diagnostic
-  let _ = { // expected-error {{unable to infer closure type in the current context}}
+  let _ = {
     switch x {
-    case Optional<Int>.some(x):
+    case Optional<Int>.some(x): // expected-error {{pattern of type 'Optional<Int>' cannot match 'Int'}} {{none}}
       break
     default:
       break