From f3e6b4ceda591ed5beded81a8cf82ffcd42b8b92 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Dec 2025 17:32:48 -0500 Subject: [PATCH] Sema: Disambiguate some more bidirectional conversions This fixes an ambiguity introduced by the stdlib change in 0f99458900624c69a07bbc6ee259a4d8aded07dc. Since (borrowing T) -> () and (T) -> () both convert to each other, we could end up with ambiguous solutions where neither one was better than the other. Generalize the existing trick we use for labeled vs unlabeled tuples to also strip off ownership specifiers and @convention(...) from function types. This fixes the regression, as well an existing FIXME in a test I added a while ago where the same problem arises with @convention(block). --- lib/Sema/CSRanking.cpp | 40 +++++++++++++++---- .../bidirectional_conversions.swift | 34 ++++++++++++++++ .../bidirectional_conversions_objc.swift | 6 +-- test/expr/closure/closures.swift | 2 +- 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/lib/Sema/CSRanking.cpp b/lib/Sema/CSRanking.cpp index 9830c66dbe9..9c91d382f0f 100644 --- a/lib/Sema/CSRanking.cpp +++ b/lib/Sema/CSRanking.cpp @@ -852,7 +852,7 @@ Comparison TypeChecker::compareDeclarations(DeclContext *dc, return decl1Better ? Comparison::Better : Comparison::Worse; } -static Type getUnlabeledType(Type type, ASTContext &ctx) { +static Type getStrippedType(Type type, ASTContext &ctx) { return type.transformRec([&](TypeBase *type) -> std::optional { if (auto *tupleType = dyn_cast(type)) { if (tupleType->getNumElements() == 1) @@ -866,6 +866,31 @@ static Type getUnlabeledType(Type type, ASTContext &ctx) { return TupleType::get(elts, ctx); } + if (auto *funcType = dyn_cast(type)) { + auto params = funcType->getParams(); + SmallVector newParams; + for (auto param : params) { + auto newParam = param; + switch (param.getParameterFlags().getOwnershipSpecifier()) { + case ParamSpecifier::Borrowing: + case ParamSpecifier::Consuming: { + auto flags = param.getParameterFlags() + .withOwnershipSpecifier(ParamSpecifier::Default); + newParams.push_back(param.withFlags(flags)); + break; + } + default: + newParams.push_back(newParam); + break; + } + } + auto newExtInfo = funcType->getExtInfo().withRepresentation( + AnyFunctionType::Representation::Swift); + return FunctionType::get(newParams, + funcType->getResult(), + newExtInfo); + } + return std::nullopt; }); } @@ -1438,15 +1463,16 @@ SolutionCompareResult ConstraintSystem::compareSolutions( if (type2Better) ++score2; - // Prefer the unlabeled form of a type. - auto unlabeled1 = getUnlabeledType(type1, cs.getASTContext()); - auto unlabeled2 = getUnlabeledType(type2, cs.getASTContext()); - if (unlabeled1->isEqual(unlabeled2)) { - if (type1->isEqual(unlabeled1) && !types.Type1WasLabeled) { + // Prefer the "stripped" form of a type. See getStrippedType() + // for the definition. + auto stripped1 = getStrippedType(type1, cs.getASTContext()); + auto stripped2 = getStrippedType(type2, cs.getASTContext()); + if (stripped1->isEqual(stripped2)) { + if (type1->isEqual(stripped1) && !types.Type1WasLabeled) { ++score1; continue; } - if (type2->isEqual(unlabeled2) && !types.Type2WasLabeled) { + if (type2->isEqual(stripped2) && !types.Type2WasLabeled) { ++score2; continue; } diff --git a/test/Constraints/bidirectional_conversions.swift b/test/Constraints/bidirectional_conversions.swift index 55abd75a413..18612106a1e 100644 --- a/test/Constraints/bidirectional_conversions.swift +++ b/test/Constraints/bidirectional_conversions.swift @@ -43,3 +43,37 @@ func baz2(x: (Int, Int)?) { func f(_: (x: Int, y: Int)) {} f(unwrap(x)) } + +///////////// + +func borrowingFn(fn: @escaping (borrowing AnyObject) -> ()) -> (AnyObject) -> () { + return id(id(fn)) +} + +///////////// + +infix operator <+ +infix operator >+ + +protocol P { + static func <+ (lhs: borrowing Self, rhs: borrowing Self) + static func >+ (lhs: borrowing Self, rhs: borrowing Self) +} + +extension P { + static func >+ (lhs: borrowing Self, rhs: borrowing Self) {} +} + +struct S: P { + static func <+ (lhs: Self, rhs: Self) {} +} + +let _: (S, S) -> () = false ? (<+) : (>+) + +///////////// + +struct MyString: Comparable { + static func < (lhs: Self, rhs: Self) -> Bool { fatalError() } +} + +let _: (MyString, MyString) -> Bool = false ? (<) : (>) diff --git a/test/Constraints/bidirectional_conversions_objc.swift b/test/Constraints/bidirectional_conversions_objc.swift index 069ab2438d0..72b4f10aeef 100644 --- a/test/Constraints/bidirectional_conversions_objc.swift +++ b/test/Constraints/bidirectional_conversions_objc.swift @@ -36,14 +36,12 @@ func id(_: T) -> T {} func bar3(x: @escaping () -> ()) { func f(_: @escaping @convention(block) () -> ()) {} - // FIXME - f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('@convention(block) () -> ()' vs. '() -> ()')}} + f(id(x)) } func bar4(x: @escaping @convention(block) () -> ()) { func f(_: @escaping () -> ()) {} - // FIXME - f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('() -> ()' vs. '@convention(block) () -> ()')}} + f(id(x)) } func bar5(x: Double) { diff --git a/test/expr/closure/closures.swift b/test/expr/closure/closures.swift index b8af7e06c65..e421758544f 100644 --- a/test/expr/closure/closures.swift +++ b/test/expr/closure/closures.swift @@ -551,7 +551,7 @@ do { let qux: () -> Void f(qux) - f(id(qux)) // expected-error {{conflicting arguments to generic parameter 'T' ('() -> Void' vs. '@convention(block) () -> Void')}} + f(id(qux)) func forceUnwrap(_: T?) -> T {}