[CSOptimizer] Emulate old hack for unary argument matching more precisely

Having it be part of the other matching wasn't a good idea because
previous "favoring" happened only in a few situations - if argument
was a declaration reference, application or (dynamic) subscript that
had overload choice selected during constraint generation.

Author: xedin

lib/Sema/CSOptimizer.cpp

@@ -338,6 +338,52 @@ static void findFavoredChoicesBasedOnArity(
     favoredChoice(choice);
 }
 
+/// Preserves old behavior where, for unary calls, the solver would not previously
+/// consider choices that didn't match on the number of parameters (regardless of
+/// defaults and variadics) and only exact matches were favored.
+static std::optional<DisjunctionInfo> preserveFavoringOfUnlabeledUnaryArgument(
+    ConstraintSystem &cs, Constraint *disjunction, ArgumentList *argumentList) {
+  if (!argumentList->isUnlabeledUnary())
+    return std::nullopt;
+
+  auto ODRE = isOverloadedDeclRef(disjunction);
+  bool preserveFavoringOfUnlabeledUnaryArgument =
+      !ODRE || numOverloadChoicesMatchingOnArity(ODRE, argumentList) < 2;
+
+  if (!preserveFavoringOfUnlabeledUnaryArgument)
+    return std::nullopt;
+
+  auto *argument =
+      argumentList->getUnlabeledUnaryExpr()->getSemanticsProvidingExpr();
+  // The hack operated on "favored" types and only declaration references,
+  // applications, and (dynamic) subscripts had them if they managed to
+  // get an overload choice selected during constraint generation.
+  if (!(isExpr<DeclRefExpr>(argument) || isExpr<ApplyExpr>(argument) ||
+        isExpr<SubscriptExpr>(argument) ||
+        isExpr<DynamicSubscriptExpr>(argument)))
+    return {/*score=*/0};
+
+  auto argumentType = cs.getType(argument);
+  if (argumentType->hasTypeVariable() || argumentType->hasDependentMember())
+    return {/*score=*/0};
+
+  SmallVector<Constraint *, 2> favoredChoices;
+  forEachDisjunctionChoice(
+      cs, disjunction,
+      [&argumentType, &favoredChoices](Constraint *choice, ValueDecl *decl,
+                                       FunctionType *overloadType) {
+        if (overloadType->getNumParams() != 1)
+          return;
+
+        auto paramType = overloadType->getParams()[0].getPlainType();
+        if (paramType->isEqual(argumentType))
+          favoredChoices.push_back(choice);
+      });
+
+  return DisjunctionInfo(/*score=*/favoredChoices.empty() ? 0 : 1,
+                         favoredChoices);
+}
+
 } // end anonymous namespace
 
 /// Given a set of disjunctions, attempt to determine
@@ -444,6 +490,16 @@ static void determineBestChoicesInContext(
       }
     }
 
+    // Preserves old behavior where, for unary calls, the solver
+    // would not consider choices that didn't match on the number
+    // of parameters (regardless of defaults) and only exact
+    // matches were favored.
+    if (auto info = preserveFavoringOfUnlabeledUnaryArgument(cs, disjunction,
+                                                             argumentList)) {
+      recordResult(disjunction, std::move(info.value()));
+      continue;
+    }
+
     if (!isSupportedDisjunction(disjunction))
       continue;
 
@@ -931,17 +987,6 @@ static void determineBestChoicesInContext(
     double bestScore = 0.0;
     SmallVector<std::pair<Constraint *, double>, 2> favoredChoices;
 
-    // Preserves old behavior where, for unary calls, the solver
-    // would not consider choices that didn't match on the number
-    // of parameters (regardless of defaults) and only exact
-    // matches were favored.
-    bool preserveFavoringOfUnlabeledUnaryArgument = false;
-    if (argumentList->isUnlabeledUnary()) {
-      auto ODRE = isOverloadedDeclRef(disjunction);
-      preserveFavoringOfUnlabeledUnaryArgument =
-          !ODRE || numOverloadChoicesMatchingOnArity(ODRE, argumentList) < 2;
-    }
-
     forEachDisjunctionChoice(
         cs, disjunction,
         [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) {
@@ -970,15 +1015,6 @@ static void determineBestChoicesInContext(
               onlyLiteralCandidates &&
               (!canUseContextualResultTypes() || resultTypes.empty());
 
-          if (preserveFavoringOfUnlabeledUnaryArgument) {
-            // Old behavior completely disregarded the fact that some of
-            // the parameters could be defaulted.
-            if (overloadType->getNumParams() != 1)
-              return;
-
-            favorExactMatchesOnly = true;
-          }
-
           // This is important for SIMD operators in particular because
           // a lot of their overloads have same-type requires to a concrete
           // type:  `<Scalar == (U)Int*>(_: SIMD*<Scalar>, ...) -> ...`.

test/Constraints/old_hack_related_ambiguities.swift

@@ -6,14 +6,18 @@ func entity(_: Int) -> Int {
 
 struct Test {
   func test(_ v: Int) -> Int { v }
+  // expected-note@-1 {{found this candidate}}
   func test(_ v: Int?) -> Int? { v }
+  // expected-note@-1 {{found this candidate}}
 }
 
 func test_ternary_literal(v: Test) -> Int? {
-  true ? v.test(0) : nil // Ok
+  // Literals don't have a favored type
+  true ? v.test(0) : nil // expected-error {{ambiguous use of 'test'}}
 }
 
 func test_ternary(v: Test) -> Int? {
+  // Because calls had favored types set if they were resolved during constraint generation.
   true ? v.test(entity(0)) : nil // Ok
 }
 
@@ -159,12 +163,14 @@ do {
     var p: UnsafeMutableRawPointer { get { fatalError() } }
 
     func f(_ p: UnsafeMutableRawPointer) {
+      // The old hack (which is now removed) couldn't handle member references, only direct declaration references.
       guard let x = UnsafeMutablePointer<Double>(OpaquePointer(self.p)) else {
         return
       }
       _ = x
 
       guard let x = UnsafeMutablePointer<Double>(OpaquePointer(p)) else {
+        // expected-error@-1 {{initializer for conditional binding must have Optional type, not 'UnsafeMutablePointer<Double>'}}
         return
       }
       _ = x
@@ -257,3 +263,15 @@ func test_non_default_literal_use(arg: Float) {
     let v = arg * 2.0 // shouldn't use `(Float, Double) -> Double` overload
     let _: Float = v // Ok
 }
+
+// This should be ambiguous without contextual type but was accepted before during to
+// unlabeled unary argument favoring.
+func test_variadic_static_member_is_preferred_over_partially_applied_instance_overload() {
+  struct Test {
+    func fn() {}
+    static func fn(_: Test...) {}
+  }
+
+  let t: Test
+  Test.fn(t) // Ok
+}