Skip to content

Commit 9d7bd09

Browse files
committed
[CSBindings] Don't attempt to strip optional for closure result types
Let's not perform $T? -> $T for closure result types to avoid having to re-discover solutions that differ only in location of optional injection. The pattern with such type variables is: ``` $T_body <conv/subtype> $T_result <conv/subtype> $T_contextual_result ``` When `$T_contextual_result` is `Optional<$U>`, the optional injection can either happen from `$T_body` or from `$T_result` (if `return` expression is non-optional), if we allow both the solver would find two solutions that differ only in location of optional injection.
1 parent 8f2e2f7 commit 9d7bd09

File tree

3 files changed

+33
-11
lines changed

3 files changed

+33
-11
lines changed

lib/Sema/CSBindings.cpp

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2447,17 +2447,35 @@ bool TypeVarBindingProducer::computeNext() {
24472447
if (binding.Kind == BindingKind::Subtypes || CS.shouldAttemptFixes()) {
24482448
// If we were unsuccessful solving for T?, try solving for T.
24492449
if (auto objTy = type->getOptionalObjectType()) {
2450-
// If T is a type variable, only attempt this if both the
2451-
// type variable we are trying bindings for, and the type
2452-
// variable we will attempt to bind, both have the same
2453-
// polarity with respect to being able to bind lvalues.
2454-
if (auto otherTypeVar = objTy->getAs<TypeVariableType>()) {
2455-
if (TypeVar->getImpl().canBindToLValue() ==
2456-
otherTypeVar->getImpl().canBindToLValue()) {
2457-
addNewBinding(binding.withSameSource(objTy, binding.Kind));
2450+
// TODO: This could be generalized in the future to cover all patterns
2451+
// that have an intermediate type variable in subtype/conversion chain.
2452+
//
2453+
// Let's not perform $T? -> $T for closure result types to avoid having
2454+
// to re-discover solutions that differ only in location of optional
2455+
// injection.
2456+
//
2457+
// The pattern with such type variables is:
2458+
//
2459+
// $T_body <conv/subtype> $T_result <conv/subtype> $T_contextual_result
2460+
//
2461+
// When $T_contextual_result is Optional<$U>, the optional injection
2462+
// can either happen from $T_body or from $T_result (if `return`
2463+
// expression is non-optional), if we allow both the solver would
2464+
// find two solutions that differ only in location of optional
2465+
// injection.
2466+
if (!TypeVar->getImpl().isClosureResultType()) {
2467+
// If T is a type variable, only attempt this if both the
2468+
// type variable we are trying bindings for, and the type
2469+
// variable we will attempt to bind, both have the same
2470+
// polarity with respect to being able to bind lvalues.
2471+
if (auto otherTypeVar = objTy->getAs<TypeVariableType>()) {
2472+
if (TypeVar->getImpl().canBindToLValue() ==
2473+
otherTypeVar->getImpl().canBindToLValue()) {
2474+
addNewBinding(binding.withType(objTy));
2475+
}
2476+
} else {
2477+
addNewBinding(binding.withType(objTy));
24582478
}
2459-
} else {
2460-
addNewBinding(binding.withSameSource(objTy, binding.Kind));
24612479
}
24622480
}
24632481
}

test/Constraints/argument_matching.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,15 +1488,19 @@ func trailingclosure4(f: () -> Int) {}
14881488
trailingclosure4 { 5 }
14891489

14901490
func trailingClosure5<T>(_ file: String = #file, line: UInt = #line, expression: () -> T?) { }
1491+
// expected-note@-1 {{in call to function 'trailingClosure5(_:line:expression:)'}}
14911492
func trailingClosure6<T>(value: Int, expression: () -> T?) { }
1493+
// expected-note@-1 {{in call to function 'trailingClosure6(value:expression:)'}}
14921494

14931495
trailingClosure5(file: "hello", line: 17) { // expected-error{{extraneous argument label 'file:' in call}}{{18-24=}}
1496+
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
14941497
return Optional.Some(5)
14951498
// expected-error@-1 {{enum type 'Optional<Wrapped>' has no case 'Some'; did you mean 'some'?}} {{19-23=some}}
14961499
// expected-error@-2 {{generic parameter 'Wrapped' could not be inferred}}
14971500
// expected-note@-3 {{explicitly specify the generic arguments to fix this issue}}
14981501
}
14991502
trailingClosure6(5) { // expected-error{{missing argument label 'value:' in call}}{{18-18=value: }}
1503+
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
15001504
return Optional.Some(5)
15011505
// expected-error@-1 {{enum type 'Optional<Wrapped>' has no case 'Some'; did you mean 'some'?}} {{19-23=some}}
15021506
// expected-error@-2 {{generic parameter 'Wrapped' could not be inferred}}

validation-test/Sema/type_checker_perf/slow/rdar22079400.swift renamed to validation-test/Sema/type_checker_perf/fast/rdar22079400.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1
22
// REQUIRES: tools-release,no_asan
33

4-
let _ = (0...1).lazy.flatMap { // expected-error {{reasonable time}}
4+
let _ = (0...1).lazy.flatMap {
55
a in (1...2).lazy.map { b in (a, b) }
66
}.filter {
77
1 < $0 && $0 < $1 && $0 + $1 < 3

0 commit comments

Comments
 (0)