Skip to content

[5.0] [CSSolver] Use correct locator when matching function result types related to closures #14926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions lib/SILGen/SILGenPoly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,9 @@ class ResultPlanner {
///
/// Valid: reabstraction info, InnerResult, OuterResult.
ReabstractDirectToDirect,

/// Ignore the next direct inner result, since the outer is 'Void'.
IgnoreDirectResult,
};

Operation(Kind kind) : TheKind(kind) {}
Expand Down Expand Up @@ -1709,6 +1712,24 @@ class ResultPlanner {
SILResultInfo outerResult,
SILValue optOuterResultAddr);

void planIgnoredResult(AbstractionPattern innerOrigType,
CanType innerSubstType, PlanData &planData) {
if (innerOrigType.isTuple()) {
auto innerSubstTuple = cast<TupleType>(innerSubstType);
for (unsigned i = 0, n = innerSubstTuple->getNumElements(); i != n; ++i)
planIgnoredResult(innerOrigType.getTupleElementType(i),
innerSubstTuple.getElementType(i), planData);
return;
}

auto innerResult = claimNextInnerResult(planData);
if (innerResult.isFormalIndirect() &&
SGF.silConv.isSILIndirect(innerResult))
(void)addInnerIndirectResultTemporary(planData, innerResult);
else
addIgnoreDirectResult();
}

/// Claim the next inner result from the plan data.
SILResultInfo claimNextInnerResult(PlanData &data) {
return claimNext(data.InnerResults);
Expand Down Expand Up @@ -1858,6 +1879,10 @@ class ResultPlanner {
op.OuterOrigType = outerOrigType;
op.OuterSubstType = outerSubstType;
}

void addIgnoreDirectResult() {
(void)addOperation(Operation::IgnoreDirectResult);
}
};

} // end anonymous namespace
Expand All @@ -1868,6 +1893,13 @@ void ResultPlanner::plan(AbstractionPattern innerOrigType,
AbstractionPattern outerOrigType,
CanType outerSubstType,
PlanData &planData) {
// Conversion from `() -> T` to `() -> Void` is allowed when
// the argument is a closure.
if (!innerSubstType->isVoid() && outerSubstType->isVoid()) {
planIgnoredResult(innerOrigType, innerSubstType, planData);
return;
}

// The substituted types must match up in tuple-ness and arity.
assert(isa<TupleType>(innerSubstType) == isa<TupleType>(outerSubstType) ||
(isa<TupleType>(innerSubstType) &&
Expand Down Expand Up @@ -2580,6 +2612,10 @@ void ResultPlanner::execute(ArrayRef<SILValue> innerDirectResults,
case Operation::InjectOptionalIndirect:
SGF.B.createInjectEnumAddr(Loc, op.OuterResultAddr, op.SomeDecl);
continue;

case Operation::IgnoreDirectResult:
(void)claimNext(innerDirectResults);
continue;
}
llvm_unreachable("bad operation kind");
}
Expand Down
26 changes: 22 additions & 4 deletions lib/Sema/CSRanking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1027,14 +1027,32 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
// Compare the type variable bindings.
auto &tc = cs.getTypeChecker();
for (auto &binding : diff.typeBindings) {
auto type1 = binding.bindings[idx1];
auto type2 = binding.bindings[idx2];

auto &impl = binding.typeVar->getImpl();

if (auto *locator = impl.getLocator()) {
auto path = locator->getPath();
if (!path.empty() &&
path.back().getKind() == ConstraintLocator::ClosureResult) {
// Since we support `() -> T` to `() -> Void` and
// `() -> Never` to `() -> T` conversions, it's always
// preferable to pick `T` rather than `Never` with
// all else being equal.
if (type2->isUninhabited())
++score1;

if (type1->isUninhabited())
++score2;
}
}

// If the type variable isn't one for which we should be looking at the
// bindings, don't.
if (!binding.typeVar->getImpl().prefersSubtypeBinding())
if (!impl.prefersSubtypeBinding())
continue;

auto type1 = binding.bindings[idx1];
auto type2 = binding.bindings[idx2];

// If the types are equivalent, there's nothing more to do.
if (type1->isEqual(type2))
continue;
Expand Down
35 changes: 30 additions & 5 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1235,10 +1235,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
}

// Result type can be covariant (or equal).
return matchTypes(func1->getResult(), func2->getResult(), subKind,
subflags,
locator.withPathElement(
ConstraintLocator::FunctionResult));
return matchTypes(func1->getResult(), func2->getResult(), subKind, subflags,
locator.withPathElement(ConstraintLocator::FunctionResult));
}

ConstraintSystem::SolutionKind
Expand Down Expand Up @@ -1564,6 +1562,8 @@ ConstraintSystem::SolutionKind
ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
TypeMatchOptions flags,
ConstraintLocatorBuilder locator) {
auto origType1 = type1;

bool isArgumentTupleConversion
= kind == ConstraintKind::ArgumentTupleConversion ||
kind == ConstraintKind::OperatorArgumentTupleConversion;
Expand Down Expand Up @@ -2324,7 +2324,32 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
// Allow '() -> T' to '() -> ()' and '() -> Never' to '() -> T' for closure
// literals.
if (auto elt = locator.last()) {
if (elt->getKind() == ConstraintLocator::ClosureResult) {
auto isClosureResult = [&]() {
if (elt->getKind() == ConstraintLocator::ClosureResult)
return true;

// If constraint is matching function results where
// left-hand side is a 'closure result' we need to allow
// certain implicit conversions.
if (elt->getKind() != ConstraintLocator::FunctionResult)
return false;

if (auto *typeVar = origType1->getAs<TypeVariableType>()) {
auto *locator = typeVar->getImpl().getLocator();
if (!locator)
return false;

auto path = locator->getPath();
if (path.empty())
return false;

return path.back().getKind() == ConstraintLocator::ClosureResult;
}

return false;
};

if (isClosureResult()) {
if (concrete && kind >= ConstraintKind::Subtype &&
(type1->isUninhabited() || type2->isVoid())) {
increaseScore(SK_FunctionConversion);
Expand Down
36 changes: 35 additions & 1 deletion test/Constraints/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,6 @@ func rdar33429010_2() {
let iter = I_33429010()
var acc: Int = 0 // expected-warning {{}}
let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0 + $1 })
// expected-warning@-1 {{result of operator '+' is unused}}
let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0.rdar33429010_incr($1) })
}

Expand All @@ -652,3 +651,38 @@ func rdar36054961() {
str.replaceSubrange(range, with: str[range].reversed())
}])
}

