From 1e9f8826a7ae7aad8a070f110fc74aad91f473e5 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Dec 2025 17:31:19 -0500 Subject: [PATCH 1/3] Sema: Split off Apple-specific parts of bidirectional_conversions.swift test --- .../bidirectional_conversions.swift | 67 --------------- .../bidirectional_conversions_objc.swift | 81 +++++++++++++++++++ 2 files changed, 81 insertions(+), 67 deletions(-) create mode 100644 test/Constraints/bidirectional_conversions_objc.swift diff --git a/test/Constraints/bidirectional_conversions.swift b/test/Constraints/bidirectional_conversions.swift index 43620893f9693..55abd75a413f3 100644 --- a/test/Constraints/bidirectional_conversions.swift +++ b/test/Constraints/bidirectional_conversions.swift @@ -1,8 +1,4 @@ // RUN: %target-typecheck-verify-swift -// REQUIRES: objc_interop - -import Foundation -import CoreGraphics ///////////// @@ -20,26 +16,6 @@ func foo2(x: (Int, Int)?, y: (x: Int, y: Int)) -> G<(Int, Int)> { return g } -func foo3(x: (@convention(block) () -> ())?, y: @escaping () -> ()) -> G<@convention(block) () -> ()> { - let g = G(t: x ?? y) - return g -} - -func foo4(x: (() -> ())?, y: @escaping @convention(block) () -> ()) -> G<() -> ()> { - let g = G(t: x ?? y) - return g -} - -func foo5(x: CGFloat?, y: Double) -> G { - let g = G(t: x ?? y) - return g -} - -func foo6(x: Double?, y: CGFloat) -> G { - let g = G(t: x ?? y) - return g -} - ///////////// func id(_: T) -> T {} @@ -54,28 +30,6 @@ func bar2(x: (Int, Int)) { f(id(x)) } -func bar3(x: @escaping () -> ()) { - func f(_: @escaping @convention(block) () -> ()) {} - // FIXME - f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('@convention(block) () -> ()' vs. '() -> ()')}} -} - -func bar4(x: @escaping @convention(block) () -> ()) { - func f(_: @escaping () -> ()) {} - // FIXME - f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('() -> ()' vs. '@convention(block) () -> ()')}} -} - -func bar5(x: Double) { - func f(_: CGFloat) {} - f(id(x)) -} - -func bar6(x: CGFloat) { - func f(_: Double) {} - f(id(x)) -} - ///////////// func unwrap(_: T?) -> T {} @@ -89,24 +43,3 @@ func baz2(x: (Int, Int)?) { func f(_: (x: Int, y: Int)) {} f(unwrap(x)) } - -func baz3(x: (() -> ())?) { - func f(_: @escaping @convention(block) () -> ()) {} - f(unwrap(x)) -} - -func baz4(x: (@convention(block) () -> ())?) { - func f(_: @escaping () -> ()) {} - f(unwrap(x)) -} - -func baz5(x: Double?) { - func f(_: CGFloat) {} - f(unwrap(x)) -} - -func baz6(x: CGFloat?) { - func f(_: Double) {} - f(unwrap(x)) -} - diff --git a/test/Constraints/bidirectional_conversions_objc.swift b/test/Constraints/bidirectional_conversions_objc.swift new file mode 100644 index 0000000000000..069ab2438d0ff --- /dev/null +++ b/test/Constraints/bidirectional_conversions_objc.swift @@ -0,0 +1,81 @@ +// RUN: %target-typecheck-verify-swift +// REQUIRES: objc_interop + +import Foundation +import CoreGraphics + +///////////// + +struct G { + var t: T +} + +func foo3(x: (@convention(block) () -> ())?, y: @escaping () -> ()) -> G<@convention(block) () -> ()> { + let g = G(t: x ?? y) + return g +} + +func foo4(x: (() -> ())?, y: @escaping @convention(block) () -> ()) -> G<() -> ()> { + let g = G(t: x ?? y) + return g +} + +func foo5(x: CGFloat?, y: Double) -> G { + let g = G(t: x ?? y) + return g +} + +func foo6(x: Double?, y: CGFloat) -> G { + let g = G(t: x ?? y) + return g +} + +///////////// + +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. '() -> ()')}} +} + +func bar4(x: @escaping @convention(block) () -> ()) { + func f(_: @escaping () -> ()) {} + // FIXME + f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('() -> ()' vs. '@convention(block) () -> ()')}} +} + +func bar5(x: Double) { + func f(_: CGFloat) {} + f(id(x)) +} + +func bar6(x: CGFloat) { + func f(_: Double) {} + f(id(x)) +} + +///////////// + +func unwrap(_: T?) -> T {} + +func baz3(x: (() -> ())?) { + func f(_: @escaping @convention(block) () -> ()) {} + f(unwrap(x)) +} + +func baz4(x: (@convention(block) () -> ())?) { + func f(_: @escaping () -> ()) {} + f(unwrap(x)) +} + +func baz5(x: Double?) { + func f(_: CGFloat) {} + f(unwrap(x)) +} + +func baz6(x: CGFloat?) { + func f(_: Double) {} + f(unwrap(x)) +} \ No newline at end of file From f3e6b4ceda591ed5beded81a8cf82ffcd42b8b92 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Dec 2025 17:32:48 -0500 Subject: [PATCH 2/3] 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 9830c66dbe91e..9c91d382f0f59 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 55abd75a413f3..18612106a1eb8 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 069ab2438d0ff..72b4f10aeef90 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 b8af7e06c653c..e421758544f59 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 {} From d5c049ed69393fc43cc6dbdb8fe5805a0d5f33eb Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Dec 2025 15:48:57 -0500 Subject: [PATCH 3/3] Sema: Better debug output in compareSolutions() --- lib/Sema/CSRanking.cpp | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSRanking.cpp b/lib/Sema/CSRanking.cpp index 9c91d382f0f59..8569c77ff39e8 100644 --- a/lib/Sema/CSRanking.cpp +++ b/lib/Sema/CSRanking.cpp @@ -1085,8 +1085,14 @@ SolutionCompareResult ConstraintSystem::compareSolutions( // In this case solver would produce 2 solutions: one where `count` // is a property reference on `[Int]` and another one is tuple access // for a `count:` element. - if (choice1.isDecl() != choice2.isDecl()) + if (choice1.isDecl() != choice2.isDecl()) { + if (cs.isDebugMode()) { + llvm::errs().indent(cs.solverState->getCurrentIndent()) + << "- incomparable\n"; + } + return SolutionCompareResult::Incomparable; + } auto decl1 = choice1.getDecl(); auto dc1 = decl1->getDeclContext(); @@ -1553,13 +1559,39 @@ SolutionCompareResult ConstraintSystem::compareSolutions( // If the scores are different, we have a winner. if (score1 != score2) { - return score1 > score2? SolutionCompareResult::Better - : SolutionCompareResult::Worse; + if (score1 > score2) { + if (cs.isDebugMode()) { + llvm::errs().indent(cs.solverState->getCurrentIndent()) + << "- better\n"; + } + + return SolutionCompareResult::Better; + } else { + if (cs.isDebugMode()) { + llvm::errs().indent(cs.solverState->getCurrentIndent()) + << "- worse\n"; + } + + return SolutionCompareResult::Worse; + } } // Neither system wins; report whether they were identical or not. - return identical? SolutionCompareResult::Identical - : SolutionCompareResult::Incomparable; + if (identical) { + if (cs.isDebugMode()) { + llvm::errs().indent(cs.solverState->getCurrentIndent()) + << "- identical\n"; + } + + return SolutionCompareResult::Identical; + } else { + if (cs.isDebugMode()) { + llvm::errs().indent(cs.solverState->getCurrentIndent()) + << "- incomparable\n"; + } + + return SolutionCompareResult::Incomparable; + } } std::optional