SR-14635: Casts to NSCopying should not always succeed

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,