[CSOptimizer] Restore old hack behavior which used to favor overloads based on arity matches

This maintains an "old hack" behavior where overloads  of some
`OverloadedDeclRef` calls were favored purely based on number of
argument and (non-defaulted) parameters matching.

This is important to maintain source compatibility.

Author: xedin

lib/Sema/CSOptimizer.cpp

@@ -172,9 +172,67 @@ void forEachDisjunctionChoice(
   }
 }
 
-static bool isOverloadedDeclRef(Constraint *disjunction) {
+static OverloadedDeclRefExpr *isOverloadedDeclRef(Constraint *disjunction) {
   assert(disjunction->getKind() == ConstraintKind::Disjunction);
-  return disjunction->getLocator()->directlyAt<OverloadedDeclRefExpr>();
+
+  auto *locator = disjunction->getLocator();
+  if (locator->getPath().empty())
+    return getAsExpr<OverloadedDeclRefExpr>(locator->getAnchor());
+  return nullptr;
+}
+
+/// This maintains an "old hack" behavior where overloads  of some
+/// `OverloadedDeclRef` calls were favored purely based on number of
+/// argument and (non-defaulted) parameters matching.
+static void findFavoredChoicesBasedOnArity(
+    ConstraintSystem &cs, Constraint *disjunction, ArgumentList *argumentList,
+    llvm::function_ref<void(Constraint *)> favoredChoice) {
+  auto *ODRE = isOverloadedDeclRef(disjunction);
+  if (!ODRE)
+    return;
+
+  if (llvm::count_if(ODRE->getDecls(), [&argumentList](auto *choice) {
+        if (auto *paramList = getParameterList(choice))
+          return argumentList->size() == paramList->size();
+        return false;
+      }) > 1)
+    return;
+
+  auto isVariadicGenericOverload = [&](ValueDecl *choice) {
+    auto genericContext = choice->getAsGenericContext();
+    if (!genericContext)
+      return false;
+
+    auto *GPL = genericContext->getGenericParams();
+    if (!GPL)
+      return false;
+
+    return llvm::any_of(GPL->getParams(), [&](const GenericTypeParamDecl *GP) {
+      return GP->isParameterPack();
+    });
+  };
+
+  bool hasVariadicGenerics = false;
+  SmallVector<Constraint *> favored;
+
+  forEachDisjunctionChoice(
+      cs, disjunction,
+      [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) {
+        if (isVariadicGenericOverload(decl))
+          hasVariadicGenerics = true;
+
+        if (overloadType->getNumParams() == argumentList->size() ||
+            llvm::count_if(*getParameterList(decl), [](auto *param) {
+              return !param->isDefaultArgument();
+            }) == argumentList->size())
+          favored.push_back(choice);
+      });
+
+  if (hasVariadicGenerics)
+    return;
+
+  for (auto *choice : favored)
+    favoredChoice(choice);
 }
 
 } // end anonymous namespace
@@ -193,9 +251,6 @@ static Constraint *determineBestChoicesInContext(
       favoredChoicesPerDisjunction;
 
   for (auto *disjunction : disjunctions) {
-    if (!isSupportedDisjunction(disjunction))
-      continue;
-
     auto applicableFn =
         getApplicableFnConstraint(cs.getConstraintGraph(), disjunction);
 
@@ -218,6 +273,25 @@ static Constraint *determineBestChoicesInContext(
       }
     }
 
+    // This maintains an "old hack" behavior where overloads
+    // of `OverloadedDeclRef` calls were favored purely
+    // based on arity of arguments and parameters matching.
+    {
+      findFavoredChoicesBasedOnArity(
+          cs, disjunction, argumentList, [&](Constraint *choice) {
+            favoredChoicesPerDisjunction[disjunction].push_back(choice);
+          });
+
+      if (!favoredChoicesPerDisjunction[disjunction].empty()) {
+        disjunctionScores[disjunction] = 0.01;
+        bestOverallScore = std::max(bestOverallScore, 0.01);
+        continue;
+      }
+    }
+
+    if (!isSupportedDisjunction(disjunction))
+      continue;
+
     SmallVector<FunctionType::Param, 8> argsWithLabels;
     {
       argsWithLabels.append(argFuncType->getParams().begin(),

test/Constraints/old_hack_related_ambiguities.swift

@@ -187,3 +187,53 @@ do {
   let result = test(10, accuracy: 1)
   let _: Int = result
 }
+
+// swift-distributed-tracing snippet that relies on old hack behavior.
+protocol TracerInstant {
+}
+
+extension Int: TracerInstant {}
+
+do {
+  enum SpanKind {
+    case `internal`
+  }
+
+  func withSpan<Instant: TracerInstant>(
+    _ operationName: String,
+    at instant: @autoclosure () -> Instant,
+    context: @autoclosure () -> Int = 0,
+    ofKind kind: SpanKind = .internal
+  ) {}
+
+  func withSpan(
+     _ operationName: String,
+     context: @autoclosure () -> Int = 0,
+     ofKind kind: SpanKind = .internal,
+     at instant: @autoclosure () -> some TracerInstant = 42
+  ) {}
+
+  withSpan("", at: 0) // Ok
+}
+
+protocol ForAssert {
+  var isEmpty: Bool { get }
+}
+
+extension ForAssert {
+  var isEmpty: Bool { false }
+}
+
+do {
+  func assert(_ condition: @autoclosure () -> Bool, _ message: @autoclosure () -> String = String(), file: StaticString = #file, line: UInt = #line) {}
+  func assert(_ condition: Bool, _ message: @autoclosure () -> String, file: StaticString = #file, line: UInt = #line) {}
+  func assert(_ condition: Bool, file: StaticString = #fileID, line: UInt = #line) {}
+
+  struct S : ForAssert {
+    var isEmpty: Bool { false }
+  }
+
+  func test(s: S) {
+    assert(s.isEmpty, "") // Ok
+  }
+}

test/Constraints/ranking.swift

@@ -450,11 +450,3 @@ struct HasIntInit {
 func compare_solutions_with_bindings(x: UInt8, y: UInt8) -> HasIntInit {
   return .init(Int(x / numericCast(y)))
 }
-
-// Test to make sure that previous favoring behavior is maintained and @autoclosure makes a difference.
-func test_no_ambiguity_with_autoclosure(x: Int) {
-  func test(_ condition: Bool, file: StaticString = #file, line: UInt = #line) {}
-  func test(_ condition: @autoclosure () -> Bool, file: StaticString = #file, line: UInt = #line) {}
-
-  test(x >= 0) // Ok
-}