Skip to content

Commit d99242c

Browse files
xedinsusmonteiro
authored andcommitted
[CSBindings] Prevent BindingSet::isViable from dropping viable bindings (v2)
The original attempt to do this was reverted by swiftlang#77653 The problem is that the fix was too broad, I narrowed it down to non-exact uses of stdlib collections that support upcasts. (cherry picked from commit b7e7493)
1 parent 79aec31 commit d99242c

File tree

1 file changed

+31
-2
lines changed

1 file changed

+31
-2
lines changed

lib/Sema/CSBindings.cpp

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,19 @@ using namespace swift;
3131
using namespace constraints;
3232
using namespace inference;
3333

34-
3534
void ConstraintGraphNode::initBindingSet() {
3635
ASSERT(!hasBindingSet());
3736
ASSERT(forRepresentativeVar());
3837

3938
Set.emplace(CG.getConstraintSystem(), TypeVar, Potential);
4039
}
4140

41+
/// Check whether there exists a type that could be implicitly converted
42+
/// to a given type i.e. is the given type is Double or Optional<..> this
43+
/// function is going to return true because CGFloat could be converted
44+
/// to a Double and non-optional value could be injected into an optional.
45+
static bool hasConversions(Type);
46+
4247
static std::optional<Type> checkTypeOfBinding(TypeVariableType *typeVar,
4348
Type type);
4449

@@ -1348,7 +1353,31 @@ bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) {
13481353
if (!existingNTD || NTD != existingNTD)
13491354
continue;
13501355

1351-
// FIXME: What is going on here needs to be thoroughly re-evaluated.
1356+
// What is going on in this method needs to be thoroughly re-evaluated!
1357+
//
1358+
// This logic aims to skip dropping bindings if
1359+
// collection type has conversions i.e. in situations like:
1360+
//
1361+
// [$T1] conv $T2
1362+
// $T2 conv [(Int, String)]
1363+
// $T2.Element equal $T5.Element
1364+
//
1365+
// `$T1` could be bound to `(i: Int, v: String)` after
1366+
// `$T2` is bound to `[(Int, String)]` which is is a problem
1367+
// because it means that `$T2` was attempted to early
1368+
// before the solver had a chance to discover all viable
1369+
// bindings.
1370+
//
1371+
// Let's say existing binding is `[(Int, String)]` and
1372+
// relation is "exact", in this case there is no point
1373+
// tracking `[$T1]` because upcasts are only allowed for
1374+
// subtype and other conversions.
1375+
if (existing->Kind != AllowedBindingKind::Exact) {
1376+
if (existingType->isKnownStdlibCollectionType() &&
1377+
hasConversions(existingType)) {
1378+
continue;
1379+
}
1380+
}
13521381

13531382
// If new type has a type variable it shouldn't
13541383
// be considered viable.

0 commit comments

Comments
 (0)