Skip to content

Commit bf8ae3b

Browse files
committed
[CSOptimizer] Allow only widening CGFloat->Double conversions while matching candidate arguments
Allow CGFloat -> Double widening conversions between candidate argument types and parameter types. This would make sure that Double is always preferred over CGFloat when using literals and ranking supported disjunction choices. Narrowing conversion (Double -> CGFloat) should be delayed as much as possible.
1 parent cb876cb commit bf8ae3b

File tree

2 files changed

+59
-11
lines changed

2 files changed

+59
-11
lines changed

lib/Sema/CSOptimizer.cpp

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -530,18 +530,22 @@ static void determineBestChoicesInContext(
530530
scoreCandidateMatch =
531531
[&](GenericSignature genericSig, Type candidateType, Type paramType,
532532
MatchOptions options) -> std::optional<double> {
533-
auto areEqual = [&options](Type a, Type b) {
534-
// Double<->CGFloat implicit conversion support for literals
535-
// only since in this case the conversion might not result in
536-
// score penalty.
537-
if (options.contains(MatchFlag::Literal) &&
538-
((a->isDouble() && b->isCGFloat()) ||
539-
(a->isCGFloat() && b->isDouble())))
540-
return true;
541-
533+
auto areEqual = [&](Type a, Type b) {
542534
return a->getDesugaredType()->isEqual(b->getDesugaredType());
543535
};
544536

537+
// Allow CGFloat -> Double widening conversions between
538+
// candidate argument types and parameter types. This would
539+
// make sure that Double is always preferred over CGFloat
540+
// when using literals and ranking supported disjunction
541+
// choices. Narrowing conversion (Double -> CGFloat) should
542+
// be delayed as much as possible.
543+
if (options.contains(MatchFlag::OnParam)) {
544+
if (candidateType->isCGFloat() && paramType->isDouble()) {
545+
return options.contains(MatchFlag::Literal) ? 0.2 : 0.9;
546+
}
547+
}
548+
545549
if (options.contains(MatchFlag::ExactOnly))
546550
return areEqual(candidateType, paramType) ? 1 : 0;
547551

@@ -559,12 +563,12 @@ static void determineBestChoicesInContext(
559563
if (candidateType->isInt() &&
560564
TypeChecker::conformsToKnownProtocol(
561565
paramType, KnownProtocolKind::ExpressibleByIntegerLiteral))
562-
return 0.2;
566+
return paramType->isDouble() ? 0.2 : 0.3;
563567

564568
if (candidateType->isDouble() &&
565569
TypeChecker::conformsToKnownProtocol(
566570
paramType, KnownProtocolKind::ExpressibleByFloatLiteral))
567-
return 0.2;
571+
return 0.3;
568572
}
569573

570574
return 0;
@@ -916,6 +920,19 @@ static void determineBestChoicesInContext(
916920
// with a lot of favored overloads because on the result type alone.
917921
if (decl->isOperator() && !isStandardComparisonOperator(decl)) {
918922
if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) {
923+
// Avoid increasing weight based on CGFloat result type
924+
// match because that could require narrowing conversion
925+
// in the arguments and that is always detrimental.
926+
//
927+
// For example, `has_CGFloat_param(1.0 + 2.0)` should use
928+
// `+(_: Double, _: Double) -> Double` instead of
929+
// `+(_: CGFloat, _: CGFloat) -> CGFloat` which would match
930+
// parameter of `has_CGFloat_param` exactly but use a
931+
// narrowing conversion for both literals.
932+
if (candidateResultTy->lookThroughAllOptionalTypes()
933+
->isCGFloat())
934+
return false;
935+
919936
return scoreCandidateMatch(genericSig,
920937
overloadType->getResult(),
921938
candidateResultTy,
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) %s -typecheck -solver-expression-time-threshold=1
2+
3+
// REQUIRES: asserts,no_asan
4+
// REQUIRES: objc_interop
5+
6+
// FIXME: This should be a scale-test but it doesn't allow passing `%clang-importer-sdk`
7+
8+
// This import is important because it brings CGFloat and
9+
// enables Double<->CGFloat implicit conversion that affects
10+
// literals below.
11+
import Foundation
12+
13+
let p/*: [(String, Bool, Bool, Double)]*/ = [
14+
("", true, true, 0 * 0.0 * 0.0),
15+
("", true, true, 0 * 0.0 * 0.0),
16+
("", true, true, 0 * 0.0 * 0.0),
17+
("", true, true, 0 * 0.0 * 0.0),
18+
("", true, true, 0 * 0.0 * 0.0),
19+
("", true, true, 0 * 0.0 * 0.0),
20+
("", true, true, 0 * 0.0 * 0.0),
21+
("", true, true, 0 * 0.0 * 0.0),
22+
("", true, true, 0 * 0.0 * 0.0),
23+
("", true, true, 0 * 0.0 * 0.0),
24+
("", true, true, 0 * 0.0 * 0.0),
25+
("", true, true, 0 * 0.0 * 0.0),
26+
("", true, true, 0 * 0.0 * 0.0),
27+
("", true, true, 0 * 0.0 * 0.0),
28+
("", true, true, 0 * 0.0 * 0.0),
29+
("", true, true, 0 * 0.0 * 0.0),
30+
("", true, true, 0 * 0.0 * 0.0)
31+
]

0 commit comments

Comments
 (0)