Merge pull request swiftlang#28700 from DougGregor/bridge-all-the-errors

[Runtime] Handle Error-conforming-to-NSObject casting fully.

Author: DougGregor

stdlib/public/runtime/Casting.cpp

@@ -588,15 +588,6 @@ swift_dynamicCastMetatypeToObjectUnconditional(const Metadata *metatype,
   }
 }
 
-// internal func _getErrorEmbeddedNSErrorIndirect<T : Error>(
-//   _ x: UnsafePointer<T>) -> AnyObject?
-#define getErrorEmbeddedNSErrorIndirect \
-  MANGLE_SYM(s32_getErrorEmbeddedNSErrorIndirectyyXlSgSPyxGs0B0RzlF)
-SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL
-id getErrorEmbeddedNSErrorIndirect(const OpaqueValue *error,
-                                   const Metadata *T,
-                                   const WitnessTable *Error);
-
 #endif
 
 /******************************************************************************/
@@ -1256,7 +1247,6 @@ static bool _dynamicCastUnknownClassIndirect(OpaqueValue *dest,
   // Okay, we're doing a conditional cast.
   void *result =
     const_cast<void*>(swift_dynamicCastUnknownClass(object, targetType));
-  assert(result == nullptr || object == result);
 
   // If the cast failed, destroy the input and return false.
   if (!result) {
@@ -1278,14 +1268,6 @@ static bool _dynamicCastUnknownClassIndirect(OpaqueValue *dest,
 /******************************** Existentials ********************************/
 /******************************************************************************/
 
-#if SWIFT_OBJC_INTEROP
-extern "C" const ProtocolDescriptor PROTOCOL_DESCR_SYM(s5Error);
-
-static const WitnessTable *findErrorWitness(const Metadata *srcType) {
-  return swift_conformsToProtocol(srcType, &PROTOCOL_DESCR_SYM(s5Error));
-}
-#endif
-
 /// Perform a dynamic cast from an existential type to some kind of
 /// class type.
 static bool _dynamicCastToUnknownClassFromExistential(OpaqueValue *dest,
@@ -1298,14 +1280,6 @@ static bool _dynamicCastToUnknownClassFromExistential(OpaqueValue *dest,
     auto classContainer =
       reinterpret_cast<ClassExistentialContainer*>(src);
     void *obj = classContainer->Value;
-#if SWIFT_OBJC_INTEROP
-    // If we're casting to NSError, we may need a representation change,
-    // so fall into the general swift_dynamicCast path.
-    if (targetType == getNSErrorMetadata()) {
-      return swift_dynamicCast(dest, src, swift_getObjectType((HeapObject*)obj),
-                               targetType, flags);
-    }
-#endif
     return _dynamicCastUnknownClassIndirect(dest, obj, targetType, flags);
   }
   case ExistentialTypeRepresentation::Opaque: {
@@ -1814,32 +1788,6 @@ static bool _dynamicCastToFunction(OpaqueValue *dest,
   }
 }
 
-/******************************************************************************/
-/****************************** Bridging NSError ******************************/
-/******************************************************************************/
-
-#if SWIFT_OBJC_INTEROP
-static id dynamicCastValueToNSError(OpaqueValue *src,
-                                    const Metadata *srcType,
-                                    const WitnessTable *srcErrorWitness,
-                                    DynamicCastFlags flags) {
-  // Check whether there is an embedded NSError.
-  if (auto embedded = getErrorEmbeddedNSErrorIndirect(src, srcType,
-                                                      srcErrorWitness)) {
-    if (flags & DynamicCastFlags::TakeOnSuccess)
-      srcType->vw_destroy(src);
-
-    return embedded;
-  }
-
-  BoxPair errorBox = swift_allocError(srcType, srcErrorWitness, src,
-                            /*isTake*/ flags & DynamicCastFlags::TakeOnSuccess);
-  auto *error = (SwiftError *)errorBox.object;
-  return _swift_stdlib_bridgeErrorToNSError(error);
-}
-
-#endif
-
 /******************************************************************************/
 /********************************* Optionals **********************************/
 /******************************************************************************/
@@ -2332,26 +2280,6 @@ static bool swift_dynamicCastImpl(OpaqueValue *dest, OpaqueValue *src,
   // Casts to class type.
   case MetadataKind::Class:
   case MetadataKind::ObjCClassWrapper:
-#if SWIFT_OBJC_INTEROP
-    // If the destination type is an NSError or NSObject, and the source type
-    // is an Error, then the cast can succeed by NSError bridging.
-    if (targetType == getNSErrorMetadata() ||
-        targetType == getNSObjectMetadata()) {
-      // Don't rebridge if the source is already some kind of NSError.
-      if (srcType->isAnyClass()
-          && swift_dynamicCastObjCClass(*reinterpret_cast<id*>(src),
-               static_cast<const ObjCClassWrapperMetadata*>(targetType)->Class))
-        return _succeed(dest, src, srcType, flags);
-      if (auto srcErrorWitness = findErrorWitness(srcType)) {
-        auto error = dynamicCastValueToNSError(src, srcType,
-                                               srcErrorWitness, flags);
-        *reinterpret_cast<id *>(dest) = error;
-        return true;
-      }
-    }
-    LLVM_FALLTHROUGH;
-#endif
-
   case MetadataKind::ForeignClass:
     switch (srcType->getKind()) {
     case MetadataKind::Class:
@@ -2387,6 +2315,21 @@ static bool swift_dynamicCastImpl(OpaqueValue *dest, OpaqueValue *src,
                                                          srcBridgeWitness,
                                                          flags);
       }
+
+#if SWIFT_OBJC_INTEROP
+      // If the destination type is an NSError or NSObject, and the source type
+      // is an Error, then the cast can succeed by NSError bridging.
+      if (targetType == getNSErrorMetadata() ||
+          targetType == getNSObjectMetadata()) {
+        if (auto srcErrorWitness = findErrorWitness(srcType)) {
+          auto error = dynamicCastValueToNSError(src, srcType,
+                                                 srcErrorWitness, flags);
+          *reinterpret_cast<id *>(dest) = error;
+          return true;
+        }
+      }
+#endif
+
       return _fail(src, srcType, targetType, flags);
     }
 
@@ -2846,9 +2789,9 @@ static id bridgeAnythingNonVerbatimToObjectiveC(OpaqueValue *src,
   // Handle Errors.
   } else if (auto srcErrorWitness = findErrorWitness(srcType)) {
     // Bridge the source value to an NSError.
-    auto box = swift_allocError(srcType, srcErrorWitness, src, consume)
-      .object;
-    return _swift_stdlib_bridgeErrorToNSError((SwiftError*)box);
+    auto flags = consume ? DynamicCastFlags::TakeOnSuccess
+                         : DynamicCastFlags::Default;
+    return dynamicCastValueToNSError(src, srcType, srcErrorWitness, flags);
   }
 
   // Fall back to boxing.

stdlib/public/runtime/ErrorObject.h

@@ -253,6 +253,17 @@ Class getNSErrorClass();
 /// Get the NSError metadata.
 const Metadata *getNSErrorMetadata();
 
+/// Find the witness table for the conformance of the given type to the
+/// Error protocol, or return nullptr if it does not conform.
+const WitnessTable *findErrorWitness(const Metadata *srcType);
+
+/// Dynamically cast a value whose conformance to the Error protocol is known
+/// into an NSError instance.
+id dynamicCastValueToNSError(OpaqueValue *src,
+                             const Metadata *srcType,
+                             const WitnessTable *srcErrorWitness,
+                             DynamicCastFlags flags);
+
 #endif
 
 SWIFT_RUNTIME_STDLIB_SPI
@@ -263,4 +274,15 @@ const size_t _swift_lldb_sizeof_SwiftError;
 
 } // namespace swift
 
+#if SWIFT_OBJC_INTEROP
+// internal func _getErrorEmbeddedNSErrorIndirect<T : Error>(
+//   _ x: UnsafePointer<T>) -> AnyObject?
+#define getErrorEmbeddedNSErrorIndirect \
+  MANGLE_SYM(s32_getErrorEmbeddedNSErrorIndirectyyXlSgSPyxGs0B0RzlF)
+SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL
+id getErrorEmbeddedNSErrorIndirect(const swift::OpaqueValue *error,
+                                   const swift::Metadata *T,
+                                   const swift::WitnessTable *Error);
+#endif
+
 #endif

stdlib/public/runtime/ErrorObject.mm

@@ -178,6 +178,31 @@ - (BOOL)isEqual:(id)other {
     swift_getObjCClassMetadata((const ClassMetadata *)getNSErrorClass()));
 }
 
+extern "C" const ProtocolDescriptor PROTOCOL_DESCR_SYM(s5Error);
+
+const WitnessTable *swift::findErrorWitness(const Metadata *srcType) {
+  return swift_conformsToProtocol(srcType, &PROTOCOL_DESCR_SYM(s5Error));
+}
+
+id swift::dynamicCastValueToNSError(OpaqueValue *src,
+                                    const Metadata *srcType,
+                                    const WitnessTable *srcErrorWitness,
+                                    DynamicCastFlags flags) {
+  // Check whether there is an embedded NSError.
+  if (id embedded = getErrorEmbeddedNSErrorIndirect(src, srcType,
+                                                    srcErrorWitness)) {
+    if (flags & DynamicCastFlags::TakeOnSuccess)
+      srcType->vw_destroy(src);
+
+    return embedded;
+  }
+
+  BoxPair errorBox = swift_allocError(srcType, srcErrorWitness, src,
+                            /*isTake*/ flags & DynamicCastFlags::TakeOnSuccess);
+  auto *error = (SwiftError *)errorBox.object;
+  return _swift_stdlib_bridgeErrorToNSError(error);
+}
+
 static Class getAndBridgeSwiftNativeNSErrorClass() {
   Class nsErrorClass = swift::getNSErrorClass();
   Class ourClass = [__SwiftNativeNSError class];

stdlib/public/runtime/SwiftObject.mm

@@ -35,6 +35,7 @@
 #include "../SwiftShims/RuntimeShims.h"
 #include "../SwiftShims/AssertionReporting.h"
 #include "CompatibilityOverride.h"
+#include "ErrorObject.h"
 #include "Private.h"
 #include "SwiftObject.h"
 #include "WeakReference.h"
@@ -1098,6 +1099,20 @@ static bool isObjCForUnownedReference(void *value) {
     return object;
   }
 
+  // For casts to NSError or NSObject, we might need to bridge via the Error
+  // protocol. Try it now.
+  if (targetType == reinterpret_cast<const ClassMetadata*>(getNSErrorClass()) ||
+      targetType == reinterpret_cast<const ClassMetadata*>([NSObject class])) {
+    auto srcType = swift_getObjCClassMetadata(
+        reinterpret_cast<const ClassMetadata*>(
+          object_getClass(id_const_cast(object))));
+    if (auto srcErrorWitness = findErrorWitness(srcType)) {
+      return dynamicCastValueToNSError((OpaqueValue*)&object, srcType,
+                                       srcErrorWitness,
+                                       DynamicCastFlags::TakeOnSuccess);
+    }
+  }
+
   return nullptr;
 }
 
@@ -1114,6 +1129,20 @@ static bool isObjCForUnownedReference(void *value) {
     return object;
   }
 
+  // For casts to NSError or NSObject, we might need to bridge via the Error
+  // protocol. Try it now.
+  if (targetType == reinterpret_cast<const ClassMetadata*>(getNSErrorClass()) ||
+      targetType == reinterpret_cast<const ClassMetadata*>([NSObject class])) {
+    auto srcType = swift_getObjCClassMetadata(
+        reinterpret_cast<const ClassMetadata*>(
+          object_getClass(id_const_cast(object))));
+    if (auto srcErrorWitness = findErrorWitness(srcType)) {
+      return dynamicCastValueToNSError((OpaqueValue*)&object, srcType,
+                                       srcErrorWitness,
+                                       DynamicCastFlags::TakeOnSuccess);
+    }
+  }
+
   Class sourceType = object_getClass(id_const_cast(object));
   swift_dynamicCastFailure(reinterpret_cast<const Metadata *>(sourceType),
                            targetType);

test/stdlib/BridgeIdAsAny.swift.gyb

@@ -175,10 +175,7 @@ protocol P {}
 %{
 testCases = [
   # testName                     type                               valueExpr                      testFunc                                     conformsToError  conformsToHashable
-
-  # disabled to unblock CI: rdar://problem/57393991
-  #  ("classes",                    "LifetimeTracked",                 "LifetimeTracked(0)",          "bridgedObjectPreservesIdentity",            True,            True),
-
+  ("classes",                    "LifetimeTracked",                 "LifetimeTracked(0)",          "bridgedObjectPreservesIdentity",            True,            True),
   ("strings",                    "String",                          '"vitameatavegamin"',          "stringBridgesToEqualNSString",              True,            True),
   ("unbridged type",             "KnownUnbridged",                  "KnownUnbridged()",            "boxedTypeRoundTripsThroughDynamicCasting",  False,           True),
   ("tuple",                      "(Int, String)",                   '(1, "2")',                    "tupleCanBeDynamicallyCast",                 False,           False),
@@ -199,6 +196,11 @@ testCases = [
 ]
 }%
 
+/// Whether this can be safely casted to NSObject
+func isNSObject<T>(_ value: T) -> Bool {
+  return (value is NSObject) && !(value is LifetimeTracked)
+}
+
 % for testName, type, valueExpr, testFunc, conformsToError, conformsToHashable in testCases:
 BridgeAnything.test("${testName}") {
   autoreleasepool {
@@ -210,7 +212,7 @@ BridgeAnything.test("${testName}") {
     let xInArray = [x]
     ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInArray) as! [AnyObject])[0])
     ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInArray) as? [AnyObject])![0])
