Skip to content

Commit 858c109

Browse files
authored
Revert "SR-14635: Casts to NSCopying should not always succeed (#37683)" (#40859)
This reverts commit c1fce93. The stricter semantics for casting to a protocol-constrained class existential seems to break some existing code. So we're going to revert that in the 5.6 branch to eliminate problems. Note: The new semantics will remain in the main branch with the usual backwards-compatibility allowances so that we can continue to improve the correctness of the casting behavior.
1 parent 7cf944d commit 858c109

File tree

3 files changed

+27
-93
lines changed

3 files changed

+27
-93
lines changed

stdlib/public/runtime/DynamicCast.cpp

Lines changed: 27 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,62 +1514,41 @@ tryCastToClassExistentialViaSwiftValue(
15141514
auto destExistentialLocation
15151515
= reinterpret_cast<ClassExistentialContainer *>(destLocation);
15161516

1517-
switch (srcType->getKind()) {
1518-
case MetadataKind::Class:
1519-
case MetadataKind::ObjCClassWrapper:
1520-
case MetadataKind::ForeignClass:
1521-
// Class references always go directly into
1522-
// class existentials; it makes no sense to wrap them.
1523-
return DynamicCastResult::Failure;
1524-
1525-
case MetadataKind::Metatype: {
1526-
#if SWIFT_OBJC_INTEROP
1527-
auto metatypePtr = reinterpret_cast<const Metadata **>(srcValue);
1528-
auto metatype = *metatypePtr;
1529-
switch (metatype->getKind()) {
1530-
case MetadataKind::Class:
1531-
case MetadataKind::ObjCClassWrapper:
1532-
case MetadataKind::ForeignClass:
1533-
// Exclude class metatypes on Darwin, since those are object types and can
1534-
// be stored directly.
1517+
// Fail if the target has constraints that make it unsuitable for
1518+
// a __SwiftValue box.
1519+
// FIXME: We should not have different checks here for
1520+
// Obj-C vs non-Obj-C. The _SwiftValue boxes should conform
1521+
// to the exact same protocols on both platforms.
1522+
bool destIsConstrained = destExistentialType->NumProtocols != 0;
1523+
if (destIsConstrained) {
1524+
#if SWIFT_OBJC_INTEROP // __SwiftValue is an Obj-C class
1525+
if (!findSwiftValueConformances(
1526+
destExistentialType, destExistentialLocation->getWitnessTables())) {
1527+
return DynamicCastResult::Failure;
1528+
}
1529+
#else // __SwiftValue is a native class
1530+
if (!swift_swiftValueConformsTo(destType, destType)) {
15351531
return DynamicCastResult::Failure;
1536-
default:
1537-
break;
15381532
}
15391533
#endif
1540-
// Non-class metatypes are never objects, and
1541-
// metatypes on non-Darwin are never objects, so
1542-
// fall through to box those.
1543-
SWIFT_FALLTHROUGH;
15441534
}
15451535

1546-
default: {
1547-
if (destExistentialType->NumProtocols != 0) {
1548-
// The destination is a class-constrained protocol type
1549-
// and the source is not a class, so....
1550-
return DynamicCastResult::Failure;
1551-
} else {
1552-
// This is a simple (unconstrained) `AnyObject` so we can populate
1553-
// it by stuffing a non-class instance into a __SwiftValue box
15541536
#if SWIFT_OBJC_INTEROP
1555-
auto object = bridgeAnythingToSwiftValueObject(
1556-
srcValue, srcType, takeOnSuccess);
1557-
destExistentialLocation->Value = object;
1558-
if (takeOnSuccess) {
1559-
return DynamicCastResult::SuccessViaTake;
1560-
} else {
1561-
return DynamicCastResult::SuccessViaCopy;
1562-
}
1537+
auto object = bridgeAnythingToSwiftValueObject(
1538+
srcValue, srcType, takeOnSuccess);
1539+
destExistentialLocation->Value = object;
1540+
if (takeOnSuccess) {
1541+
return DynamicCastResult::SuccessViaTake;
1542+
} else {
1543+
return DynamicCastResult::SuccessViaCopy;
1544+
}
15631545
# else
1564-
// Note: Code below works correctly on both Obj-C and non-Obj-C platforms,
1565-
// but the code above is slightly faster on Obj-C platforms.
1566-
auto object = _bridgeAnythingToObjectiveC(srcValue, srcType);
1567-
destExistentialLocation->Value = object;
1568-
return DynamicCastResult::SuccessViaCopy;
1546+
// Note: Code below works correctly on both Obj-C and non-Obj-C platforms,
1547+
// but the code above is slightly faster on Obj-C platforms.
1548+
auto object = _bridgeAnythingToObjectiveC(srcValue, srcType);
1549+
destExistentialLocation->Value = object;
1550+
return DynamicCastResult::SuccessViaCopy;
15691551
#endif
1570-
}
1571-
}
1572-
}
15731552
}
15741553

15751554
static DynamicCastResult

test/Casting/Casts.swift

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -954,35 +954,6 @@ CastsTests.test("Recursive AnyHashable") {
954954
expectEqual(s.x, p)
955955
}
956956

957-
// SR-14635 (aka rdar://78224322)
958-
#if _runtime(_ObjC)
959-
CastsTests.test("Do not overuse __SwiftValue") {
960-
struct Bar {}
961-
// This used to succeed because of overeager __SwiftValue
962-
// boxing (and __SwiftValue does satisfy NSCopying)
963-
expectFalse(Bar() is NSCopying)
964-
expectFalse(Bar() as Any is NSCopying)
965-
966-
// This seems unavoidable?
967-
// `Bar() as! AnyObject` gets boxed as a __SwiftValue,
968-
// and __SwiftValue does conform to NSCopying
969-
expectTrue(Bar() as! AnyObject is NSCopying)
970-
971-
class Foo {}
972-
// Foo does not conform to NSCopying
973-
// (This used to succeed due to over-eager __SwiftValue boxing)
974-
expectFalse(Foo() is NSCopying)
975-
expectFalse(Foo() as Any is NSCopying)
976-
977-
// A type that really does conform should cast to NSCopying
978-
class Foo2: NSCopying {
979-
func copy(with: NSZone?) -> Any { return self }
980-
}
981-
expectTrue(Foo2() is NSCopying)
982-
expectTrue(Foo2() is AnyObject)
983-
}
984-
#endif
985-
986957
#if _runtime(_ObjC)
987958
CastsTests.test("Artificial subclass protocol conformance") {
988959
class SwiftClass: NSObject {}
@@ -993,15 +964,4 @@ CastsTests.test("Artificial subclass protocol conformance") {
993964
}
994965
#endif
995966

996-
CastsTests.test("Do not overuse __SwiftValue (non-ObjC)") {
997-
struct Bar {}
998-
// This should succeed because this is what __SwiftValue boxing is for
999-
expectTrue(Bar() is AnyObject)
1000-
expectTrue(Bar() as Any is AnyObject)
1001-
1002-
class Foo {}
1003-
// Any class type can be cast to AnyObject
1004-
expectTrue(Foo() is AnyObject)
1005-
}
1006-
1007967
runAllTests()

validation-test/Casting/Inputs/BoxingCasts.swift.gyb

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,6 @@ contents = [
173173
extra_targets=["StructInt.Type"],
174174
roundtrips=False, # Compiler bug rejects roundtrip cast T? => Any => T?
175175
),
176-
Contents(name="ClassInt.Type",
177-
constructor="ClassInt.self",
178-
hashable=False,
179-
protocols=[],
180-
),
181176
Contents(name="PublicProtocol.Protocol",
182177
constructor="PublicProtocol.self",
183178
hashable=False,

0 commit comments

Comments
 (0)