[CSOptimizer] Skip ?? if it's involved in optional chain with unresolved result type

`??` operator is overloaded on optionality of its result. When the
first argument matches exactly, the ranking is going to be skewed
towards selecting an overload choice that returns a non-optional type.
This is not always correct i.e. when operator is involved in optional
chaining. To avoid producing an incorrect favoring, let's skip the this
disjunction when constraints associated with result type indicate
that it should be optional.

Simply adding it as a binding won't work because if the second argument
is non-optional the overload that returns `T?` would still have a lower
score.

Resolves: rdar://164201746

Author: xedin

lib/Sema/CSOptimizer.cpp

@@ -261,6 +261,14 @@ static bool isStandardInfixLogicalOperator(Constraint *disjunction) {
   return false;
 }
 
+static bool isNilCoalescingOperator(Constraint *disjunction) {
+  auto *choice = disjunction->getNestedConstraints()[0];
+  if (auto *decl = getOverloadChoiceDecl(choice))
+    return decl->isOperator() &&
+           decl->getBaseIdentifier().isNilCoalescingOperator();
+  return false;
+}
+
 static bool isArithmeticOperator(ValueDecl *decl) {
   return decl->isOperator() && decl->getBaseIdentifier().isArithmeticOperator();
 }
@@ -1086,6 +1094,30 @@ static void determineBestChoicesInContext(
     if (auto *typeVar = resultType->getAs<TypeVariableType>()) {
       auto bindingSet = cs.getBindingsFor(typeVar);
 
+      // `??` operator is overloaded on optionality of its result. When the
+      // first argument matches exactly, the ranking is going to be skewed
+      // towards selecting an overload choice that returns a non-optional type.
+      // This is not always correct i.e. when operator is involved in optional
+      // chaining. To avoid producing an incorrect favoring, let's skip the this
+      // disjunction when constraints associated with result type indicate
+      // that it should be optional.
+      //
+      // Simply adding it as a binding won't work because if the second argument
+      // is non-optional the overload that returns `T?` would still have a lower
+      // score.
+      if (!bindingSet && isNilCoalescingOperator(disjunction)) {
+        auto &cg = cs.getConstraintGraph();
+        if (llvm::any_of(cg[typeVar].getConstraints(),
+                         [&typeVar](Constraint *constraint) {
+                           if (constraint->getKind() !=
+                               ConstraintKind::OptionalObject)
+                             return false;
+                           return constraint->getFirstType()->isEqual(typeVar);
+                         })) {
+          continue;
+        }
+      }
+
       for (const auto &binding : bindingSet.Bindings) {
         resultTypes.push_back(binding.BindingType);
       }

test/Constraints/nil-coalescing-favoring.swift

@@ -62,3 +62,12 @@ func test_no_incorrect_favoring(v: Int?, o: Int) {
   sameType(s1, as: Int?.self)
   sameType(s2, as: Int?.self)
 }
+
+extension Int {
+  func test() -> Int { 42 }
+}
+
+func test_optional_chaining(v: Int?, defaultV: Int) {
+  // CHECK: declref_expr type="(consuming Int?, @autoclosure () throws -> Int?) throws -> Int?" {{.*}} decl="Swift.(file).??
+  _ = (v ?? defaultV)?.test() // Ok (no warnings)
+}