Skip to content

Commit 6a4b7e7

Browse files
committed
[CSOptimizer] Prevent candidate inference from unresolved generic parameters and ternary expressions
We need to have a notion of "complete" binding set before we can allow inference from generic parameters and ternary, otherwise we'd make a favoring decision that might not be correct i.e. `v ?? (<<cond>> ? nil : o)` where `o` is `Int`. `getBindingsFor` doesn't currently infer transitive bindings which means that for a ternary we'd only have a single binding - `Int` which could lead to favoring overload of `??` and has non-optional parameter on the right-hand side.
1 parent 8d177da commit 6a4b7e7

File tree

4 files changed

+40
-0
lines changed

4 files changed

+40
-0
lines changed

include/swift/Sema/ConstraintSystem.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,10 @@ class TypeVariableType::Implementation {
496496
/// function call.
497497
bool isFunctionResult() const;
498498

499+
/// Determine whether this type variable represents a type of the ternary
500+
/// operator.
501+
bool isTernary() const;
502+
499503
/// Retrieve the representative of the equivalence class to which this
500504
/// type variable belongs.
501505
///

lib/Sema/CSOptimizer.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,18 @@ static void determineBestChoicesInContext(
857857
if (auto *typeVar = argType->getAs<TypeVariableType>()) {
858858
auto bindingSet = cs.getBindingsFor(typeVar);
859859

860+
// We need to have a notion of "complete" binding set before
861+
// we can allow inference from generic parameters and ternary,
862+
// otherwise we'd make a favoring decision that might not be
863+
// correct i.e. `v ?? (<<cond>> ? nil : o)` where `o` is `Int`.
864+
// `getBindingsFor` doesn't currently infer transitive bindings
865+
// which means that for a ternary we'd only have a single
866+
// binding - `Int` which could lead to favoring overload of
867+
// `??` and has non-optional parameter on the right-hand side.
868+
if (typeVar->getImpl().getGenericParameter() ||
869+
typeVar->getImpl().isTernary())
870+
continue;
871+
860872
auto restoreOptionality = [](Type type, unsigned numOptionals) {
861873
for (unsigned i = 0; i != numOptionals; ++i)
862874
type = type->wrapInOptionalType();

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,10 @@ bool TypeVariableType::Implementation::isFunctionResult() const {
212212
return locator && locator->isLastElement<LocatorPathElt::FunctionResult>();
213213
}
214214

215+
bool TypeVariableType::Implementation::isTernary() const {
216+
return locator && locator->directlyAt<TernaryExpr>();
217+
}
218+
215219
void *operator new(size_t bytes, ConstraintSystem& cs,
216220
size_t alignment) {
217221
return cs.getAllocator().Allocate(bytes, alignment);

test/Constraints/nil-coalescing-favoring.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,23 @@ extension Array where Element == UInt8 {
4242
fatalError()
4343
}
4444
}
45+
46+
func test_no_incorrect_favoring(v: Int?, o: Int) {
47+
func ternary<T>(_: T, _: T) -> T { fatalError() }
48+
49+
func nilCoelesing<T>(_: T?, _: T) -> T { fatalError() }
50+
func nilCoelesing<T>(_: T?, _: T?) -> T? { fatalError() }
51+
52+
let t1 = v ?? (true ? nil : v)
53+
let t2 = v ?? ternary(nil, o)
54+
55+
let s1 = nilCoelesing(v, (true ? nil : v))
56+
let s2 = nilCoelesing(v, ternary(nil, o))
57+
58+
func sameType<T>(_: T, as: T.Type) {}
59+
60+
sameType(t1, as: Int?.self)
61+
sameType(t2, as: Int?.self)
62+
sameType(s1, as: Int?.self)
63+
sameType(s2, as: Int?.self)
64+
}

0 commit comments

Comments
 (0)