Skip to content

Commit d7f1cdf

Browse files
xedinxedin
authored
Merge pull request #14926 from xedin/rdar-37790062-5.0
[5.0] [CSSolver] Use correct locator when matching function result types related to closures
2 parents 2946e6e + 189819f commit d7f1cdf

File tree

7 files changed

+174
-14
lines changed

7 files changed

+174
-14
lines changed

‎lib/SILGen/SILGenPoly.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,6 +1565,9 @@ class ResultPlanner {
15651565
///
15661566
/// Valid: reabstraction info, InnerResult, OuterResult.
15671567
ReabstractDirectToDirect,
1568+
1569+
/// Ignore the next direct inner result, since the outer is 'Void'.
1570+
IgnoreDirectResult,
15681571
};
15691572

15701573
Operation(Kind kind) : TheKind(kind) {}
@@ -1709,6 +1712,24 @@ class ResultPlanner {
17091712
SILResultInfo outerResult,
17101713
SILValue optOuterResultAddr);
17111714

1715+
void planIgnoredResult(AbstractionPattern innerOrigType,
1716+
CanType innerSubstType, PlanData &planData) {
1717+
if (innerOrigType.isTuple()) {
1718+
auto innerSubstTuple = cast<TupleType>(innerSubstType);
1719+
for (unsigned i = 0, n = innerSubstTuple->getNumElements(); i != n; ++i)
1720+
planIgnoredResult(innerOrigType.getTupleElementType(i),
1721+
innerSubstTuple.getElementType(i), planData);
1722+
return;
1723+
}
1724+
1725+
auto innerResult = claimNextInnerResult(planData);
1726+
if (innerResult.isFormalIndirect() &&
1727+
SGF.silConv.isSILIndirect(innerResult))
1728+
(void)addInnerIndirectResultTemporary(planData, innerResult);
1729+
else
1730+
addIgnoreDirectResult();
1731+
}
1732+
17121733
/// Claim the next inner result from the plan data.
17131734
SILResultInfo claimNextInnerResult(PlanData &data) {
17141735
return claimNext(data.InnerResults);
@@ -1858,6 +1879,10 @@ class ResultPlanner {
18581879
op.OuterOrigType = outerOrigType;
18591880
op.OuterSubstType = outerSubstType;
18601881
}
1882+
1883+
void addIgnoreDirectResult() {
1884+
(void)addOperation(Operation::IgnoreDirectResult);
1885+
}
18611886
};
18621887

