Skip to content

Commit b91f29d

Browse files
committed
SR-14635: __SwiftValue should be transparent to casting
This PR changes the casting machinery to avoid casting `__SwiftValue` boxes directly. This forces the caster to instead unwrap `__SwiftValue` boxes and retry with the inner content. This results in boxed values being cast like the inner content. This fixes the behavior in situations like the following: ``` let t = ... let s = t as Any as! AnyObject // `s` is now a `__SwiftValue` box // Next line should be true iff t conforms to NSCopying // Prior to this change, it always succeeds s is NSCopying ``` After this change, the above cast succeeds only if `t` actually conforms to `NSCopying`. This is a follow-on to PR#37683. Related to: SR-14635
1 parent bbda52f commit b91f29d

File tree

3 files changed

+66
-6
lines changed

3 files changed

+66
-6
lines changed

stdlib/public/runtime/DynamicCast.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,29 @@ tryCastUnwrappingObjCSwiftValueSource(
425425
destFailureType, srcFailureType,
426426
/*takeOnSuccess=*/ false, mayDeferChecks);
427427
}
428+
#else
429+
static DynamicCastResult
430+
tryCastUnwrappingSwiftValueSource(
431+
OpaqueValue *destLocation, const Metadata *destType,
432+
OpaqueValue *srcValue, const Metadata *srcType,
433+
const Metadata *&destFailureType, const Metadata *&srcFailureType,
434+
bool takeOnSuccess, bool mayDeferChecks)
435+
{
436+
const Metadata *srcInnerType;
437+
const OpaqueValue *srcInnerValue;
438+
if (swift_unboxFromSwiftValueWithType(srcValue, &srcInnerValue, &srcInnerType)) {
439+
return DynamicCastResult::SuccessViaCopy;
440+
}
441+
442+
// Note: We never `take` the contents from a SwiftValue box as
443+
// it might have other references. Instead, let our caller
444+
// destroy the reference if necessary.
445+
return tryCast(
446+
destLocation, destType,
447+
const_cast<OpaqueValue *>(srcInnerValue), srcInnerType,
448+
destFailureType, srcFailureType,
449+
/*takeOnSuccess=*/ false, mayDeferChecks);
450+
}
428451
#endif
429452

430453
/******************************************************************************/
@@ -1474,6 +1497,16 @@ tryCastToClassExistential(
14741497
}
14751498

