SR-14635: Casts to NSCopying should not always succeed (#37683)

* SR-14635: Casts to NSCopying should not always succeed

The runtime dynamic casting logic explores a variety of strategies for
each cast request.  One of the last options is to wrap the source
in a `__SwiftValue` box so it can bridge to Obj-C.  The previous
code was overly aggressive about such boxing; it performed the boxing
for any source type and only checked to verify that the `__SwiftValue`
box itself was compatible with the destination.

Among other oddities, this results in the behavior discussed
in SR-14635, where any Swift or Obj-C type will always successfully cast
to NSCopying because `__SwiftValue` is compatible with NSCopying.

This is actually two subtly different issues:

* Class types should not be subject to `__SwiftValue` boxing at all.
  Casting class types to class existentials is already handled elsewhere,
  so this function should just reject any source with class type.

* Non-class types should be boxed only when being assigned to
  an AnyObject (an "unconstrained class existential").  If
  the class existential has constraints, it is by definition
  a class-constrained existential which should not receive
  any non-class object.

To solve these, this PR disables `__SwiftValue` boxing in two cases:
1. If the source is a class (reference) type.
2. If the destination has constraints

Resolves SR-14635
Resolves rdar://78224322

* Avoid boxing class metatypes on Darwin

But continue boxing
 * Non-class metatypes on all platforms
 * All metatypes on non-Darwin platforms

Obj-C interop requires that we do not box class metatypes;
those must be usable as simple pointers when passed to Obj-C.
But no other metatype is object-compatible, so we have to
continue boxing everything else.

* Split out ObjC-specific test cases

Author: tbkka

stdlib/public/runtime/DynamicCast.cpp

@@ -1514,41 +1514,62 @@ tryCastToClassExistentialViaSwiftValue(
   auto destExistentialLocation
     = reinterpret_cast<ClassExistentialContainer *>(destLocation);
 
-  // Fail if the target has constraints that make it unsuitable for
-  // a __SwiftValue box.
-  // FIXME: We should not have different checks here for
-  // Obj-C vs non-Obj-C.  The _SwiftValue boxes should conform
-  // to the exact same protocols on both platforms.
-  bool destIsConstrained = destExistentialType->NumProtocols != 0;
-  if (destIsConstrained) {
-#if SWIFT_OBJC_INTEROP // __SwiftValue is an Obj-C class
-    if (!findSwiftValueConformances(
-          destExistentialType, destExistentialLocation->getWitnessTables())) {
-      return DynamicCastResult::Failure;
-    }
-#else // __SwiftValue is a native class
-    if (!swift_swiftValueConformsTo(destType, destType)) {
+  switch (srcType->getKind()) {
+  case MetadataKind::Class:
+  case MetadataKind::ObjCClassWrapper:
+  case MetadataKind::ForeignClass:
+    // Class references always go directly into
+    // class existentials; it makes no sense to wrap them.
+    return DynamicCastResult::Failure;
+
+  case MetadataKind::Metatype: {
+#if SWIFT_OBJC_INTEROP
+    auto metatypePtr = reinterpret_cast<const Metadata **>(srcValue);
+    auto metatype = *metatypePtr;
+    switch (metatype->getKind()) {
+    case MetadataKind::Class:
+    case MetadataKind::ObjCClassWrapper:
+    case MetadataKind::ForeignClass:
+      // Exclude class metatypes on Darwin, since those are object types and can
+      // be stored directly.
       return DynamicCastResult::Failure;
+    default:
+      break;
     }
 #endif
+    // Non-class metatypes are never objects, and
+    // metatypes on non-Darwin are never objects, so
+    // fall through to box those.
+    SWIFT_FALLTHROUGH;
   }
 
+  default: {
+    if (destExistentialType->NumProtocols != 0) {
+      // The destination is a class-constrained protocol type
+      // and the source is not a class, so....
+      return DynamicCastResult::Failure;
+    } else {
+      // This is a simple (unconstrained) `AnyObject` so we can populate
+      // it by stuffing a non-class instance into a __SwiftValue box
 #if SWIFT_OBJC_INTEROP
-  auto object = bridgeAnythingToSwiftValueObject(
-    srcValue, srcType, takeOnSuccess);
-  destExistentialLocation->Value = object;
-  if (takeOnSuccess) {
-    return DynamicCastResult::SuccessViaTake;
-  } else {
-    return DynamicCastResult::SuccessViaCopy;
-  }
+      auto object = bridgeAnythingToSwiftValueObject(
+        srcValue, srcType, takeOnSuccess);
+      destExistentialLocation->Value = object;
+      if (takeOnSuccess) {
+        return DynamicCastResult::SuccessViaTake;
+      } else {
+        return DynamicCastResult::SuccessViaCopy;
+      }
 # else
-  // Note: Code below works correctly on both Obj-C and non-Obj-C platforms,
-  // but the code above is slightly faster on Obj-C platforms.
-  auto object = _bridgeAnythingToObjectiveC(srcValue, srcType);
-  destExistentialLocation->Value = object;
-  return DynamicCastResult::SuccessViaCopy;
+      // Note: Code below works correctly on both Obj-C and non-Obj-C platforms,
+      // but the code above is slightly faster on Obj-C platforms.
+      auto object = _bridgeAnythingToObjectiveC(srcValue, srcType);
+      destExistentialLocation->Value = object;
+      return DynamicCastResult::SuccessViaCopy;
 #endif
+    }
+  }
+  }
 }
 
 static DynamicCastResult

test/Casting/Casts.swift

@@ -954,4 +954,44 @@ CastsTests.test("Recursive AnyHashable") {
   expectEqual(s.x, p)
 }
 
+// SR-14635 (aka rdar://78224322)
+#if _runtime(_ObjC)
+CastsTests.test("Do not overuse __SwiftValue") {
+  struct Bar {}
+  // This used to succeed because of overeager __SwiftValue
+  // boxing (and __SwiftValue does satisfy NSCopying)
+  expectFalse(Bar() is NSCopying)
+  expectFalse(Bar() as Any is NSCopying)
+
+  // This seems unavoidable?
+  // `Bar() as! AnyObject` gets boxed as a __SwiftValue,
+  // and __SwiftValue does conform to NSCopying
+  expectTrue(Bar() as! AnyObject is NSCopying)
+
+  class Foo {}
+  // Foo does not conform to NSCopying
+  // (This used to succeed due to over-eager __SwiftValue boxing)
+  expectFalse(Foo() is NSCopying)
+  expectFalse(Foo() as Any is NSCopying)
+
+  // A type that really does conform should cast to NSCopying
+  class Foo2: NSCopying {
+    func copy(with: NSZone?) -> Any { return self }
+  }
+  expectTrue(Foo2() is NSCopying)
+  expectTrue(Foo2() is AnyObject)
+}
+#endif
+
+CastsTests.test("Do not overuse __SwiftValue (non-ObjC)") {
+  struct Bar {}
+  // This should succeed because this is what __SwiftValue boxing is for
+  expectTrue(Bar() is AnyObject)
+  expectTrue(Bar() as Any is AnyObject)
+
+  class Foo {}
+  // Any class type can be cast to AnyObject
+  expectTrue(Foo() is AnyObject)
+}
+
 runAllTests()

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

@@ -173,6 +173,11 @@ contents = [
     extra_targets=["StructInt.Type"],
     roundtrips=False, # Compiler bug rejects roundtrip cast T? => Any => T?
   ),
+  Contents(name="ClassInt.Type",
+    constructor="ClassInt.self",
+    hashable=False,
+    protocols=[],
+  ),
   Contents(name="PublicProtocol.Protocol",
     constructor="PublicProtocol.self",
     hashable=False,