Skip to content

Commit e7d2c16

Browse files
slavapestovslavapestov
authored
Merge pull request #85933 from slavapestov/fix-ben-regression
Sema: Disambiguate some more bidirectional conversions
2 parents 5803583 + d5c049e commit e7d2c16

File tree

4 files changed

+175
-71
lines changed

4 files changed

+175
-71
lines changed

‎lib/Sema/CSRanking.cpp‎

Lines changed: 70 additions & 12 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
}
@@ -1060,8 +1085,14 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
10601085
// In this case solver would produce 2 solutions: one where `count`
10611086
// is a property reference on `[Int]` and another one is tuple access
10621087
// for a `count:` element.
1063-
if (choice1.isDecl() != choice2.isDecl())
1088+
if (choice1.isDecl() != choice2.isDecl()) {
1089+
if (cs.isDebugMode()) {
1090+
llvm::errs().indent(cs.solverState->getCurrentIndent())
1091+
<< "- incomparable\n";
1092+
}
1093+
10641094
return SolutionCompareResult::Incomparable;
1095+
}
10651096

10661097
auto decl1 = choice1.getDecl();
10671098
auto dc1 = decl1->getDeclContext();
@@ -1438,15 +1469,16 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
14381469
if (type2Better)
14391470
++score2;
14401471

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) {
1472+
// Prefer the "stripped" form of a type. See getStrippedType()
1473+
// for the definition.
1474+
auto stripped1 = getStrippedType(type1, cs.getASTContext());
1475+
auto stripped2 = getStrippedType(type2, cs.getASTContext());
1476+
if (stripped1->isEqual(stripped2)) {
1477+
if (type1->isEqual(stripped1) && !types.Type1WasLabeled) {
14461478
++score1;
14471479
continue;
14481480
}
1449-
if (type2->isEqual(unlabeled2) && !types.Type2WasLabeled) {
1481+
if (type2->isEqual(stripped2) && !types.Type2WasLabeled) {
14501482
++score2;
14511483
continue;
14521484
}
@@ -1527,13 +1559,39 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
15271559

15281560
// If the scores are different, we have a winner.
15291561
if (score1 != score2) {
1530-
return score1 > score2? SolutionCompareResult::Better
1531-
: SolutionCompareResult::Worse;
1562+
if (score1 > score2) {
1563+
if (cs.isDebugMode()) {
1564+
llvm::errs().indent(cs.solverState->getCurrentIndent())
1565+
<< "- better\n";
1566+
}
1567+
1568+
return SolutionCompareResult::Better;
1569+
} else {
1570+
if (cs.isDebugMode()) {
1571+
llvm::errs().indent(cs.solverState->getCurrentIndent())
1572+
<< "- worse\n";
1573+
}
1574+
1575+
return SolutionCompareResult::Worse;
1576+
}
15321577
}
15331578

15341579
// Neither system wins; report whether they were identical or not.
1535-
return identical? SolutionCompareResult::Identical
1536-
: SolutionCompareResult::Incomparable;
1580+
if (identical) {
1581+
if (cs.isDebugMode()) {
1582+
llvm::errs().indent(cs.solverState->getCurrentIndent())
1583+
<< "- identical\n";
1584+
}
1585+
1586+
return SolutionCompareResult::Identical;
1587+
} else {
1588+
if (cs.isDebugMode()) {
1589+
llvm::errs().indent(cs.solverState->getCurrentIndent())
1590+
<< "- incomparable\n";
1591+
}
1592+
1593+
return SolutionCompareResult::Incomparable;
1594+
}
15371595
}
15381596

15391597
std::optional<unsigned>

‎test/Constraints/bidirectional_conversions.swift‎

Lines changed: 25 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
11
// RUN: %target-typecheck-verify-swift
2-
// REQUIRES: objc_interop
3-
4-
import Foundation
5-
import CoreGraphics
62

73
/////////////
84

@@ -20,26 +16,6 @@ func foo2(x: (Int, Int)?, y: (x: Int, y: Int)) -> G<(Int, Int)> {
2016
return g
2117
}
2218

23-
func foo3(x: (@convention(block) () -> ())?, y: @escaping () -> ()) -> G<@convention(block) () -> ()> {
24-
let g = G(t: x ?? y)
25-
return g
26-
}
27-
28-
func foo4(x: (() -> ())?, y: @escaping @convention(block) () -> ()) -> G<() -> ()> {
29-
let g = G(t: x ?? y)
30-
return g
31-
}
32-
33-
func foo5(x: CGFloat?, y: Double) -> G<CGFloat> {
34-
let g = G(t: x ?? y)
35-
return g
36-
}
37-
38-
func foo6(x: Double?, y: CGFloat) -> G<Double> {
39-
let g = G(t: x ?? y)
40-
return g
41-
}
42-
4319
/////////////
4420

4521
func id<T>(_: T) -> T {}
@@ -54,28 +30,6 @@ func bar2(x: (Int, Int)) {
5430
f(id(x))
5531
}
5632

57-
func bar3(x: @escaping () -> ()) {
58-
func f(_: @escaping @convention(block) () -> ()) {}
59-
// FIXME
60-
f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('@convention(block) () -> ()' vs. '() -> ()')}}
61-
}
62-
63-
func bar4(x: @escaping @convention(block) () -> ()) {
64-
func f(_: @escaping () -> ()) {}
65-
// FIXME
66-
f(id(x)) // expected-error {{conflicting arguments to generic parameter 'T' ('() -> ()' vs. '@convention(block) () -> ()')}}
67-
}
68-
69-
func bar5(x: Double) {
70-
func f(_: CGFloat) {}
71-
f(id(x))
72-
}
73-
74-
func bar6(x: CGFloat) {
75-
func f(_: Double) {}
76-
f(id(x))
77-
}
78-
7933
/////////////
8034

