Merge pull request #9130 from rudkx/disjunction-selection

Author: swift-ci

lib/Sema/CSSolver.cpp

@@ -823,13 +823,24 @@ static PotentialBindings getPotentialBindings(ConstraintSystem &cs,
       // Relational constraints: break out to look for types above/below.
       break;
 
+    case ConstraintKind::DynamicTypeOf: {
+      auto otherTy = constraint->getSecondType();
+      if (constraint->getSecondType()->isEqual(typeVar))
+        otherTy = constraint->getFirstType();
+
+      auto simplified = cs.simplifyType(otherTy);
+      if (simplified->hasTypeVariable())
+        result.InvolvesTypeVariables = true;
+      continue;
+    }
+
     case ConstraintKind::BridgingConversion:
     case ConstraintKind::CheckedCast:
-    case ConstraintKind::DynamicTypeOf:
     case ConstraintKind::EscapableFunctionOf:
     case ConstraintKind::OpenedExistentialOf:
     case ConstraintKind::KeyPath:
     case ConstraintKind::KeyPathApplication:
+      // FIXME: check for other type variables?
       // Constraints from which we can't do anything.
       continue;
 
@@ -1027,6 +1038,17 @@ static PotentialBindings getPotentialBindings(ConstraintSystem &cs,
         continue;
       }
 
+      // If the other side of an argument conversion is also a type
+      // variable, we shouldn't allow that to block us from attempting
+      // bindings if it is the only constraint involving other type
+      // variables. Doing this allows us to attempt a binding much
+      // earlier in solving which can result in backing out of
+      // solutions that cannot work due to conformance constraints.
+      if (constraint->getKind() ==
+              ConstraintKind::OperatorArgumentTupleConversion ||
+          constraint->getKind() == ConstraintKind::OperatorArgumentConversion)
+        continue;
+
       result.InvolvesTypeVariables = true;
       continue;
     }
@@ -2389,6 +2411,81 @@ determineBestBindings(ConstraintSystem &CS) {
   return std::make_pair(bestBindings, bestTypeVar);
 }
 