-    if (x as? NSObject) != nil {
+    if isNSObject(x) {
       ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInArray) as! [AnyObject])[0])
       ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInArray) as? [AnyObject])![0])
     }
@@ -219,7 +221,7 @@ BridgeAnything.test("${testName}") {
     let xInDictValue = ["key" : x]
     ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictValue) as! [String: AnyObject])["key"]!)
     ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictValue) as? [String: AnyObject])!["key"]!)
-    if (x as? NSObject) != nil {
+    if isNSObject(x) {
       ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictValue) as! [String: NSObject])["key"]!)
       ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictValue) as? [String: NSObject])!["key"]!)
     }
@@ -231,7 +233,7 @@ BridgeAnything.test("${testName}") {
     // The NSObject version below can't test class LifetimeTracked.
     // ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictKey) as! [(AnyObject & Hashable): String]).keys.first!)
     // ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictKey) as? [(AnyObject & Hashable): String])!.keys.first!)
-    if (x as? NSObject) != nil {
+    if isNSObject(x) {
       ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictKey) as! [NSObject: String]).keys.first!)
       ${testFunc}(original: x, bridged: (_bridgeAnythingToObjectiveC(xInDictKey) as? [NSObject: String])!.keys.first!)
     }

test/stdlib/ErrorBridged.swift

@@ -785,6 +785,11 @@ ErrorBridgingTests.test("error-to-NSObject casts") {
 
     // "is" check
     expectTrue(error is NSObject)
+
+    // Unconditional cast to a dictionary.
+    let dict = ["key" : NoisyError()]
+    let anyOfDict = dict as AnyObject
+    let dict2 = anyOfDict as! [String: NSObject]
   }
 }