Skip to content

Commit 45c71de

Browse files
committed
Sema: Disambiguate some more bidirectional conversions
This fixes an ambiguity introduced by the stdlib change in 0f99458. 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).
1 parent 1e9f882 commit 45c71de

File tree

3 files changed

+69
-11
lines changed

3 files changed

+69
-11
lines changed

lib/Sema/CSRanking.cpp

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,7 @@ Comparison TypeChecker::compareDeclarations(DeclContext *dc,
852852
return decl1Better ? Comparison::Better : Comparison::Worse;
853853
}
854854

855-
static Type getUnlabeledType(Type type, ASTContext &ctx) {
855+
static Type getStrippedType(Type type, ASTContext &ctx) {
856856
return type.transformRec([&](TypeBase *type) -> std::optional<Type> {
857857
if (auto *tupleType = dyn_cast<TupleType>(type)) {
858858
if (tupleType->getNumElements() == 1)
@@ -866,6 +866,31 @@ static Type getUnlabeledType(Type type, ASTContext &ctx) {
866866
return TupleType::get(elts, ctx);
867867
}
868868

869+
if (auto *funcType = dyn_cast<FunctionType>(type)) {
870+
auto params = funcType->getParams();
871+
SmallVector<AnyFunctionType::Param, 4> newParams;
872+
for (auto param : params) {
873+
auto newParam = param;
874+
switch (param.getParameterFlags().getOwnershipSpecifier()) {
875+
case ParamSpecifier::Borrowing:
876+
case ParamSpecifier::Consuming: {
877+
auto flags = param.getParameterFlags()
878+
.withOwnershipSpecifier(ParamSpecifier::Default);
879+
newParams.push_back(param.withFlags(flags));
880+
break;
881+
}
882+
default:
883+
newParams.push_back(newParam);
884+
break;
885+
}
886+
}
887+
auto newExtInfo = funcType->getExtInfo().withRepresentation(
888+
AnyFunctionType::Representation::Swift);
889+
return FunctionType::get(newParams,
890+
funcType->getResult(),
891+
newExtInfo);
892+
}
893+
869894
return std::nullopt;
870895
});
871896
}
@@ -1438,15 +1463,16 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
14381463
if (type2Better)
14391464
++score2;
14401465

1441-
// Prefer the unlabeled form of a type.
1442-
auto unlabeled1 = getUnlabeledType(type1, cs.getASTContext());
1443-
auto unlabeled2 = getUnlabeledType(type2, cs.getASTContext());
1444-
if (unlabeled1->isEqual(unlabeled2)) {
1445-
if (type1->isEqual(unlabeled1) && !types.Type1WasLabeled) {
1466+
// Prefer the "stripped" form of a type. See getStrippedType()
1467+
// for the definition.
1468+
auto stripped1 = getStrippedType(type1, cs.getASTContext());
1469+
auto stripped2 = getStrippedType(type2, cs.getASTContext());
1470+
if (stripped1->isEqual(stripped2)) {
1471+
if (type1->isEqual(stripped1) && !types.Type1WasLabeled) {
14461472
++score1;
14471473
continue;
14481474
}
1449-
if (type2->isEqual(unlabeled2) && !types.Type2WasLabeled) {
1475+
if (type2->isEqual(stripped2) && !types.Type2WasLabeled) {
14501476
++score2;
14511477
continue;
14521478
}

test/Constraints/bidirectional_conversions.swift

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,37 @@ func baz2(x: (Int, Int)?) {
4343
func f(_: (x: Int, y: Int)) {}
4444
f(unwrap(x))
4545
}
46+
47+
/////////////
48+
49+
func borrowingFn(fn: @escaping (borrowing AnyObject) -> ()) -> (AnyObject) -> () {
50+
return id(id(fn))
51+
}
52+
53+
/////////////
54+
55+
infix operator <+
56+
infix operator >+
57+
58+
protocol P {
59+
static func <+ (lhs: borrowing Self, rhs: borrowing Self)
60+
static func >+ (lhs: borrowing Self, rhs: borrowing Self)
61+
}
62+
63+
extension P {
64+
static func >+ (lhs: borrowing Self, rhs: borrowing Self) {}
65+
}
66+
67+
struct S: P {
68+
static func <+ (lhs: Self, rhs: Self) {}
69+
}
70+
71+
let _: (S, S) -> () = false ? (<+) : (>+)
72+
73+
/////////////
74+
75+
struct MyString: Comparable {
76+
static func < (lhs: Self, rhs: Self) -> Bool { fatalError() }
77+
}
78+
79+
let _: (MyString, MyString) -> Bool = false ? (<) : (>)

test/Constraints/bidirectional_conversions_objc.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,12 @@ func id<T>(_: T) -> T {}
3636

3737
func bar3(x: @escaping () -> ()) {
3838
func f(_: @escaping @convention(block) () -> ()) {}
39-
// FIXME
40-
f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('@convention(block) () -> ()' vs. '() -> ()')}}
39+
f(id(x))
4140
}
4241

4342
func bar4(x: @escaping @convention(block) () -> ()) {
4443
func f(_: @escaping () -> ()) {}
45-
// FIXME
46-
f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('() -> ()' vs. '@convention(block) () -> ()')}}
44+
f(id(x))
4745
}
4846

4947
func bar5(x: Double) {

0 commit comments

Comments
 (0)