Skip to content

Commit 6ea8763

Browse files
hamishknighthamishknight
authored
Merge pull request #41315 from hamishknight/shortsighted
2 parents f75ed39 + 04b37ae commit 6ea8763

File tree

5 files changed

+149
-27
lines changed

5 files changed

+149
-27
lines changed

‎lib/Sema/CSApply.cpp

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4045,20 +4045,19 @@ namespace {
40454045
}
40464046

40474047
Expr *visitCoerceExpr(CoerceExpr *expr) {
4048+
auto *coerced = visitCoerceExprImpl(expr);
4049+
if (!coerced)
4050+
return nullptr;
4051+
40484052
// If we need to insert a force-unwrap for coercions of the form
40494053
// 'as T!', do so now.
4050-
if (hasForcedOptionalResult(expr)) {
4051-
auto *coerced = visitCoerceExpr(expr, None);
4052-
if (!coerced)
4053-
return nullptr;
4054-
4055-
return forceUnwrapIUO(coerced);
4056-
}
4054+
if (hasForcedOptionalResult(expr))
4055+
coerced = forceUnwrapIUO(coerced);
40574056

4058-
return visitCoerceExpr(expr, None);
4057+
return coerced;
40594058
}
40604059

4061-
Expr *visitCoerceExpr(CoerceExpr *expr, Optional<unsigned> choice) {
4060+
Expr *visitCoerceExprImpl(CoerceExpr *expr) {
40624061
// Simplify and update the type we're coercing to.
40634062
assert(expr->getCastTypeRepr());
40644063
const auto toType = simplifyType(cs.getType(expr->getCastTypeRepr()));
@@ -4107,14 +4106,11 @@ namespace {
41074106
// get it from the solution to determine whether we've picked a coercion
41084107
// or a bridging conversion.
41094108
auto *locator = cs.getConstraintLocator(expr);
4110-
4111-
if (!choice) {
4112-
choice = solution.getDisjunctionChoice(locator);
4113-
}
4109+
auto choice = solution.getDisjunctionChoice(locator);
41144110

41154111
// Handle the coercion/bridging of the underlying subexpression, where
41164112
// optionality has been removed.
4117-
if (*choice == 0) {
4113+
if (choice == 0) {
41184114
// Convert the subexpression.
41194115
Expr *sub = expr->getSubExpr();
41204116

@@ -4128,7 +4124,7 @@ namespace {
41284124
}
41294125

41304126
// Bridging conversion.
4131-
assert(*choice == 1 && "should be bridging");
4127+
assert(choice == 1 && "should be bridging");
41324128

41334129
// Handle optional bindings.
41344130
Expr *sub = handleOptionalBindings(expr->getSubExpr(), toType,

‎lib/Sema/CSSimplify.cpp

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9555,16 +9555,35 @@ ConstraintSystem::simplifyBridgingConstraint(Type type1,
95559555
// we generated somewhat reasonable code that performed a force cast. To
95569556
// maintain compatibility with that behavior, allow the coercion between
95579557
// two collections, but add a warning fix telling the user to use as! or as?
9558-
// instead.
9558+
// instead. In Swift 6 mode, this becomes an error.
95599559
//
9560-
// We only need to perform this compatibility logic if the LHS type is a
9561-
// (potentially optional) type variable, as only such a constraint could have
9562-
// been previously been left unsolved.
9563-
//
9564-
// FIXME: Once we get a new language version, change this condition to only
9565-
// preserve compatibility for Swift 5.x mode.
9566-
auto canUseCompatFix =
9567-
rawType1->lookThroughAllOptionalTypes()->isTypeVariableOrMember();
9560+
// We only need to perform this compatibility logic if this is a coercion of
9561+
// something that isn't a collection expr (as collection exprs would have
9562+
// crashed in codegen due to CSApply peepholing them). Additionally, the LHS
9563+
// type must be a (potentially optional) type variable, as only such a
9564+
// constraint could have been previously been left unsolved.
9565+
auto canUseCompatFix = [&]() {
9566+
if (Context.isSwiftVersionAtLeast(6))
9567+
return false;
9568+
9569+
if (!rawType1->lookThroughAllOptionalTypes()->isTypeVariableOrMember())
9570+
return false;
9571+
9572+
SmallVector<LocatorPathElt, 4> elts;
9573+
auto anchor = locator.getLocatorParts(elts);
9574+
if (!elts.empty())
9575+
return false;
9576+
9577+
auto *coercion = getAsExpr<CoerceExpr>(anchor);
9578+
if (!coercion)
9579+
return false;
9580+
9581+
auto *subject = coercion->getSubExpr();
9582+
while (auto *paren = dyn_cast<ParenExpr>(subject))
9583+
subject = paren->getSubExpr();
9584+
9585+
return !isa<CollectionExpr>(subject);
9586+
}();
95689587

95699588
// Unless we're allowing the collection compatibility fix, the source cannot
95709589
// be more optional than the destination.

‎test/Constraints/casts.swift

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func test_coercions_with_overloaded_operator(str: String, optStr: String?, veryO
280280

281281
func id<T>(_ x: T) -> T { x }
282282

283-
func test_compatibility_coercions(_ arr: [Int], _ optArr: [Int]?, _ dict: [String: Int], _ set: Set<Int>) {
283+
func test_compatibility_coercions(_ arr: [Int], _ optArr: [Int]?, _ dict: [String: Int], _ set: Set<Int>, _ i: Int, _ stringAnyDict: [String: Any]) {
284284
// Successful coercions don't raise a warning.
285285
_ = arr as [Any]?
286286
_ = dict as [String: Int]?
@@ -337,6 +337,24 @@ func test_compatibility_coercions(_ arr: [Int], _ optArr: [Int]?, _ dict: [Strin
337337

338338
// The array can also be inferred to be [Any].
339339
_ = ([] ?? []) as Array // expected-warning {{left side of nil coalescing operator '??' has non-optional type '[Any]', so the right side is never used}}
340+
341+
// rdar://88334481 – Don't apply the compatibility logic for collection literals.
342+
typealias Magic<T> = T
343+
_ = [i] as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
344+
_ = [i] as Magic as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
345+
_ = ([i]) as Magic as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
346+
_ = [i: i] as [String: Any] // expected-error {{cannot convert value of type 'Int' to expected dictionary key type 'String'}}
347+
_ = ([i: i]) as [String: Any] // expected-error {{cannot convert value of type 'Int' to expected dictionary key type 'String'}}
348+
_ = [i: stringAnyDict] as [String: Any] // expected-error {{cannot convert value of type 'Int' to expected dictionary key type 'String'}}
349+
350+
// These are currently not peepholed.
351+
_ = [i].self as Magic as [String] // expected-warning {{coercion from '[Int]' to '[String]' may fail; use 'as?' or 'as!' instead}}
352+
_ = (try [i]) as Magic as [String] // expected-warning {{coercion from '[Int]' to '[String]' may fail; use 'as?' or 'as!' instead}}
353+
// expected-warning@-1 {{no calls to throwing functions occur within 'try' expression}}
354+
355+
// These are wrong, but make sure we don't warn about the value cast always succeeding.
356+
_ = [i: i] as! [String: Any]
357+
_ = [i: stringAnyDict] as! [String: Any]
340358
}
341359

342360
// SR-13088

‎test/Constraints/casts_swift6.swift

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// RUN: %target-typecheck-verify-swift -enable-objc-interop -swift-version 6
2+
3+
// -swift-version 6 is currently asserts-only
4+
// REQUIRES: asserts
5+
6+
func id<T>(_ x: T) -> T { x }
7+
func ohno<T>(_ x: T) -> T? { nil }
8+
9+
// Swift 6 version of the test in casts.swift
10+
func test_compatibility_coercions(_ arr: [Int], _ optArr: [Int]?, _ dict: [String: Int], _ set: Set<Int>, _ i: Int, _ stringAnyDict: [String: Any]) {
11+
// These have always been fine.
12+
_ = arr as [Any]?
13+
_ = dict as [String: Int]?
14+
_ = set as Set<Int>
15+
16+
// These have always been errors.
17+
_ = arr as [String] // expected-error {{cannot convert value of type '[Int]' to type '[String]' in coercion}}
18+
// expected-note@-1 {{arguments to generic parameter 'Element' ('Int' and 'String') are expected to be equal}}
19+
_ = dict as [String: String] // expected-error {{cannot convert value of type '[String : Int]' to type '[String : String]' in coercion}}
20+
// expected-note@-1 {{arguments to generic parameter 'Value' ('Int' and 'String') are expected to be equal}}
21+
_ = dict as [String: String]? // expected-error {{'[String : Int]' is not convertible to '[String : String]?'}}
22+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{12-14=as!}}
23+
_ = (dict as [String: Int]?) as [String: Int] // expected-error {{value of optional type '[String : Int]?' must be unwrapped to a value of type '[String : Int]'}}
24+
// expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
25+
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
26+
_ = set as Set<String> // expected-error {{cannot convert value of type 'Set<Int>' to type 'Set<String>' in coercion}}
27+
// expected-note@-1 {{arguments to generic parameter 'Element' ('Int' and 'String') are expected to be equal}}
28+
29+
// Make sure we error on the following in Swift 6 mode.
30+
31+
// FIXME: Bad diagnostics (SR-15843)
32+
_ = id(arr) as [String] // expected-error {{type of expression is ambiguous without more context}}
33+
_ = (arr ?? []) as [String] // expected-error {{type of expression is ambiguous without more context}}
34+
_ = (arr ?? [] ?? []) as [String] // expected-error {{type of expression is ambiguous without more context}}
35+
_ = (optArr ?? []) as [String] // expected-error {{type of expression is ambiguous without more context}}
36+
37+
_ = (arr ?? []) as [String]? // expected-error {{'[Int]' is not convertible to '[String]?'}}
38+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}}
39+
_ = (arr ?? []) as [String?]? // expected-error {{'[Int]' is not convertible to '[String?]?'}}
40+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}}
41+
_ = (arr ?? []) as [String??]?? // expected-error {{'[Int]' is not convertible to '[String??]??'}}
42+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}}
43+
_ = (dict ?? [:]) as [String: String?]? // expected-error {{'[String : Int]' is not convertible to '[String : String?]?'}}
44+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}}
45+
_ = (set ?? []) as Set<String>?? // expected-error {{'Set<Int>' is not convertible to 'Set<String>??'}}
46+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}}
47+
48+
_ = ohno(ohno(ohno(arr))) as [String] // expected-error {{cannot convert value of type '[Int]???' to type '[String]' in coercion}}
49+
_ = ohno(ohno(ohno(arr))) as [Int] // expected-error {{cannot convert value of type '[Int]???' to type '[Int]' in coercion}}
50+
_ = ohno(ohno(ohno(Set<Int>()))) as Set<String> // expected-error {{cannot convert value of type 'Set<Int>???' to type 'Set<String>' in coercion}}
51+
_ = ohno(ohno(ohno(["": ""]))) as [Int: String] // expected-error {{cannot convert value of type '[String : String]???' to type '[Int : String]' in coercion}}
52+
_ = ohno(ohno(ohno(dict))) as [String: Int] // expected-error {{cannot convert value of type '[String : Int]???' to type '[String : Int]' in coercion}}
53+
54+
// In this case the array literal can be inferred to be [String], so totally
55+
// valid.
56+
_ = ([] ?? []) as [String] // expected-warning {{left side of nil coalescing operator '??' has non-optional type '[String]', so the right side is never used}}
57+
_ = (([] as Optional) ?? []) as [String]
58+
59+
// The array can also be inferred to be [Any].
60+
_ = ([] ?? []) as Array // expected-warning {{left side of nil coalescing operator '??' has non-optional type '[Any]', so the right side is never used}}
61+
62+
// Cases from rdar://88334481
63+
typealias Magic<T> = T
64+
_ = [i] as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
65+
_ = [i] as Magic as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
66+
_ = ([i]) as Magic as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
67+
_ = [i: i] as [String: Any] // expected-error {{cannot convert value of type 'Int' to expected dictionary key type 'String'}}
68+
_ = ([i: i]) as [String: Any] // expected-error {{cannot convert value of type 'Int' to expected dictionary key type 'String'}}
69+
_ = [i: stringAnyDict] as [String: Any] // expected-error {{cannot convert value of type 'Int' to expected dictionary key type 'String'}}
70+
71+
_ = [i].self as Magic as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
72+
_ = (try [i]) as Magic as [String] // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
73+
// expected-warning@-1 {{no calls to throwing functions occur within 'try' expression}}
74+
75+
// These are wrong, but make sure we don't warn about the value cast always succeeding.
76+
_ = [i: i] as! [String: Any]
77+
_ = [i: stringAnyDict] as! [String: Any]
78+
}