+Constraint *getApplicableFunctionConstraint(ConstraintSystem &CS,
+                                             Constraint *disjunction) {
+  auto *nested = disjunction->getNestedConstraints().front();
+  auto *firstTy = nested->getFirstType()->castTo<TypeVariableType>();
+
+  // FIXME: Is there something else we should be doing when a member
+  //        of a disjunction is already bound because it's in an
+  //        equivalence class?
+  if (CS.getFixedType(firstTy))
+    return nullptr;
+
+  Constraint *found = nullptr;
+  for (auto *constraint : CS.getConstraintGraph()[firstTy].getConstraints()) {
+    if (constraint->getKind() != ConstraintKind::ApplicableFunction)
+      continue;
+
+    // Unapplied function reference, e.g. a.map(String.init)
+    if (!constraint->getSecondType()->isEqual(firstTy))
+      return nullptr;
+
+    found = constraint;
+    break;
+  }
+
+  if (!found)
+    return nullptr;
+
+#if !defined(NDEBUG)
+  for (auto *constraint : CS.getConstraintGraph()[firstTy].getConstraints()) {
+    if (constraint == found)
+      continue;
+
+    assert(constraint->getKind() != ConstraintKind::ApplicableFunction &&
+           "Type variable is involved in more than one applicable!");
+  }
+#endif
+
+  return found;
+}
+
+static unsigned countUnboundTypes(ConstraintSystem &CS,
+                                  Constraint *disjunction) {
+  auto *nested = disjunction->getNestedConstraints().front();
+  assert(nested->getKind() == ConstraintKind::BindOverload &&
+         "Expected bind overload constraint!");
+
+  auto firstTy = nested->getFirstType();
+
+  if (!firstTy->is<FunctionType>()) {
+    // If the disjunction is of the form "$T1 bound to..." we should have
+    // an associated applicable function constraint.
+    auto *applicableFn = getApplicableFunctionConstraint(CS, disjunction);
+
+    // If we have an unapplied function, we should be able to leave
+    // that disjunction for last without any downside.
+    if (!applicableFn)
+      return 100;
+
+    firstTy = applicableFn->getFirstType();
+  }
+
+  auto *fnTy = firstTy->getAs<FunctionType>();
+  assert(fnTy && "Expected function type!");
+  auto simplifiedTy = CS.simplifyType(fnTy->getInput());
+
+  SmallPtrSet<TypeVariableType *, 4> typeVars;
+  findInferableTypeVars(simplifiedTy, typeVars);
+  unsigned count = 0;
+  for (auto *tv : typeVars)
+    if (!CS.getFixedType(tv))
+      ++count;
+
+  return count;
+}
+
 bool ConstraintSystem::solveSimplified(
     SmallVectorImpl<Solution> &solutions,
     FreeTypeVariableBinding allowFreeTypeVariables) {
@@ -2445,29 +2542,78 @@ bool ConstraintSystem::solveSimplified(
     return false;
   }
 
-  // Pick the smallest disjunction.
-  // FIXME: This heuristic isn't great, but it helped somewhat for
-  // overload sets.
-  auto disjunction = disjunctions[0];
-  auto bestSize = disjunction->countActiveNestedConstraints();
-  if (bestSize > 2) {
-    for (auto contender : llvm::makeArrayRef(disjunctions).slice(1)) {
-      unsigned newSize = contender->countActiveNestedConstraints();
-      if (newSize < bestSize) {
-        bestSize = newSize;
-        disjunction = contender;
-
-        if (bestSize == 2)
+  // Select a disjunction based on a few factors, prioritizing
+  // coercions and initializer disjunctions, followed by bind overload
+  // disjunctions with with fewest unbound argument types.
+  Optional<Constraint *> selection;
+  Optional<unsigned> lowestUnbound;
+  Optional<unsigned> lowestActive;
+
+  for (auto candidate : disjunctions) {
+    unsigned countActive = candidate->countActiveNestedConstraints();
+    // Skip disjunctions with no active constraints.
+    if (!countActive)
+      continue;
+
+    // If this is the first disjunction with an active constraint,
+    // consider this our selection until we find a better one.
+    if (!selection)
+      selection = candidate;
+
+    if (auto loc = candidate->getLocator()) {
+      // If we have a coercion disjunction remaining, select that as
+      // they tend to constraint type variables maximally and give us
+      // information about potential bindings.
+      if (auto anchor = loc->getAnchor()) {
+        if (isa<CoerceExpr>(anchor)) {
+          selection = candidate;
           break;
+        }
+      }
+
+      // Initializers also provide a lot of information in the form of
+      // the result type and tend to split the system into independent
+      // pieces.
+      auto path = loc->getPath();
+      if (!path.empty() &&
+          path.back().getKind() ==
+          ConstraintLocator::PathElementKind::ConstructorMember) {
+        selection = candidate;
+        break;
       }
     }
+
+    // Attempt to find a bind overload disjunction before considering
+    // this one.
+    assert(!candidate->getNestedConstraints().empty() &&
+           "Expected non-empty disjunction!");
+    if (candidate->getNestedConstraints().front()->getKind() !=
+        ConstraintKind::BindOverload)
+      continue;
+
+    // Otherwise, attempt to count the number of unbound types used
+    // for bind overload disjunctions.
+    unsigned countUnbound = countUnboundTypes(*this, candidate);
+
+    // If the number of unbound arguments is smaller than previous
+    // candidate, or if we had no previous candidate, this is our
+    // newest best option.
+    if (!lowestUnbound || countUnbound < lowestUnbound.getValue() ||
+        (countUnbound == lowestUnbound.getValue() &&
+         countActive < lowestActive.getValue())) {
+      selection = candidate;
+      lowestUnbound = countUnbound;
+      lowestActive = countActive;
+    }
   }
 
   // If there are no active constraints in the disjunction, there is
   // no solution.
-  if (bestSize == 0)
+  if (!selection)
     return true;
 
+  auto *disjunction = selection.getValue();
+
   // Remove this disjunction constraint from the list.
   auto afterDisjunction = InactiveConstraints.erase(disjunction);
   CG.removeConstraint(disjunction);

validation-test/Sema/type_checker_perf/generic_operators.swift.gyb

@@ -0,0 +1,20 @@
+// RUN: %scale-test --begin 1 --end 5 --step 1 --select incrementScopeCounter %s --polynomial-threshold=2.3
+// REQUIRES: OS=macosx
+// REQUIRES: asserts
+
+infix operator +|+ : AdditionPrecedence
+
+func +|+ <T: FloatingPoint>(lhs: T, rhs: T) -> T {
+  return lhs
+}
+
+func +|+ <T: SignedInteger>(lhs: T, rhs: T) -> T {
+  return lhs
+}
+
+func test(i: Int, j: Int, k: Int, l: Int) -> Int {
+  return i +|+ j +|+ k +|+ l
+%for i in range(1, N):
+         +|+ i +|+ j +|+ k +|+ l
+%end
+}

validation-test/stdlib/Data.swift

@@ -28,7 +28,10 @@ DataTestSuite.test("Data.Iterator semantics") {
       ptr[i] = UInt8(i % 23)
     }
   }
-  checkSequence((0..<65535).lazy.map({ UInt8($0 % 23) }), data)
+  // SR-4724: Should not need to be split out but if it is not it
+  // is considered ambiguous.
+  let temp = (0..<65535).lazy.map({ UInt8($0 % 23) })
+  checkSequence(temp, data)
 }
 
 DataTestSuite.test("associated types") {

validation-test/stdlib/Set.swift

@@ -203,7 +203,10 @@ func getBridgedVerbatimSet(_ members: [Int] = [1010, 2020, 3030])
 /// Get a Set<NSObject> (Set<TestObjCKeyTy>) backed by native storage
 func getNativeBridgedVerbatimSet(_ members: [Int] = [1010, 2020, 3030]) ->
   Set<NSObject> {
-  let result: Set<NSObject> = Set(members.map({ TestObjCKeyTy($0) }))
+  // SR-4724: Should not need to be split out but if it is not it
+  // is considered ambiguous.
+  let temp = members.map({ TestObjCKeyTy($0) })
+  let result: Set<NSObject> = Set(temp)
   expectTrue(isNativeSet(result))
   return result
 }