Skip to content

Commit f1c35d5

Browse files
authored
Merge pull request #59896 from xedin/rdar-96469597-5.7
[5.7] Revert "[TypeChecker] Adjust Double<->CGFloat conversion to always pr…
2 parents d0961a4 + 1261f27 commit f1c35d5

File tree

5 files changed

+30
-73
lines changed

5 files changed

+30
-73
lines changed

include/swift/Sema/ConstraintSystem.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3691,12 +3691,6 @@ class ConstraintSystem {
36913691
ConstraintLocator *
36923692
getConstraintLocator(const ConstraintLocatorBuilder &builder);
36933693

3694-
/// Compute a constraint locator for an implicit value-to-value
3695-
/// conversion rooted at the given location.
3696-
ConstraintLocator *
3697-
getImplicitValueConversionLocator(ConstraintLocatorBuilder root,
3698-
ConversionRestrictionKind restriction);
3699-
37003694
/// Lookup and return parent associated with given expression.
37013695
Expr *getParentExpr(Expr *expr) {
37023696
if (auto result = getExprDepthAndParent(expr))

lib/Sema/CSApply.cpp

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6855,13 +6855,10 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
68556855
callLocator, {ConstraintLocator::ApplyFunction,
68566856
ConstraintLocator::ConstructorMember});
68576857

6858-
ConstraintLocator *baseLoc =
6859-
cs.getImplicitValueConversionLocator(locator, conversionKind);
6860-
6861-
auto overload =
6862-
solution.getOverloadChoice(solution.getConstraintLocator(
6863-
baseLoc, {ConstraintLocator::ApplyFunction,
6864-
ConstraintLocator::ConstructorMember}));
6858+
auto overload = solution.getOverloadChoice(cs.getConstraintLocator(
6859+
ASTNode(), {LocatorPathElt::ImplicitConversion(conversionKind),
6860+
ConstraintLocator::ApplyFunction,
6861+
ConstraintLocator::ConstructorMember}));
68656862

68666863
solution.overloadChoices.insert({memberLoc, overload});
68676864
}
@@ -9066,19 +9063,15 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) {
90669063
// If we're supposed to convert the expression to some particular type,
90679064
// do so now.
90689065
if (shouldCoerceToContextualType()) {
9069-
resultExpr = Rewriter.coerceToType(
9070-
resultExpr, solution.simplifyType(convertType),
9071-
cs.getConstraintLocator(resultExpr,
9072-
LocatorPathElt::ContextualType(
9073-
target.getExprContextualTypePurpose())));
9066+
resultExpr =
9067+
Rewriter.coerceToType(resultExpr, solution.simplifyType(convertType),
9068+
cs.getConstraintLocator(resultExpr));
90749069
} else if (cs.getType(resultExpr)->hasLValueType() &&
90759070
!target.isDiscardedExpr()) {
90769071
// We referenced an lvalue. Load it.
90779072
resultExpr = Rewriter.coerceToType(
90789073
resultExpr, cs.getType(resultExpr)->getRValueType(),
9079-
cs.getConstraintLocator(resultExpr,
9080-
LocatorPathElt::ContextualType(
9081-
target.getExprContextualTypePurpose())));
9074+
cs.getConstraintLocator(resultExpr));
90829075
}
90839076

90849077
if (!resultExpr)

lib/Sema/CSSimplify.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12455,26 +12455,28 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
1245512455
if (worseThanBestSolution())
1245612456
return SolutionKind::Error;
1245712457

12458-
auto *conversionLoc =
12459-
getImplicitValueConversionLocator(locator, restriction);
12458+
auto *conversionLoc = getConstraintLocator(
12459+
/*anchor=*/ASTNode(), LocatorPathElt::ImplicitConversion(restriction));
1246012460

1246112461
auto *applicationLoc =
1246212462
getConstraintLocator(conversionLoc, ConstraintLocator::ApplyFunction);
1246312463

1246412464
auto *memberLoc = getConstraintLocator(
1246512465
applicationLoc, ConstraintLocator::ConstructorMember);
1246612466