8135
func unwrap<T>(_: T?) -> T {}
@@ -90,23 +44,36 @@ func baz2(x: (Int, Int)?) {
9044
f(unwrap(x))
9145
}
9246

93-
func baz3(x: (() -> ())?) {
94-
func f(_: @escaping @convention(block) () -> ()) {}
95-
f(unwrap(x))
47+
/////////////
48+
49+
func borrowingFn(fn: @escaping (borrowing AnyObject) -> ()) -> (AnyObject) -> () {
50+
return id(id(fn))
9651
}
9752

98-
func baz4(x: (@convention(block) () -> ())?) {
99-
func f(_: @escaping () -> ()) {}
100-
f(unwrap(x))
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)
10161
}
10262

103-
func baz5(x: Double?) {
104-
func f(_: CGFloat) {}
105-
f(unwrap(x))
63+
extension P {
64+
static func >+ (lhs: borrowing Self, rhs: borrowing Self) {}
10665
}
10766

108-
func baz6(x: CGFloat?) {
109-
func f(_: Double) {}
110-
f(unwrap(x))
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() }
11177
}
11278

79+
let _: (MyString, MyString) -> Bool = false ? (<) : (>)

‎test/Constraints/bidirectional_conversions_objc.swift‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// RUN: %target-typecheck-verify-swift
2+
// REQUIRES: objc_interop
3+
4+
import Foundation
5+
import CoreGraphics
6+
7+
/////////////
8+
9+
struct G<T> {
10+
var t: T
11+
}
12+
13+
func foo3(x: (@convention(block) () -> ())?, y: @escaping () -> ()) -> G<@convention(block) () -> ()> {
14+
let g = G(t: x ?? y)
15+
return g
16+
}
17+
18+
func foo4(x: (() -> ())?, y: @escaping @convention(block) () -> ()) -> G<() -> ()> {
19+
let g = G(t: x ?? y)
20+
return g
21+
}
22+
23+
func foo5(x: CGFloat?, y: Double) -> G<CGFloat> {
24+
let g = G(t: x ?? y)
25+
return g
26+
}
27+
28+
func foo6(x: Double?, y: CGFloat) -> G<Double> {
29+
let g = G(t: x ?? y)
30+
return g
31+
}
32+
33+
/////////////
34+
35+
func id<T>(_: T) -> T {}
36+
37+
func bar3(x: @escaping () -> ()) {
38+
func f(_: @escaping @convention(block) () -> ()) {}
39+
f(id(x))
40+
}
41+
42+
func bar4(x: @escaping @convention(block) () -> ()) {
43+
func f(_: @escaping () -> ()) {}
44+
f(id(x))
45+
}
46+
47+
func bar5(x: Double) {
48+
func f(_: CGFloat) {}
49+
f(id(x))
50+
}
51+
52+
func bar6(x: CGFloat) {
53+
func f(_: Double) {}
54+
f(id(x))
55+
}
56+
57+
/////////////
58+
59+
func unwrap<T>(_: T?) -> T {}
60+
61+
func baz3(x: (() -> ())?) {
62+
func f(_: @escaping @convention(block) () -> ()) {}
63+
f(unwrap(x))
64+
}
65+
66+
func baz4(x: (@convention(block) () -> ())?) {
67+
func f(_: @escaping () -> ()) {}
68+
f(unwrap(x))
69+
}
70+
71+
func baz5(x: Double?) {
72+
func f(_: CGFloat) {}
73+
f(unwrap(x))
74+
}
75+
76+
func baz6(x: CGFloat?) {
77+
func f(_: Double) {}
78+
f(unwrap(x))
79+
}

‎test/expr/closure/closures.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ do {
551551
let qux: () -> Void
552552

553553
f(qux)
554-
f(id(qux)) // expected-error {{conflicting arguments to generic parameter 'T' ('() -> Void' vs. '@convention(block) () -> Void')}}
554+
f(id(qux))
555555

556556
func forceUnwrap<T>(_: T?) -> T {}
557557

0 commit comments

Comments
 (0)