18631888
} // end anonymous namespace
@@ -1868,6 +1893,13 @@ void ResultPlanner::plan(AbstractionPattern innerOrigType,
18681893
AbstractionPattern outerOrigType,
18691894
CanType outerSubstType,
18701895
PlanData &planData) {
1896+
// Conversion from `() -> T` to `() -> Void` is allowed when
1897+
// the argument is a closure.
1898+
if (!innerSubstType->isVoid() && outerSubstType->isVoid()) {
1899+
planIgnoredResult(innerOrigType, innerSubstType, planData);
1900+
return;
1901+
}
1902+
18711903
// The substituted types must match up in tuple-ness and arity.
18721904
assert(isa<TupleType>(innerSubstType) == isa<TupleType>(outerSubstType) ||
18731905
(isa<TupleType>(innerSubstType) &&
@@ -2580,6 +2612,10 @@ void ResultPlanner::execute(ArrayRef<SILValue> innerDirectResults,
25802612
case Operation::InjectOptionalIndirect:
25812613
SGF.B.createInjectEnumAddr(Loc, op.OuterResultAddr, op.SomeDecl);
25822614
continue;
2615+
2616+
case Operation::IgnoreDirectResult:
2617+
(void)claimNext(innerDirectResults);
2618+
continue;
25832619
}
25842620
llvm_unreachable("bad operation kind");
25852621
}

‎lib/Sema/CSRanking.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,14 +1027,32 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
10271027
// Compare the type variable bindings.
10281028
auto &tc = cs.getTypeChecker();
10291029
for (auto &binding : diff.typeBindings) {
1030+
auto type1 = binding.bindings[idx1];
1031+
auto type2 = binding.bindings[idx2];
1032+
1033+
auto &impl = binding.typeVar->getImpl();
1034+
1035+
if (auto *locator = impl.getLocator()) {
1036+
auto path = locator->getPath();
1037+
if (!path.empty() &&
1038+
path.back().getKind() == ConstraintLocator::ClosureResult) {
1039+
// Since we support `() -> T` to `() -> Void` and
1040+
// `() -> Never` to `() -> T` conversions, it's always
1041+
// preferable to pick `T` rather than `Never` with
1042+
// all else being equal.
1043+
if (type2->isUninhabited())
1044+
++score1;
1045+
1046+
if (type1->isUninhabited())
1047+
++score2;
1048+
}
1049+
}
1050+
10301051
// If the type variable isn't one for which we should be looking at the
10311052
// bindings, don't.
1032-
if (!binding.typeVar->getImpl().prefersSubtypeBinding())
1053+
if (!impl.prefersSubtypeBinding())
10331054
continue;
10341055

1035-
auto type1 = binding.bindings[idx1];
1036-
auto type2 = binding.bindings[idx2];
1037-
10381056
// If the types are equivalent, there's nothing more to do.
10391057
if (type1->isEqual(type2))
10401058
continue;

‎lib/Sema/CSSimplify.cpp

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,10 +1235,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
12351235
}
12361236

12371237
// Result type can be covariant (or equal).
1238-
return matchTypes(func1->getResult(), func2->getResult(), subKind,
1239-
subflags,
1240-
locator.withPathElement(
1241-
ConstraintLocator::FunctionResult));
1238+
return matchTypes(func1->getResult(), func2->getResult(), subKind, subflags,
1239+
locator.withPathElement(ConstraintLocator::FunctionResult));
12421240
}
12431241

12441242
ConstraintSystem::SolutionKind
@@ -1564,6 +1562,8 @@ ConstraintSystem::SolutionKind
15641562
ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
15651563
TypeMatchOptions flags,
15661564
ConstraintLocatorBuilder locator) {
1565+
auto origType1 = type1;
1566+
15671567
bool isArgumentTupleConversion
15681568
= kind == ConstraintKind::ArgumentTupleConversion ||
15691569
kind == ConstraintKind::OperatorArgumentTupleConversion;
@@ -2324,7 +2324,32 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
23242324
// Allow '() -> T' to '() -> ()' and '() -> Never' to '() -> T' for closure
23252325
// literals.
23262326
if (auto elt = locator.last()) {
2327-
if (elt->getKind() == ConstraintLocator::ClosureResult) {
2327+
auto isClosureResult = [&]() {
2328+
if (elt->getKind() == ConstraintLocator::ClosureResult)
2329+
return true;
2330+
2331+
// If constraint is matching function results where
2332+
// left-hand side is a 'closure result' we need to allow
2333+
// certain implicit conversions.
2334+
if (elt->getKind() != ConstraintLocator::FunctionResult)
2335+
return false;
2336+
2337+
if (auto *typeVar = origType1->getAs<TypeVariableType>()) {
2338+
auto *locator = typeVar->getImpl().getLocator();
2339+
if (!locator)
2340+
return false;
2341+
2342+
auto path = locator->getPath();
2343+
if (path.empty())
2344+
return false;
2345+
2346+
return path.back().getKind() == ConstraintLocator::ClosureResult;
2347+
}
2348+
2349+
return false;
2350+
};
2351+
2352+
if (isClosureResult()) {
23282353
if (concrete && kind >= ConstraintKind::Subtype &&
23292354
(type1->isUninhabited() || type2->isVoid())) {
23302355
increaseScore(SK_FunctionConversion);

‎test/Constraints/closures.swift

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,6 @@ func rdar33429010_2() {
630630
let iter = I_33429010()
631631
var acc: Int = 0 // expected-warning {{}}
632632
let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0 + $1 })
633-
// expected-warning@-1 {{result of operator '+' is unused}}
634633
let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0.rdar33429010_incr($1) })
635634
}
636635

@@ -652,3 +651,38 @@ func rdar36054961() {
652651
str.replaceSubrange(range, with: str[range].reversed())
653652
}])
654653
}
654+
655+
protocol P_37790062 {
656+
associatedtype T
657+
var elt: T { get }
658+
}
659+
660+
func rdar37790062() {
661+
struct S<T> {
662+
init(_ a: () -> T, _ b: () -> T) {}
663+
}
664+
665+
class C1 : P_37790062 {
666+
typealias T = Int
667+
var elt: T { return 42 }
668+
}
669+
670+
class C2 : P_37790062 {
671+
typealias T = (String, Int, Void)
672+
var elt: T { return ("question", 42, ()) }
673+
}
674+
675+
func foo() -> Int { return 42 }
676+
func bar() -> Void {}
677+
func baz() -> (String, Int) { return ("question", 42) }
678+
func bzz<T>(_ a: T) -> T { return a }
679+
func faz<T: P_37790062>(_ a: T) -> T.T { return a.elt }
680+
681+
_ = S({ foo() }, { bar() }) // Ok, should infer T to be 'Void'
682+
_ = S({ baz() }, { bar() }) // Ok, should infer T to be 'Void'
683+
_ = S({ bzz(("question", 42)) }, { bar() }) // Ok
684+
_ = S({ bzz(String.self) }, { bar() }) // Ok
685+
_ = S({ bzz(((), (()))) }, { bar() }) // Ok
686+
_ = S({ bzz(C1()) }, { bar() }) // Ok
687+
_ = S({ faz(C2()) }, { bar() }) // Ok
688+
}