12467+
// Conversion has been already attempted for this direction
12468+
// and constructor choice has been recorded.
12469+
if (findSelectedOverloadFor(memberLoc))
12470+
return SolutionKind::Solved;
12471+
1246712472
// Allocate a single argument info to cover all possible
1246812473
// Double <-> CGFloat conversion locations.
12469-
auto *argumentsLoc =
12470-
getConstraintLocator(conversionLoc, ConstraintLocator::ApplyArgument);
12471-
12472-
if (!ArgumentLists.count(argumentsLoc)) {
12474+
if (!ArgumentLists.count(memberLoc)) {
1247312475
auto *argList = ArgumentList::createImplicit(
1247412476
getASTContext(), {Argument(SourceLoc(), Identifier(), nullptr)},
1247512477
/*firstTrailingClosureIndex=*/None,
1247612478
AllocationArena::ConstraintSolver);
12477-
ArgumentLists.insert({argumentsLoc, argList});
12479+
ArgumentLists.insert({memberLoc, argList});
1247812480
}
1247912481

1248012482
auto *memberTypeLoc = getConstraintLocator(

lib/Sema/ConstraintSystem.cpp

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -424,43 +424,21 @@ ConstraintLocator *ConstraintSystem::getConstraintLocator(
424424
return getConstraintLocator(anchor, newPath);
425425
}
426426

427-
ConstraintLocator *ConstraintSystem::getImplicitValueConversionLocator(
428-
ConstraintLocatorBuilder root, ConversionRestrictionKind restriction) {
429-
SmallVector<LocatorPathElt, 4> path;
430-
auto anchor = root.getLocatorParts(path);
431-
{
432-
// Drop any value-to-optional conversions that were applied along the
433-
// way to reach this one.
434-
while (!path.empty()) {
435-
if (path.back().is<LocatorPathElt::OptionalPayload>()) {
436-
path.pop_back();
437-
continue;
438-
}
439-
break;
440-
}
441-
442-
// If the conversion is associated with a contextual type e.g.
443-
// `_: Double = CGFloat(1)` then drop `ContextualType` so that
444-
// it's easy to find when the underlying expression has been
445-
// rewritten.
446-
if (!path.empty() && path.back().is<LocatorPathElt::ContextualType>()) {
447-
anchor = ASTNode();
448-
path.clear();
449-
}
450-
}
451-
452-
return getConstraintLocator(/*base=*/getConstraintLocator(anchor, path),
453-
LocatorPathElt::ImplicitConversion(restriction));
454-
}
455-
456427
ConstraintLocator *ConstraintSystem::getCalleeLocator(
457428
ConstraintLocator *locator, bool lookThroughApply,
458429
llvm::function_ref<Type(Expr *)> getType,
459430
llvm::function_ref<Type(Type)> simplifyType,
460431
llvm::function_ref<Optional<SelectedOverload>(ConstraintLocator *)>
461432
getOverloadFor) {
462-
if (locator->findLast<LocatorPathElt::ImplicitConversion>())
463-
return locator;
433+
if (auto conversion =
434+
locator->findLast<LocatorPathElt::ImplicitConversion>()) {
435+
if (conversion->is(ConversionRestrictionKind::DoubleToCGFloat) ||
436+
conversion->is(ConversionRestrictionKind::CGFloatToDouble)) {
437+
return getConstraintLocator(
438+
ASTNode(), {*conversion, ConstraintLocator::ApplyFunction,
439+
ConstraintLocator::ConstructorMember});
440+
}
441+
}
464442

465443
auto anchor = locator->getAnchor();
466444
auto path = locator->getPath();
@@ -5391,9 +5369,6 @@ ConstraintSystem::getArgumentInfoLocator(ConstraintLocator *locator) {
53915369
if (anchor.isNull() && locator->getPath().empty())
53925370
return nullptr;
53935371

5394-
if (locator->findLast<LocatorPathElt::ImplicitConversion>())
5395-
return locator;
5396-
53975372
// Applies and unresolved member exprs can have callee locators that are
53985373
// dependent on the type of their function, which may not have been resolved
53995374
// yet. Therefore we need to handle them specially.

test/Constraints/implicit_double_cgfloat_conversion.swift

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -194,25 +194,18 @@ func test_no_ambiguity_with_unary_operators(width: CGFloat, height: CGFloat) {
194194
}
195195

196196
func test_conversions_with_optional_promotion(d: Double, cgf: CGFloat) {
197-
func test_double(_: Double??, _: Double???) {}
198-
func test_cgfloat(_: CGFloat??, _: CGFloat???) {}
197+
func test_double(_: Double??) {}
198+
func test_cgfloat(_: CGFloat??) {}
199199

200200
// CHECK: function_ref @$sSd12CoreGraphicsEySdAA7CGFloatVcfC
201201
// CHECK-NEXT: apply
202202
// CHECK-NEXT: enum $Optional<Double>, #Optional.some!enumelt
203203
// CHECK-NEXT: enum $Optional<Optional<Double>>, #Optional.some!enumelt
204-
test_double(cgf, cgf)
204+
test_double(cgf)
205205

206206
// CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcfC
207207
// CHECK-NEXT: apply
208208
// CHECK-NEXT: enum $Optional<CGFloat>, #Optional.some!enumelt
209209
// CHECK-NEXT: enum $Optional<Optional<CGFloat>>, #Optional.some!enumelt
210-
test_cgfloat(d, d)
211-
}
212-
213-
// https://github.com/apple/swift/issues/59374
214-
func test_multi_argument_conversion_with_optional(d: Double, cgf: CGFloat) {
215-
func test(_: Double, _: CGFloat?) {}
216-
217-
test(cgf, d) // Ok (CGFloat -> Double and Double? -> CGFloat?)
210+
test_cgfloat(d)
218211
}

0 commit comments

Comments
 (0)