Merge pull request swiftlang#30006 from xedin/rdar-56212087

[ConstraintSystem] Make it possible to infer subtype bindings through argument conversions

Author: xedin

lib/Sema/CSBindings.cpp

@@ -30,14 +30,17 @@ void ConstraintSystem::inferTransitiveSupertypeBindings(
   llvm::SmallVector<Constraint *, 4> subtypeOf;
   // First, let's collect all of the `subtype` constraints associated
   // with this type variable.
-  llvm::copy_if(bindings.Sources, std::back_inserter(subtypeOf),
-                [&](const Constraint *constraint) -> bool {
-                  if (constraint->getKind() != ConstraintKind::Subtype)
-                    return false;
-
-                  auto rhs = simplifyType(constraint->getSecondType());
-                  return rhs->getAs<TypeVariableType>() == typeVar;
-                });
+  llvm::copy_if(
+      bindings.Sources, std::back_inserter(subtypeOf),
+      [&](const Constraint *constraint) -> bool {
+        if (constraint->getKind() != ConstraintKind::Subtype &&
+            constraint->getKind() != ConstraintKind::ArgumentConversion &&
+            constraint->getKind() != ConstraintKind::OperatorArgumentConversion)
+          return false;
+
+        auto rhs = simplifyType(constraint->getSecondType());
+        return rhs->getAs<TypeVariableType>() == typeVar;
+      });
 
   if (subtypeOf.empty())
     return;
@@ -618,7 +621,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) const {
           continue;
 
         literalBindings.push_back(
-            {defaultType, AllowedBindingKind::Subtypes, constraint});
+            {defaultType, AllowedBindingKind::Exact, constraint});
         continue;
       }
 
@@ -644,7 +647,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) const {
       if (!matched) {
         exactTypes.insert(defaultType->getCanonicalType());
         literalBindings.push_back(
-            {defaultType, AllowedBindingKind::Subtypes, constraint});
+            {defaultType, AllowedBindingKind::Exact, constraint});
       }
 
       break;

lib/Sema/CSSimplify.cpp

@@ -9103,7 +9103,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     // subscript, which requires changes to declaration to become mutable.
     if (auto last = locator.last()) {
       impact += (last->is<LocatorPathElt::FunctionResult>() ||
-                 last->is<LocatorPathElt::SubscriptMember>())
+                 last->is<LocatorPathElt::SubscriptMember>() ||
+                 last->is<LocatorPathElt::KeyPathDynamicMember>())
                     ? 1
                     : 0;
     }

lib/Sema/CSStep.h

