From 9bf11bc94e572ed572027194f2da994a673f5e27 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 15 Nov 2024 11:41:22 -0800 Subject: [PATCH 1/4] [CSBindings] Revert changes in `BindingSet::isViable` This change although correct cases performance issues in some scenarios. (cherry picked from commit d01c9cbaee99d9de45639c6e6e54391f063c211a) --- lib/Sema/CSBindings.cpp | 14 +---------- test/Constraints/rdar139812024.swift | 37 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 test/Constraints/rdar139812024.swift diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 30f3d7b346570..8426f7f544ada 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1208,19 +1208,7 @@ bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) { if (!existingNTD || NTD != existingNTD) continue; - // What is going on here needs to be thoroughly re-evaluated, - // but at least for now, let's not filter bindings of different - // kinds so if we have a situation like: `Array<$T0> conv $T1` - // and `$T1 conv Array<(String, Int)>` we can't lose `Array<$T0>` - // as a binding because `$T0` could be inferred to - // `(key: String, value: Int)` and binding `$T1` to `Array<(String, Int)>` - // eagerly would be incorrect. - if (existing->Kind != binding.Kind) { - // Array, Set and Dictionary allow conversions, everything else - // requires their generic arguments to match exactly. - if (existingType->isKnownStdlibCollectionType()) - continue; - } + // FIXME: What is going on here needs to be thoroughly re-evaluated. // If new type has a type variable it shouldn't // be considered viable. diff --git a/test/Constraints/rdar139812024.swift b/test/Constraints/rdar139812024.swift new file mode 100644 index 0000000000000..9f5ab0a0d8862 --- /dev/null +++ b/test/Constraints/rdar139812024.swift @@ -0,0 +1,37 @@ +// RUN: %target-typecheck-verify-swift %clang-importer-sdk + +// REQUIRES: objc_interop + +// rdar://139812024 - error: cannot convert return expression of type 'NSObject' to return type 'NSImage' + +import Foundation + +@objc +final class Image: NSObject { +} + +@available(*, unavailable) +extension Image: Sendable {} // expected-note {{explicitly marked unavailable here}} + +class Lock { + func withLock(_: @Sendable (inout State) -> R) -> R { + fatalError() + } +} + +extension Lock where State == Void { + func withLock(_: @Sendable () -> R) -> R { + fatalError() + } +} + +class Test { + var images: [Int: Image] = [:] + + func fetchImage(lock: Lock, id: Int) -> Image? { + if let existingImage = lock.withLock({ return images[id] }) { // expected-warning {{unavailable}} + return existingImage + } + return nil + } +} From fc26ad37da8f8a1e6cd03c7abcc84a3cbcaceb46 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 3 Dec 2024 13:15:46 -0800 Subject: [PATCH 2/4] Revert "[CSBindings] Don't favor application result types before application happens" This reverts commit bc949c3680521eaaf6200693064239d70d3beca4. (cherry picked from commit 8aa6280bf3dd5c8fe42e3e8632ae55c783ef8e2e) --- include/swift/Sema/ConstraintSystem.h | 4 ---- lib/Sema/CSBindings.cpp | 13 +++---------- lib/Sema/TypeCheckConstraints.cpp | 10 ---------- test/Constraints/async.swift | 17 ----------------- 4 files changed, 3 insertions(+), 41 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 1615f3a103244..c0a305f1b8f5a 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -486,10 +486,6 @@ class TypeVariableType::Implementation { /// Determine whether this type variable represents a subscript result type. bool isSubscriptResultType() const; - /// Determine whether this type variable represents a result type of an - /// application i.e. a call, an operator, or a subscript. - bool isApplicationResultType() const; - /// Determine whether this type variable represents an opened /// type parameter pack. bool isParameterPack() const; diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 8426f7f544ada..49f8d1dcabcc5 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1281,16 +1281,9 @@ bool BindingSet::favoredOverDisjunction(Constraint *disjunction) const { return !hasConversions(binding.BindingType); })) { - bool isApplicationResultType = TypeVar->getImpl().isApplicationResultType(); - if (llvm::none_of(Info.DelayedBy, [&isApplicationResultType]( - const Constraint *constraint) { - // Let's not attempt to bind result type before application - // happens. For example because it could be discardable or - // l-value (subscript applications). - if (isApplicationResultType && - constraint->getKind() == ConstraintKind::ApplicableFunction) - return true; - + // Result type of subscript could be l-value so we can't bind it early. + if (!TypeVar->getImpl().isSubscriptResultType() && + llvm::none_of(Info.DelayedBy, [](const Constraint *constraint) { return constraint->getKind() == ConstraintKind::Disjunction || constraint->getKind() == ConstraintKind::ValueMember; })) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index d39452130d23f..828ef6b005449 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -175,16 +175,6 @@ bool TypeVariableType::Implementation::isSubscriptResultType() const { KeyPathExpr::Component::Kind::UnresolvedSubscript; } -bool TypeVariableType::Implementation::isApplicationResultType() const { - if (!(locator && locator->getAnchor())) - return false; - - if (!locator->isLastElement()) - return false; - - return isExpr(locator->getAnchor()) || isSubscriptResultType(); -} - bool TypeVariableType::Implementation::isParameterPack() const { return locator && locator->isForGenericParameter() diff --git a/test/Constraints/async.swift b/test/Constraints/async.swift index f914d49b56372..732fd3b5f829d 100644 --- a/test/Constraints/async.swift +++ b/test/Constraints/async.swift @@ -180,20 +180,3 @@ struct OverloadInImplicitAsyncClosure { init(int: Int) throws { } } - -@available(SwiftStdlib 5.5, *) -func test(_: Int) async throws {} - -@discardableResult -@available(SwiftStdlib 5.5, *) -func test(_: Int) -> String { "" } - -@available(SwiftStdlib 5.5, *) -func compute(_: @escaping () -> Void) {} - -@available(SwiftStdlib 5.5, *) -func test_sync_in_closure_context() { - compute { - test(42) // Ok (select sync overloads and discards the result) - } -} From c7fb93d99e7daf8cc4edb17671b68629f0d9232a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 3 Dec 2024 13:15:49 -0800 Subject: [PATCH 3/4] Revert "[CSBindings] Adjust `hasConversions` to handle `Void` has having not conversions" This reverts commit 76f0bcb05c91a5ec9ef6c011570b12adce5acaea. (cherry picked from commit ffac97a1eea5d21fb94b2cd838312e87aed24797) --- lib/Sema/CSBindings.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 49f8d1dcabcc5..7e6a1942faa6b 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1264,8 +1264,7 @@ static bool hasConversions(Type type) { } return !(type->is() || type->is() || - type->is() || type->is() || - type->isVoid()); + type->is() || type->is()); } bool BindingSet::favoredOverDisjunction(Constraint *disjunction) const { From c55c2b6ad0e711d26cc0da2e7a56b35d85ff947f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 3 Dec 2024 13:17:25 -0800 Subject: [PATCH 4/4] [Tests] NFC: Bring back test-case that makes sure that closures discard result correctly (cherry picked from commit 92a1b1fd66250083422faf53f018278a3579bbff) --- test/Constraints/async.swift | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/Constraints/async.swift b/test/Constraints/async.swift index 732fd3b5f829d..f914d49b56372 100644 --- a/test/Constraints/async.swift +++ b/test/Constraints/async.swift @@ -180,3 +180,20 @@ struct OverloadInImplicitAsyncClosure { init(int: Int) throws { } } + +@available(SwiftStdlib 5.5, *) +func test(_: Int) async throws {} + +@discardableResult +@available(SwiftStdlib 5.5, *) +func test(_: Int) -> String { "" } + +@available(SwiftStdlib 5.5, *) +func compute(_: @escaping () -> Void) {} + +@available(SwiftStdlib 5.5, *) +func test_sync_in_closure_context() { + compute { + test(42) // Ok (select sync overloads and discards the result) + } +}