‎test/SILGen/collection_downcast.swift

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,8 @@ func testSetDowncastBridgedConditional(_ dict: Set<NSObject>)
242242

243243
func promote<T>(_ x: T) -> T? { nil }
244244

245-
// CHECK-LABEL: sil hidden [ossa] @$s19collection_downcast36testCollectionCompatibilityCoercionsyySaySiG_SayypGSgShySSGSDySSSiGtF
246-
func testCollectionCompatibilityCoercions(_ arr: [Int], _ optArr: [Any]?, _ set: Set<String>, _ dict: [String: Int]) {
245+
// CHECK-LABEL: sil hidden [ossa] @$s19collection_downcast36testCollectionCompatibilityCoercionsyySaySiG_SayypGSgShySSGSDySSSiGSitF
246+
func testCollectionCompatibilityCoercions(_ arr: [Int], _ optArr: [Any]?, _ set: Set<String>, _ dict: [String: Int], _ i: Int) {
247247
// Make sure we generate reasonable code for all of the below examples.
248248

249249
// CHECK: [[CAST_FN:%.+]] = function_ref @$ss15_arrayForceCastySayq_GSayxGr0_lF : $@convention(thin) <τ_0_0, τ_0_1> (@guaranteed Array<τ_0_0>) -> @owned Array<τ_0_1>
@@ -297,4 +297,15 @@ func testCollectionCompatibilityCoercions(_ arr: [Int], _ optArr: [Any]?, _ set:
297297
// CHECK: [[CAST_FN:%.+]] = function_ref @$ss17_dictionaryUpCastySDyq0_q1_GSDyxq_GSHRzSHR0_r2_lF : $@convention(thin) <τ_0_0, τ_0_1, τ_0_2, τ_0_3 where τ_0_0 : Hashable, τ_0_2 : Hashable> (@guaranteed Dictionary<τ_0_0, τ_0_1>) -> @owned Dictionary<τ_0_2, τ_0_3>
298298
// CHECK: apply [[CAST_FN]]<String, Int, Int, String>([[DICT]]) : $@convention(thin) <τ_0_0, τ_0_1, τ_0_2, τ_0_3 where τ_0_0 : Hashable, τ_0_2 : Hashable> (@guaranteed Dictionary<τ_0_0, τ_0_1>) -> @owned Dictionary<τ_0_2, τ_0_3>
299299
_ = promote(promote(promote(dict))) as [Int: String]
300+
301+
typealias Magic<T> = T
302+
303+
// These are currently not peepholed.
304+
// CHECK: [[CAST_FN:%.+]] = function_ref @$ss15_arrayForceCastySayq_GSayxGr0_lF : $@convention(thin) <τ_0_0, τ_0_1> (@guaranteed Array<τ_0_0>) -> @owned Array<τ_0_1>
305+
// CHECK: apply [[CAST_FN]]<Int, String>
306+
[i].self as Magic as [String]
307+
308+
// CHECK: [[CAST_FN:%.+]] = function_ref @$ss15_arrayForceCastySayq_GSayxGr0_lF : $@convention(thin) <τ_0_0, τ_0_1> (@guaranteed Array<τ_0_0>) -> @owned Array<τ_0_1>
309+
// CHECK: apply [[CAST_FN]]<Int, String>
310+
(try [i]) as Magic as [String]
300311
}

0 commit comments

Comments
 (0)