@@ -620,6 +620,15 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
   /// Check whether attempting type variable binding choices should
   /// be stopped, because optimal solution has already been found.
   bool shouldStopAt(const TypeVariableBinding &choice) const override {
+    if (CS.shouldAttemptFixes()) {
+      // Let's always attempt default types inferred from literals
+      // in diagnostic mode because that could lead to better
+      // diagnostics if the problem is contextual like argument/parameter
+      // conversion or collection element mismatch.
+      if (choice.hasDefaultedProtocol())
+        return false;
+    }
+
     // If we were able to solve this without considering
     // default literals, don't bother looking at default literals.
     return AnySolved && choice.hasDefaultedProtocol() &&

lib/Sema/ConstraintSystem.cpp

@@ -2778,45 +2778,61 @@ static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs,
 
   auto &DE = cs.getASTContext().Diags;
 
-  llvm::SmallDenseMap<TypeVariableType *, SmallVector<Type, 4>> conflicts;
+  llvm::SmallDenseMap<TypeVariableType *,
+                      std::pair<GenericTypeParamType *, SourceLoc>, 4>
+      genericParams;
+  // Consider only representative type variables shared across
+  // all of the solutions.
+  for (auto *typeVar : cs.getTypeVariables()) {
+    if (auto *GP = typeVar->getImpl().getGenericParameter()) {
+      auto *locator = typeVar->getImpl().getLocator();
+      auto *repr = cs.getRepresentative(typeVar);
+      // If representative is another generic parameter let's
+      // use its generic parameter type instead of originator's,
+      // but it's possible that generic parameter is equated to
+      // some other type e.g.
+      //
+      // func foo<T>(_: T) -> T {}
+      //
+      // In this case when reference to function `foo` is "opened"
+      // type variable representing `T` would be equated to
+      // type variable representing a result type of the reference.
+      if (auto *reprGP = repr->getImpl().getGenericParameter())
+        GP = reprGP;
+
+      genericParams[repr] = {GP, locator->getAnchor()->getLoc()};
+    }
+  }
 
-  for (const auto &binding : solutions[0].typeBindings) {
-    auto *typeVar = binding.first;
+  llvm::SmallDenseMap<std::pair<GenericTypeParamType *, SourceLoc>,
+                      SmallVector<Type, 4>>
+      conflicts;
 
-    if (!typeVar->getImpl().getGenericParameter())
-      continue;
+  for (const auto &entry : genericParams) {
+    auto *typeVar = entry.first;
+    auto GP = entry.second;
 
     llvm::SmallSetVector<Type, 4> arguments;
-    arguments.insert(binding.second);
-
-    if (!llvm::all_of(solutions.slice(1), [&](const Solution &solution) {
-          auto binding = solution.typeBindings.find(typeVar);
-          if (binding == solution.typeBindings.end())
-            return false;
-
-          // Contextual opaque result type is uniquely identified by
-          // declaration it's associated with, so we have to compare
-          // declarations instead of using pointer equality on such types.
-          if (auto *opaque =
-                  binding->second->getAs<OpaqueTypeArchetypeType>()) {
-            auto *decl = opaque->getDecl();
-            arguments.remove_if([&](Type argType) -> bool {
-              if (auto *otherOpaque =
-                      argType->getAs<OpaqueTypeArchetypeType>()) {
-                return decl == otherOpaque->getDecl();
-              }
-              return false;
-            });
+    for (const auto &solution : solutions) {
+      auto type = solution.typeBindings.lookup(typeVar);
+      // Contextual opaque result type is uniquely identified by
+      // declaration it's associated with, so we have to compare
+      // declarations instead of using pointer equality on such types.
+      if (auto *opaque = type->getAs<OpaqueTypeArchetypeType>()) {
+        auto *decl = opaque->getDecl();
+        arguments.remove_if([&](Type argType) -> bool {
+          if (auto *otherOpaque = argType->getAs<OpaqueTypeArchetypeType>()) {
+            return decl == otherOpaque->getDecl();
           }
+          return false;
+        });
+      }
 
-          arguments.insert(binding->second);
-          return true;
-        }))
-      continue;
-
-    if (arguments.size() > 1) {
-      conflicts[typeVar].append(arguments.begin(), arguments.end());
+      arguments.insert(type);
     }
+
+    if (arguments.size() > 1)
+      conflicts[GP].append(arguments.begin(), arguments.end());
   }
 
   auto getGenericTypeDecl = [&](ArchetypeType *archetype) -> ValueDecl * {
@@ -2833,8 +2849,10 @@ static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs,
 
   bool diagnosed = false;
   for (auto &conflict : conflicts) {
-    auto *typeVar = conflict.first;
-    auto *locator = typeVar->getImpl().getLocator();
+    SourceLoc loc;
+    GenericTypeParamType *GP;
+
+    std::tie(GP, loc) = conflict.first;
     auto conflictingArguments = conflict.second;
 
     llvm::SmallString<64> arguments;
@@ -2859,10 +2877,8 @@ static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs,
         },
         [&OS] { OS << " vs. "; });
 
-    auto *anchor = locator->getAnchor();
-    DE.diagnose(anchor->getLoc(),
-                diag::conflicting_arguments_for_generic_parameter,
-                typeVar->getImpl().getGenericParameter(), OS.str());
+    DE.diagnose(loc, diag::conflicting_arguments_for_generic_parameter, GP,
+                OS.str());
     diagnosed = true;
   }
 

test/Constraints/array_literal.swift

@@ -50,7 +50,7 @@ useDoubleList([1.0,2,3])
 useDoubleList([1.0,2.0,3.0])
 
 useIntDict(["Niners" => 31, "Ravens" => 34])
-useIntDict(["Niners" => 31, "Ravens" => 34.0]) // expected-error{{cannot convert value of type 'Double' to expected argument type 'Int'}}
+useIntDict(["Niners" => 31, "Ravens" => 34.0]) // expected-error{{cannot convert value of type 'Double' to expected element type 'Int'}}
 // <rdar://problem/22333090> QoI: Propagate contextual information in a call to operands
 useDoubleDict(["Niners" => 31, "Ravens" => 34.0])
 useDoubleDict(["Niners" => 31.0, "Ravens" => 34])

test/Constraints/closures.swift

@@ -456,13 +456,7 @@ extension Collection {
   }
 }
 func fn_r28909024(n: Int) {
-  // FIXME(diagnostics): Unfortunately there is no easy way to fix this diagnostic issue at the moment
-  // because the problem is related to ordering of the bindings - we'd attempt to bind result of the expression
-  // to contextual type of `Void` which prevents solver from discovering correct types for range - 0..<10
-  // (since both arguments are literal they are ranked lower than contextual type).
-  //
-  // Good diagnostic for this is - `unexpected non-void return value in void function`
-  return (0..<10).r28909024 { // expected-error {{type of expression is ambiguous without more context}}
+  return (0..<10).r28909024 { // expected-error {{unexpected non-void return value in void function}} expected-note {{did you mean to add a return type?}}
     _ in true
   }
 }

test/Constraints/default_literals.swift

@@ -42,3 +42,14 @@ extension Int {
 
 
 var (div, mod) = (9 / 4, 9 % 4)
+
+// rdar://problem/56212087 - solver fails to infer correct type for a generic parameter (Any vs. String)
+func test_transitive_inference_of_default_literal_types() {
+  func foo<T: ExpressibleByStringLiteral>(_: String, _: T) -> T {
+    fatalError()
+  }
+
+  func bar(_: Any?) {}
+
+  bar(foo("", "")) // Ok
+}

test/Generics/deduction.swift

@@ -318,8 +318,9 @@ func foo() {
     let j = min(Int(3), Float(2.5)) // expected-error{{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}}
     let k = min(A(), A()) // expected-error{{global function 'min' requires that 'A' conform to 'Comparable'}}
     let oi : Int? = 5
-    let l = min(3, oi) // expected-error{{global function 'min' requires that 'Int?' conform to 'Comparable'}}
-  // expected-note@-1{{wrapped type 'Int' satisfies this requirement}}
+    let l = min(3, oi) // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
+    // expected-note@-1 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+    // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
 }
 
 infix operator +&

test/Parse/omit_return.swift

@@ -1005,7 +1005,7 @@ var fvs_stubMyOwnFatalError: () {
 var fvs_forceTryExplicit: String {
     get { "ok" }
     set {
-        return try! failableIdentity("shucks") // expected-error {{cannot convert value of type 'String' to expected argument type '()'}}
+        return try! failableIdentity("shucks") // expected-error {{unexpected non-void return value in void function}}
     }
 }
 

test/attr/attr_dynamic_member_lookup.swift

@@ -600,10 +600,7 @@ func keypath_with_subscripts(_ arr: SubscriptLens<[Int]>,
 
 func keypath_with_incorrect_return_type(_ arr: Lens<Array<Int>>) {
   for idx in 0..<arr.count {
-    // expected-error@-1 {{protocol 'Sequence' requires that 'Lens<Int>' conform to 'Strideable'}}
-    // expected-error@-2 {{protocol 'Sequence' requires that 'Lens<Int>.Stride' conform to 'SignedInteger'}}
-    // expected-error@-3 {{cannot convert value of type 'Int' to expected argument type 'Lens<Int>'}}
-    // expected-error@-4 {{referencing operator function '..<' on 'Comparable' requires that 'Lens<Int>' conform to 'Comparable'}}
+    // expected-error@-1 {{cannot convert value of type 'Lens<Int>' to expected argument type 'Int'}}
     let _ = arr[idx]
   }
 }