[CSOptimizer] Skip ?? if it's involved in optional chain with unres…

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)
+}