Merge pull request #30283 from xedin/rdar-60185506

Revert "[ConstraintSystem] Make it possible to infer subtype bindings…

Author: xedin

lib/Sema/CSBindings.cpp

@@ -30,17 +30,14 @@ 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 &&
-            constraint->getKind() != ConstraintKind::ArgumentConversion &&
-            constraint->getKind() != ConstraintKind::OperatorArgumentConversion)
-          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)
+                    return false;
+
+                  auto rhs = simplifyType(constraint->getSecondType());
+                  return rhs->getAs<TypeVariableType>() == typeVar;
+                });
 
   if (subtypeOf.empty())
     return;
@@ -635,7 +632,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) const {
           continue;
 
         literalBindings.push_back(
-            {defaultType, AllowedBindingKind::Exact, constraint});
+            {defaultType, AllowedBindingKind::Subtypes, constraint});
         continue;
       }
 
@@ -661,7 +658,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) const {
       if (!matched) {
         exactTypes.insert(defaultType->getCanonicalType());
         literalBindings.push_back(
-            {defaultType, AllowedBindingKind::Exact, constraint});
+            {defaultType, AllowedBindingKind::Subtypes, constraint});
       }
 
       break;

lib/Sema/CSSimplify.cpp

@@ -9221,8 +9221,7 @@ 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::KeyPathDynamicMember>())
+                 last->is<LocatorPathElt::SubscriptMember>())
                     ? 1
                     : 0;
     }

lib/Sema/CSStep.h

@@ -619,15 +619,6 @@ 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

@@ -2776,61 +2776,45 @@ static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs,
 
   auto &DE = cs.getASTContext().Diags;
 
-  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()};
-    }
-  }
+  llvm::SmallDenseMap<TypeVariableType *, SmallVector<Type, 4>> conflicts;
 
-  llvm::SmallDenseMap<std::pair<GenericTypeParamType *, SourceLoc>,
-                      SmallVector<Type, 4>>
-      conflicts;
+  for (const auto &binding : solutions[0].typeBindings) {
+    auto *typeVar = binding.first;
 
-  for (const auto &entry : genericParams) {
-    auto *typeVar = entry.first;
-    auto GP = entry.second;
+    if (!typeVar->getImpl().getGenericParameter())
+      continue;
 
     llvm::SmallSetVector<Type, 4> arguments;
-    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();
+    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;
+            });
           }
-          return false;
-        });
-      }
 
-      arguments.insert(type);
-    }
+          arguments.insert(binding->second);
+          return true;
+        }))
+      continue;
 
-    if (arguments.size() > 1)
-      conflicts[GP].append(arguments.begin(), arguments.end());
+    if (arguments.size() > 1) {
+      conflicts[typeVar].append(arguments.begin(), arguments.end());
+    }
   }
 
   auto getGenericTypeDecl = [&](ArchetypeType *archetype) -> ValueDecl * {
@@ -2847,10 +2831,8 @@ static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs,
 
   bool diagnosed = false;
   for (auto &conflict : conflicts) {
-    SourceLoc loc;
-    GenericTypeParamType *GP;
-
-    std::tie(GP, loc) = conflict.first;
+    auto *typeVar = conflict.first;
+    auto *locator = typeVar->getImpl().getLocator();
     auto conflictingArguments = conflict.second;
 
     llvm::SmallString<64> arguments;
@@ -2875,8 +2857,10 @@ static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs,
         },
         [&OS] { OS << " vs. "; });
 
-    DE.diagnose(loc, diag::conflicting_arguments_for_generic_parameter, GP,
-                OS.str());
+    auto *anchor = locator->getAnchor();
+    DE.diagnose(anchor->getLoc(),
+                diag::conflicting_arguments_for_generic_parameter,
+                typeVar->getImpl().getGenericParameter(), 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 element type 'Int'}}
+useIntDict(["Niners" => 31, "Ravens" => 34.0]) // expected-error{{cannot convert value of type 'Double' to expected argument 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,7 +456,13 @@ extension Collection {
   }
 }
 func fn_r28909024(n: Int) {
-  return (0..<10).r28909024 { // expected-error {{unexpected non-void return value in void function}} expected-note {{did you mean to add a return type?}}
+  // 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}}
     _ in true
   }
 }

test/Constraints/default_literals.swift

@@ -42,14 +42,3 @@ 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/Constraints/operator.swift

@@ -254,3 +254,14 @@ extension Int {
 }
 
 _ = 1 ^^ 2 ^^ 3 * 4 // expected-error {{adjacent operators are in unordered precedence groups 'PowerPrecedence' and 'MultiplicationPrecedence'}}
+
+// rdar://problem/60185506 - Ambiguity with Float comparison
+func rdar_60185506() {
+  struct X {
+    var foo: Float
+  }
+
+  func test(x: X?) {
+    let _ = (x?.foo ?? 0) <= 0.5 // Ok
+  }
+}

test/Generics/deduction.swift

@@ -319,9 +319,8 @@ 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{{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'}}
+    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}}
 }
 
 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 {{unexpected non-void return value in void function}}
+        return try! failableIdentity("shucks") // expected-error {{cannot convert value of type 'String' to expected argument type '()'}}
     }
 }