[CS] Disfavor solutions that were unable to infer the variable type inside a named pattern

Author: ahoppen

include/swift/AST/DiagnosticsSema.def

@@ -4242,6 +4242,8 @@ ERROR(iflet_implicitly_unwraps,none,
       "pattern matching in a condition implicitly unwraps optionals", ())
 ERROR(type_pattern_missing_is,none,
       "'is' keyword required to pattern match against type name", ())
+ERROR(cannot_infer_type_for_variable,none,
+      "could not infer type of variable %0", (DeclBaseName))
 
 
 ERROR(pattern_type_mismatch_context,none,

include/swift/Sema/CSFix.h

@@ -301,6 +301,12 @@ enum class FixKind : uint8_t {
   /// Resolve type of `nil` by providing a contextual type.
   SpecifyContextualTypeForNil,
 
+  /// Explicitly specify the type of a named pattern because we were unable to
+  /// infer it from the expression.
+  /// Used as a last resort fix with very high impact that should cause fixes
+  /// inside the expression of the pattern binding decl to be preferred.
+  SpecifyPatternType,
+
   /// Allow expressions to reference invalid declarations by turning
   /// them into holes.
   AllowRefToInvalidDecl,
@@ -2766,6 +2772,34 @@ class SpecifyContextualTypeForNil final : public ConstraintFix {
   }
 };
 
+class SpecifyPatternType final : public ConstraintFix {
+  /// The pattern for which we failed to resolve the variable's type.
+  NamedPattern *pattern;
+
+  SpecifyPatternType(ConstraintSystem &cs, NamedPattern *pattern,
+                     ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::SpecifyContextualTypeForNil, locator),
+        pattern(pattern) {}
+
+public:
+  std::string getName() const override {
+    return "specify type for pattern match";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static SpecifyPatternType *create(ConstraintSystem &cs, NamedPattern *pattern,
+                                    ConstraintLocator *locator);
+
+  static bool classof(ConstraintFix *fix) {
+    return fix->getKind() == FixKind::SpecifyPatternType;
+  }
+};
+
 class SpecifyTypeForPlaceholder final : public ConstraintFix {
   SpecifyTypeForPlaceholder(ConstraintSystem &cs, ConstraintLocator *locator)
       : ConstraintFix(cs, FixKind::SpecifyTypeForPlaceholder, locator) {}

lib/Sema/CSBindings.cpp

@@ -1989,6 +1989,20 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const {
     return std::make_pair(fix, /*impact=*/(unsigned)10);
   }
 
+  if (auto pattern = dyn_cast_or_null<NamedPattern>(
+          dstLocator->getAnchor().dyn_cast<Pattern *>())) {
+    if (dstLocator->getPath().size() == 1 &&
+        dstLocator->isLastElement<LocatorPathElt::NamedPatternDecl>()) {
+      // Not being able to infer the type of a variable in a pattern binding
+      // decl is more dramatic than anything that could happen inside the
+      // expression because we want to preferrably point the diagnostic to a
+      // part of the expression that caused us to be unable to infer the
+      // variable's type.
+      ConstraintFix *fix = SpecifyPatternType::create(cs, pattern, dstLocator);
+      return std::make_pair(fix, /*impact=*/(unsigned)20);
+    }
+  }
+
   return None;
 }
 

lib/Sema/CSDiagnostics.cpp

@@ -7811,6 +7811,12 @@ bool MissingContextualTypeForNil::diagnoseAsError() {
   return true;
 }
 
+bool CouldNotInferPatternType::diagnoseAsError() {
+  emitDiagnostic(diag::cannot_infer_type_for_variable,
+                 pattern->getDecl()->getBaseName());
+  return true;
+}
+
 bool CouldNotInferPlaceholderType::diagnoseAsError() {
   // If this placeholder was explicitly written out by the user, they can maybe
   // fix things by specifying an actual type.

lib/Sema/CSDiagnostics.h

@@ -2450,6 +2450,18 @@ class MissingContextualTypeForNil final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+class CouldNotInferPatternType final : public FailureDiagnostic {
+  /// The pattern for which we failed to resolve the variable's type.
+  NamedPattern *pattern;
+
+public:
+  CouldNotInferPatternType(const Solution &solution, NamedPattern *pattern,
+                           ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), pattern(pattern) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose situations where there is no context to determine the type of a
 /// placeholder, e.g.,
 ///

lib/Sema/CSFix.cpp

@@ -1967,6 +1967,17 @@ SpecifyContextualTypeForNil::create(ConstraintSystem &cs,
   return new (cs.getAllocator()) SpecifyContextualTypeForNil(cs, locator);
 }
 
+bool SpecifyPatternType::diagnose(const Solution &solution, bool asNote) const {
+  CouldNotInferPatternType failure(solution, pattern, getLocator());
+  return failure.diagnose(asNote);
+}
+
+SpecifyPatternType *SpecifyPatternType::create(ConstraintSystem &cs,
+                                               NamedPattern *pattern,
+                                               ConstraintLocator *locator) {
+  return new (cs.getAllocator()) SpecifyPatternType(cs, pattern, locator);
+}
+
 bool SpecifyTypeForPlaceholder::diagnose(const Solution &solution,
                                            bool asNote) const {
   CouldNotInferPlaceholderType failure(solution, getLocator());

lib/Sema/CSSimplify.cpp

@@ -12921,6 +12921,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::IgnoreInvalidResultBuilderBody:
   case FixKind::IgnoreResultBuilderWithReturnStmts:
   case FixKind::SpecifyContextualTypeForNil:
+  case FixKind::SpecifyPatternType:
   case FixKind::AllowRefToInvalidDecl:
   case FixKind::SpecifyBaseTypeForOptionalUnresolvedMember:
   case FixKind::AllowCheckedCastCoercibleOptionalType:

lib/Sema/ConstraintLocator.cpp

@@ -598,6 +598,12 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
       out << '@';
       expr->getLoc().print(out, *sm);
     }
+  } else if (auto *pattern = anchor.dyn_cast<Pattern *>()) {
+    out << Pattern::getKindName(pattern->getKind()) << "Pattern";
+    if (sm) {
+      out << '@';
+      pattern->getLoc().print(out, *sm);
+    }
   }
 
   for (auto elt : getPath()) {

test/ClangImporter/MixedSource/can_import_objc_idempotent.swift

@@ -31,6 +31,8 @@
 
   let (r, s) = square.divided(atDistance: 50, from: .minXEdge)
   // expected-error@-1 {{cannot infer contextual base in reference to member 'minXEdge'}}
+  // expected-error@-2 {{could not infer type of variable 'r'}}
+  // expected-error@-3 {{could not infer type of variable 's'}}
 #endif
 
 #if canImport(MixedWithHeader)

test/NameLookup/name_lookup2.swift

@@ -55,7 +55,9 @@ func test_varname_binding() {
   var ((), (g1, g2), h) = ((), (e, d), e)
   var (j, k, l) = callee1()
   var (m, n) = callee1() // expected-error{{'(Int, Int, Int)' is not convertible to '(Int, Int)', tuples have a different number of elements}}
-  var (o, p, q, r) = callee1() // expected-error{{'(Int, Int, Int)' is not convertible to '(Int, Int, Int, _)', tuples have a different number of elements}}
+  var (o, p, q, r) = callee1()
+  // expected-error@-1 {{'(Int, Int, Int)' is not convertible to '(Int, Int, Int, _)', tuples have a different number of elements}}
+  // expected-error@-2 {{could not infer type of variable 'r'}}
 }
 
 //===----------------------------------------------------------------------===//

test/Parse/enum_element_pattern_swift4.swift

@@ -43,6 +43,7 @@ func testE(e: E) {
                    // expected-note@-1 {{remove associated values to make the pattern match}} {{12-14=}}
     case .D(let payload) = e // expected-error {{pattern with associated values does not match enum case 'D'}}
                              // expected-note@-1 {{remove associated values to make the pattern match}} {{12-25=}}
+                             // expected-error@-2 {{could not infer type of variable 'payload'}}
   else { return }
 }
 

test/Parse/matching_patterns.swift

@@ -116,8 +116,12 @@ enum Voluntary<T> : Equatable {
 var n : Voluntary<Int> = .Naught
 if case let .Naught(value) = n {} // expected-error{{pattern with associated values does not match enum case 'Naught'}}
                                   // expected-note@-1 {{remove associated values to make the pattern match}} {{20-27=}}
+                                  // expected-error@-2 {{could not infer type of variable 'value'}}
 if case let .Naught(value1, value2, value3) = n {} // expected-error{{pattern with associated values does not match enum case 'Naught'}}
                                                    // expected-note@-1 {{remove associated values to make the pattern match}} {{20-44=}}
+                                                   // expected-error@-2 {{could not infer type of variable 'value1'}}
+                                                   // expected-error@-3 {{could not infer type of variable 'value2'}}
+                                                   // expected-error@-4 {{could not infer type of variable 'value3'}}
 
 
 
@@ -142,7 +146,7 @@ case Voluntary<Int>.Mere,
   ()
 case .Twain,
      .Twain(_), // expected-warning {{enum case 'Twain' has 2 associated values; matching them as a tuple is deprecated}}
-                // expected-note@-69 {{'Twain' declared here}}
+                // expected-note@-73 {{'Twain' declared here}}
      .Twain(_, _),
      .Twain(_, _, _): // expected-error{{tuple pattern has the wrong length for tuple type '(Int, Int)'}}
   ()
@@ -330,3 +334,4 @@ let (responseObject: Int?) = op1
 // expected-error @-1 {{expected ',' separator}} {{25-25=,}}
 // expected-error @-2 {{expected pattern}}
 // expected-error @-3 {{cannot convert value of type 'Int?' to specified type '(responseObject: _)'}}
+// expected-error @-4 {{could not infer type of variable 'Int'}}

test/expr/expressions.swift

@@ -157,6 +157,7 @@ func errorRecovery() {
 
   // <rdar://problem/22426860> CrashTracer: [USER] swift at …mous_namespace::ConstraintGenerator::getTypeForPattern + 698
   var (g1, g2, g3) = (1, 2) // expected-error {{'(Int, Int)' is not convertible to '(Int, Int, _)', tuples have a different number of elements}}
+                            // expected-error@-1 {{could not infer type of variable 'g3'}}
   var (h1, h2) = (1, 2, 3) // expected-error {{'(Int, Int, Int)' is not convertible to '(Int, Int)', tuples have a different number of elements}}
   var i: (Bool, Bool) = makeTuple() // expected-error {{cannot convert value of type '(String, Int)' to specified type '(Bool, Bool)'}}
 }

test/stmt/foreach.swift

@@ -231,9 +231,13 @@ func testRepeated(ri: RepeatedSequence<Int>) {
 func sr_12398(arr1: [Int], arr2: [(a: Int, b: String)]) {
   for (x, y) in arr1 {}
   // expected-error@-1 {{tuple pattern cannot match values of non-tuple type 'Int'}}
+  // expected-error@-2 {{could not infer type of variable 'x'}}
+  // expected-error@-3 {{could not infer type of variable 'y'}}
 
   for (x, y, _) in arr2 {}
   // expected-error@-1 {{pattern cannot match values of type '(a: Int, b: String)'}}
+  // expected-error@-2 {{could not infer type of variable 'x'}}
+  // expected-error@-3 {{could not infer type of variable 'y'}}
 }
 
 // rdar://62339835

test/stmt/if_while_var.swift

@@ -51,7 +51,9 @@ guard let _ = nonOptionalEnum() else { fatalError() } // expected-error{{initial
 let optional: String? = nil
 if case let optional? { _ = optional } // expected-error{{variable binding in a condition requires an initializer}}
 if case let .some(optional) { _ = optional } // expected-error{{variable binding in a condition requires an initializer}}
+                                             // expected-error@-1 {{could not infer type of variable 'optional'}}
 if case .some(let optional) { _ = optional } // expected-error{{variable binding in a condition requires an initializer}}
+                                             // expected-error@-1 {{could not infer type of variable 'optional'}}
 
 if case let x? = nonOptionalStruct() { _ = x } // expected-error{{'?' pattern cannot match values of type 'NonOptionalStruct'}}
 if case let x? = nonOptionalEnum() { _ = x } // expected-error{{'?' pattern cannot match values of type 'NonOptionalEnum'}}