protocol P_37790062 {
associatedtype T
var elt: T { get }
}

func rdar37790062() {
struct S<T> {
init(_ a: () -> T, _ b: () -> T) {}
}

class C1 : P_37790062 {
typealias T = Int
var elt: T { return 42 }
}

class C2 : P_37790062 {
typealias T = (String, Int, Void)
var elt: T { return ("question", 42, ()) }
}

func foo() -> Int { return 42 }
func bar() -> Void {}
func baz() -> (String, Int) { return ("question", 42) }
func bzz<T>(_ a: T) -> T { return a }
func faz<T: P_37790062>(_ a: T) -> T.T { return a.elt }

_ = S({ foo() }, { bar() }) // Ok, should infer T to be 'Void'
_ = S({ baz() }, { bar() }) // Ok, should infer T to be 'Void'
_ = S({ bzz(("question", 42)) }, { bar() }) // Ok
_ = S({ bzz(String.self) }, { bar() }) // Ok
_ = S({ bzz(((), (()))) }, { bar() }) // Ok
_ = S({ bzz(C1()) }, { bar() }) // Ok
_ = S({ faz(C2()) }, { bar() }) // Ok
}
46 changes: 46 additions & 0 deletions test/Constraints/rdar37790062.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// RUN: %target-typecheck-verify-swift

protocol A {
associatedtype V
associatedtype E: Error

init(value: V)
init(error: E)

func foo<U>(value: (V) -> U, error: (E) -> U) -> U
}

enum R<T, E: Error> : A {
case foo(T)
case bar(E)

init(value: T) { self = .foo(value) }
init(error: E) { self = .bar(error) }

func foo<R>(value: (T) -> R, error: (E) -> R) -> R {
fatalError()
}
}

protocol P {
associatedtype V

@discardableResult
func baz(callback: @escaping (V) -> Void) -> Self
}

class C<V> : P {
func baz(callback: @escaping (V) -> Void) -> Self { return self }
}
class D<T, E: Error> : C<R<T, E>> {
init(fn: (_ ret: @escaping (V) -> Void) -> Void) {}
}

extension A where V: P, V.V: A, E == V.V.E {
func bar() -> D<V.V.V, V.V.E> {
return D { complete in
foo(value: { promise in promise.baz { result in } },
error: { complete(R(error: $0)) })
}
}
}
8 changes: 5 additions & 3 deletions test/IRGen/objc_retainAutoreleasedReturnValue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ public func test(_ dict: NSDictionary) {
// popq %rbp ;<== Blocks the handshake from objc_autoreleaseReturnValue
// jmp 0x01ec20 ; symbol stub for: objc_retainAutoreleasedReturnValue

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

// OPT-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue4testyySo12NSDictionaryCFyADcfU_"(%TSo12NSDictionaryC*)
// CHECK-LABEL: define {{.*}}swiftcc void @"$SSo12NSDictionaryCSo12NSEnumeratorCIgxo_ABIgx_TR"(%TSo12NSDictionaryC*, i8*, %swift.refcounted*)

// OPT-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue10useClosureyySo12NSDictionaryC_yADctF06$SSo12h43CSo12NSEnumeratorCIgxo_ABIgx_TR049$S34objc_bcD41Value4testyySo12a6CFSo12B7CADcfU_Tf3npf_nTf1nc_nTf4g_n"(%TSo12NSDictionaryC*)
// OPT: entry:
// OPT: call {{.*}}@objc_msgSend
// OPT: notail call i8* @objc_retainAutoreleasedReturnValue
Expand Down
1 change: 0 additions & 1 deletion test/expr/closure/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ func rdar19179412() -> (Int) -> Int {
func takesVoidFunc(_ f: () -> ()) {}
var i: Int = 1

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