14761499
case MetadataKind::ObjCClassWrapper:
1500+
#if SWIFT_OBJC_INTEROP
1501+
id srcObject;
1502+
memcpy(&srcObject, srcValue, sizeof(id));
1503+
if (getAsSwiftValue(srcObject) != nullptr) {
1504+
// Do not directly cast a `__SwiftValue` box
1505+
// Return failure so our caller will unwrap and try again
1506+
return DynamicCastResult::Failure;
1507+
}
1508+
#endif
1509+
SWIFT_FALLTHROUGH;
14771510
case MetadataKind::Class:
14781511
case MetadataKind::ForeignClass: {
14791512
auto srcObject = getNonNullSrcObject(srcValue, srcType, destType);
@@ -2285,8 +2318,11 @@ tryCast(
22852318
case MetadataKind::Class: {
22862319
#if !SWIFT_OBJC_INTEROP
22872320
// Try unwrapping native __SwiftValue implementation
2288-
if (swift_unboxFromSwiftValueWithType(srcValue, destLocation, destType)) {
2289-
return DynamicCastResult::SuccessViaCopy;
2321+
auto subcastResult = tryCastUnwrappingSwiftValueSource(
2322+
destLocation, destType, srcValue, srcType,
2323+
destFailureType, srcFailureType, takeOnSuccess, mayDeferChecks);
2324+
if (isSuccess(subcastResult)) {
2325+
return subcastResult;
22902326
}
22912327
#endif
22922328
break;

stdlib/public/runtime/SwiftObject.mm

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "ErrorObject.h"
4242
#include "Private.h"
4343
#include "SwiftObject.h"
44+
#include "SwiftValue.h"
4445
#include "WeakReference.h"
4546
#if SWIFT_OBJC_INTEROP
4647
#include <dlfcn.h>
@@ -1269,6 +1270,10 @@ id swift_dynamicCastObjCProtocolUnconditional(id object,
12691270
id swift_dynamicCastObjCProtocolConditional(id object,
12701271
size_t numProtocols,
12711272
Protocol * const *protocols) {
1273+
if (getAsSwiftValue(object) != nil) {
1274+
// SwiftValue wrapper never holds a class object
1275+
return nil;
1276+
}
12721277
for (size_t i = 0; i < numProtocols; ++i) {
12731278
if (![object conformsToProtocol:protocols[i]]) {
12741279
return nil;

test/Casting/Casts.swift

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -993,25 +993,33 @@ CastsTests.test("Do not overuse __SwiftValue")
993993
// This used to succeed because of overeager __SwiftValue
994994
// boxing (and __SwiftValue does satisfy NSCopying)
995995
expectFalse(Bar() is NSCopying)
996+
expectNil(runtimeCast(Bar(), to: NSCopying.self))
996997
expectFalse(Bar() as Any is NSCopying)
998+
expectNil(runtimeCast(Bar() as Any, to: NSCopying.self))
997999

998-
// This seems unavoidable?
999-
// `Bar() as! AnyObject` gets boxed as a __SwiftValue,
1000-
// and __SwiftValue does conform to NSCopying
1001-
expectTrue(Bar() as! AnyObject is NSCopying)
1000+
// `Bar() as! AnyObject` gets boxed as a __SwiftValue.
1001+
// __SwiftValue does conform to NSCopying, but that should
1002+
// not be visible here.
1003+
let anyBar = Bar() as! AnyObject
1004+
expectNil(runtimeCast(anyBar, to: NSCopying.self))
1005+
expectFalse(anyBar is NSCopying)
10021006

10031007
class Foo {}
10041008
// Foo does not conform to NSCopying
10051009
// (This used to succeed due to over-eager __SwiftValue boxing)
10061010
expectFalse(Foo() is NSCopying)
1011+
expectNil(runtimeCast(Foo(), to: NSCopying.self))
10071012
expectFalse(Foo() as Any is NSCopying)
1013+
expectNil(runtimeCast(Foo() as Any, to: NSCopying.self))
10081014

10091015
// A type that really does conform should cast to NSCopying
10101016
class Foo2: NSCopying {
10111017
func copy(with: NSZone?) -> Any { return self }
10121018
}
10131019
expectTrue(Foo2() is NSCopying)
1020+
expectNotNil(runtimeCast(Foo2(), to: NSCopying.self))
10141021
expectTrue(Foo2() is AnyObject)
1022+
expectNotNil(runtimeCast(Foo2(), to: AnyObject.self))
10151023
}
10161024
#endif
10171025

@@ -1030,10 +1038,21 @@ CastsTests.test("Do not overuse __SwiftValue (non-ObjC)") {
10301038
// This should succeed because this is what __SwiftValue boxing is for
10311039
expectTrue(Bar() is AnyObject)
10321040
expectTrue(Bar() as Any is AnyObject)
1041+
let a = Bar() as Any as! AnyObject
1042+
expectTrue(a is Bar)
10331043

10341044
class Foo {}
10351045
// Any class type can be cast to AnyObject
10361046
expectTrue(Foo() is AnyObject)
1047+
let b = Foo() as! AnyObject
1048+
expectTrue(b is Foo)
1049+
1050+
// As above, but force use of runtime casting
1051+
expectNotNil(runtimeCast(Bar(), to: AnyObject.self))
1052+
expectNotNil(runtimeCast(Bar() as Any, to: AnyObject.self))
1053+
expectNotNil(runtimeCast(a, to: Bar.self))
1054+
expectNotNil(runtimeCast(Foo(), to: AnyObject.self))
1055+
expectNotNil(runtimeCast(b, to: Foo.self))
10371056
}
10381057

10391058
runAllTests()

0 commit comments

Comments
 (0)