Skip to content

Commit 63c094c

Browse files
committed
[CS] Don't apply compatibility logic to collection literals
We currently permit (but warn on) coercions of collections with unrelated element types in certain cases. This is done in an effort to preserve compatibility with pre-5.3 compilers that may have allowed such code to compile due to dropping constraints while solving. This is limited to collection coercions as we emit somewhat reasonable code for them that performs a force cast of the elements at runtime. However, it turns out that in the case of collection literals, we peephole the element conversions in CSApply, leading to codegen crashes. As such, narrow down the condition of this compatibility logic to not apply to collection literals, as this never would have compiled properly. rdar://88334481
1 parent aa6942e commit 63c094c

File tree

3 files changed

+56
-8
lines changed

3 files changed

+56
-8
lines changed

lib/Sema/CSSimplify.cpp

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9557,14 +9557,33 @@ ConstraintSystem::simplifyBridgingConstraint(Type type1,
95579557
// two collections, but add a warning fix telling the user to use as! or as?
95589558
// instead.
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.
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.
95639565
//
95649566
// FIXME: Once we get a new language version, change this condition to only
95659567
// preserve compatibility for Swift 5.x mode.
9566-
auto canUseCompatFix =
9567-
rawType1->lookThroughAllOptionalTypes()->isTypeVariableOrMember();
9568+
auto canUseCompatFix = [&]() {
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/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)