‎test/Constraints/rdar37790062.swift

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
protocol A {
4+
associatedtype V
5+
associatedtype E: Error
6+
7+
init(value: V)
8+
init(error: E)
9+
10+
func foo<U>(value: (V) -> U, error: (E) -> U) -> U
11+
}
12+
13+
enum R<T, E: Error> : A {
14+
case foo(T)
15+
case bar(E)
16+
17+
init(value: T) { self = .foo(value) }
18+
init(error: E) { self = .bar(error) }
19+
20+
func foo<R>(value: (T) -> R, error: (E) -> R) -> R {
21+
fatalError()
22+
}
23+
}
24+
25+
protocol P {
26+
associatedtype V
27+
28+
@discardableResult
29+
func baz(callback: @escaping (V) -> Void) -> Self
30+
}
31+
32+
class C<V> : P {
33+
func baz(callback: @escaping (V) -> Void) -> Self { return self }
34+
}
35+
class D<T, E: Error> : C<R<T, E>> {
36+
init(fn: (_ ret: @escaping (V) -> Void) -> Void) {}
37+
}
38+
39+
extension A where V: P, V.V: A, E == V.V.E {
40+
func bar() -> D<V.V.V, V.V.E> {
41+
return D { complete in
42+
foo(value: { promise in promise.baz { result in } },
43+
error: { complete(R(error: $0)) })
44+
}
45+
}
46+
}

‎test/IRGen/objc_retainAutoreleasedReturnValue.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,15 @@ public func test(_ dict: NSDictionary) {
2424
// popq %rbp ;<== Blocks the handshake from objc_autoreleaseReturnValue
2525
// jmp 0x01ec20 ; symbol stub for: objc_retainAutoreleasedReturnValue
2626

27-
// CHECK-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue4testyySo12NSDictionaryCFyADcfU_"(%TSo12NSDictionaryC*)
27+
// CHECK-LABEL: define {{.*}}swiftcc %TSo12NSEnumeratorC* @"$S34objc_retainAutoreleasedReturnValue4testyySo12NSDictionaryCFSo12NSEnumeratorCADcfU_"(%TSo12NSDictionaryC*)
2828
// CHECK: entry:
2929
// CHECK: call {{.*}}@objc_msgSend
3030
// CHECK: notail call i8* @objc_retainAutoreleasedReturnValue
31-
// CHECK: ret void
31+
// CHECK: ret %TSo12NSEnumeratorC*
3232

33-
// OPT-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue4testyySo12NSDictionaryCFyADcfU_"(%TSo12NSDictionaryC*)
33+
// CHECK-LABEL: define {{.*}}swiftcc void @"$SSo12NSDictionaryCSo12NSEnumeratorCIgxo_ABIgx_TR"(%TSo12NSDictionaryC*, i8*, %swift.refcounted*)
34+
35+
// OPT-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue10useClosureyySo12NSDictionaryC_yADctF06$SSo12h43CSo12NSEnumeratorCIgxo_ABIgx_TR049$S34objc_bcD41Value4testyySo12a6CFSo12B7CADcfU_Tf3npf_nTf1nc_nTf4g_n"(%TSo12NSDictionaryC*)
3436
// OPT: entry:
3537
// OPT: call {{.*}}@objc_msgSend
3638
// OPT: notail call i8* @objc_retainAutoreleasedReturnValue

‎test/expr/closure/closures.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ func rdar19179412() -> (Int) -> Int {
259259
func takesVoidFunc(_ f: () -> ()) {}
260260
var i: Int = 1
261261

262-
// expected-warning @+1 {{expression of type 'Int' is unused}}
263262
takesVoidFunc({i})
264263
// expected-warning @+1 {{expression of type 'Int' is unused}}
265264
var f1: () -> () = {i}

0 commit comments

Comments
 (0)