From 2c9d05991f6629558447efe78e82978f6e69388c Mon Sep 17 00:00:00 2001
From: Pavel Yaskevich <xedin@apache.org>
Date: Mon, 16 Sep 2024 09:54:10 -0700
Subject: [PATCH] [CSBindings] Prevent `BindingSet::isViable` from dropping
 viable bindings

I think the original idea was to elide `Array<$T>` if there is
a binding a resolved generic arguments i.e. `Array<Float>`, but
the check doesn't account for the fact that bindings could be
of different kinds and there are some implicit conversions that
could be missed if we remove the bindings.

For example, given the following constraints:

`Array<$T0> conv $T1`
`$T1 conv Array<(String, Int)>`

`$T0` can be a supertype of `Array<$T0>` and subtype of `Array<(String, Int)>`.

The solver should accept both types as viable bindings because the
`$T0` could be bound to `(key: String, value: Int)` and that would
match `Array<(String, Int)>` conversion.
---
 lib/Sema/CSBindings.cpp                         | 10 ++++++++++
 test/Constraints/array_literal.swift            | 10 ++++++++++
 validation-test/Sema/SwiftUI/rdar98862079.swift |  4 ++--
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp
index 2074b0e39d191..ab91d02e49d6b 100644
--- a/lib/Sema/CSBindings.cpp
+++ b/lib/Sema/CSBindings.cpp
@@ -1205,6 +1205,16 @@ 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)
+      continue;
+
     // If new type has a type variable it shouldn't
     // be considered  viable.
     if (type->hasTypeVariable())
diff --git a/test/Constraints/array_literal.swift b/test/Constraints/array_literal.swift
index 575c61fa2f8f2..b49ceffd46875 100644
--- a/test/Constraints/array_literal.swift
+++ b/test/Constraints/array_literal.swift
@@ -415,3 +415,13 @@ do {
     ]
   }
 }
+
+
+do {
+  func f<R>(fn: () -> [R]) -> [R] { [] }
+
+  // Requires collection upcast from Array<(key: String, value: String)> to `Array<(String, String)>`
+  func g(v: [String: String]) {
+    let _: [(String, String)] = f { return Array(v) } + v // Ok
+  }
+}
diff --git a/validation-test/Sema/SwiftUI/rdar98862079.swift b/validation-test/Sema/SwiftUI/rdar98862079.swift
index dcc99e335a17d..40a2030539118 100644
--- a/validation-test/Sema/SwiftUI/rdar98862079.swift
+++ b/validation-test/Sema/SwiftUI/rdar98862079.swift
@@ -22,7 +22,7 @@ struct MyView : View {
 
   var body: some View {
     test(value: true ? $newBounds.maxBinding : $newBounds.minBinding, in: bounds)
-    // expected-error@-1 2 {{result values in '? :' expression have mismatching types 'Binding<Binding<Double>?>' and 'Binding<Double>'}}
-    // expected-note@-2 2 {{arguments to generic parameter 'Value' ('Binding<Double>?' and 'Double') are expected to be equal}}
+    // expected-error@-1 {{cannot convert value of type 'Binding<Binding<Double>?>' to expected argument type 'Binding<Double>'}}
+    // expected-note@-2 {{arguments to generic parameter 'Value' ('Binding<Double>?' and 'Double') are expected to be equal}}
   }
 }