From a3a3ec4fe054c07e3d004b62fb4d0733a8bbd97d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 23 Sep 2024 17:52:04 -0700 Subject: [PATCH] [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. --- lib/Sema/CSOptimizer.cpp | 84 +++++++++++++++++-- .../old_hack_related_ambiguities.swift | 50 +++++++++++ test/Constraints/ranking.swift | 8 -- 3 files changed, 129 insertions(+), 13 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c86df5e535b..c58ec1f5821 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/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(); + + auto *locator = disjunction->getLocator(); + if (locator->getPath().empty()) + return getAsExpr(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 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 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 argsWithLabels; { argsWithLabels.append(argFuncType->getParams().begin(), diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index f30bfddfaf3..d1dc81b514f 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/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( + _ 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 + } +} diff --git a/test/Constraints/ranking.swift b/test/Constraints/ranking.swift index 74e3b6842f0..82d29791e6f 100644 --- a/test/Constraints/ranking.swift +++ b/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 -}