Skip to content

Commit 6fe4ffc

Browse files
committed
[ConstraintSystem] Adjust ranking rules associated with Double/CGFloat conversions
Change the conversion rule to favor any number of widenings (CGFloat -> Double) over even a single narrowing conversion and if there is no way to avoid narrowing (due to contextual requirements) attempt it as late as possible (the deeper in the AST that conversion is located the higher its score). This is a generally better rule when it comes to rounding and information loss that would result from narrowing conversions.
1 parent 0567900 commit 6fe4ffc

File tree

2 files changed

+57
-11
lines changed

2 files changed

+57
-11
lines changed

lib/Sema/CSSimplify.cpp

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10552,20 +10552,17 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
1055210552
case ConversionRestrictionKind::DoubleToCGFloat:
1055310553
case ConversionRestrictionKind::CGFloatToDouble: {
1055410554
// Prefer CGFloat -> Double over other way araund.
10555-
auto defaultImpact =
10556-
restriction == ConversionRestrictionKind::CGFloatToDouble ? 1 : 2;
10555+
auto impact =
10556+
restriction == ConversionRestrictionKind::CGFloatToDouble ? 1 : 10;
1055710557

10558-
// Consider conversion with context to be worse
10559-
// by default because it means that expression
10560-
// as a whole couldn't get to the expected type.
10561-
if (auto last = locator.last()) {
10562-
if (last->is<LocatorPathElt::ContextualType>())
10563-
++defaultImpact;
10558+
if (restriction == ConversionRestrictionKind::DoubleToCGFloat) {
10559+
if (auto *anchor = locator.trySimplifyToExpr()) {
10560+
if (auto depth = getExprDepth(anchor))
10561+
impact = (*depth + 1) * impact;
10562+
}
1056410563
}
1056510564

10566-
auto numConversions = ImplicitValueConversions.size();
10567-
increaseScore(SK_ImplicitValueConversion,
10568-
defaultImpact * (numConversions + 1));
10565+
increaseScore(SK_ImplicitValueConversion, impact);
1056910566

1057010567
if (worseThanBestSolution())
1057110568
return SolutionKind::Error;

test/Constraints/implicit_double_cgfloat_conversion.swift

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,52 @@ func test_static_members_are_contextually_convertible() {
124124
return S.testFunc() // Ok
125125
}
126126
}
127+
128+
func test_narrowing_is_delayed(x: Double, y: CGFloat) {
129+
func test(_: CGFloat) {}
130+
131+
func overloaded(_: Double, _: Double) -> Double {}
132+
func overloaded(_: CGFloat, _: CGFloat) -> CGFloat {}
133+
134+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
135+
// CHECK: function_ref @$sSd1doiyS2d_SdtFZ
136+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcfC
137+
let _: CGFloat = x / y // CGFloat.init(x / Double.init(y))
138+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
139+
// CHECK: function_ref @$sSd1doiyS2d_SdtFZ
140+
// CHECK: function_ref @$sSd22_builtinIntegerLiteralSdBI_tcfC
141+
// CHECK: function_ref @$sSd1poiyS2d_SdtFZ
142+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcfC
143+
let _: CGFloat = x / y + 1 // CGFloat.init(x / Double(y) + 1 as Double)
144+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
145+
// CHECK: function_ref @$s34implicit_double_cgfloat_conversion25test_narrowing_is_delayed1x1yySd_12CoreGraphics7CGFloatVtF10overloadedL_yS2d_SdtF
146+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcf
147+
let _: CGFloat = overloaded(x, y) // Prefers `overloaded(Double, Double) -> Double`
148+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
149+
// CHECK: function_ref @$s34implicit_double_cgfloat_conversion25test_narrowing_is_delayed1x1yySd_12CoreGraphics7CGFloatVtF10overloadedL_yS2d_SdtF
150+
// CHECK: @$s34implicit_double_cgfloat_conversion25test_narrowing_is_delayed1x1yySd_12CoreGraphics7CGFloatVtF10overloadedL_yS2d_SdtF
151+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcf
152+
let _: CGFloat = overloaded(x, overloaded(x, y)) // Prefers `overloaded(Double, Double) -> Double` in both occurances.
153+
154+
// Calls should behave exactly the same as contextual conversions.
155+
156+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
157+
// CHECK: function_ref @$sSd1doiyS2d_SdtFZ
158+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcfC
159+
test(x / y)
160+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
161+
// CHECK: function_ref @$sSd1doiyS2d_SdtFZ
162+
// CHECK: function_ref @$sSd22_builtinIntegerLiteralSdBI_tcfC
163+
// CHECK: function_ref @$sSd1poiyS2d_SdtFZ
164+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcfC
165+
test(x / y + 1)
166+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
167+
// CHECK: function_ref @$s34implicit_double_cgfloat_conversion25test_narrowing_is_delayed1x1yySd_12CoreGraphics7CGFloatVtF10overloadedL_yS2d_SdtF
168+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcf
169+
test(overloaded(x, y))
170+
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
171+
// CHECK: function_ref @$s34implicit_double_cgfloat_conversion25test_narrowing_is_delayed1x1yySd_12CoreGraphics7CGFloatVtF10overloadedL_yS2d_SdtF
172+
// CHECK: @$s34implicit_double_cgfloat_conversion25test_narrowing_is_delayed1x1yySd_12CoreGraphics7CGFloatVtF10overloadedL_yS2d_SdtF
173+
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcf
174+
test(overloaded(x, overloaded(x, y)))
175+
}

0 commit comments

Comments
 (0)