google / promises

Promises is a modern framework that provides a synchronization construct for Swift and Objective-C.
Apache License 2.0
3.8k stars 294 forks source link

Types are erased on the Swift/Obj-C boundary #106

Open SaidinWoT opened 5 years ago

SaidinWoT commented 5 years ago

Overview

No type information is persisted when wrapping an Objective-C FBLPromise value in a Promise, nor when deriving an Objective-C value through Promise.asObjCPromise(). The Value generic type on both init and asObjCPromise is not the same Value type that is on the class, and as such they can be freely parameterized to any combination of distinct values.

Symptoms

Erasure when wrapping

The following compiles and runs without issue, demonstrating the erasure when wrapping Objective-C:

let objCPromise = FBLPromise<NSString>.__pending()
let swiftPromise = Promise<Int>(objCPromise)

objCPromise.__onQueue(.main, then: { value in
  print(value)
  return value
})
swiftPromise.fulfill(2)

Of course, doing anything with value that would not be understood by a Swift Int causes a runtime failure.

Fulfilling in the other direction with incorrect types also compiles and runs:

let objCPromise = FBLPromise<NSString>.__pending()
let swiftPromise = Promise<Int>(objCPromise)

swiftPromise.then(on: .main) { _ in }
objCPromise.__fulfill("A value")

This snippet fails at runtime regardless of the contents of the then block, as the coercion back to Int fails.

Erasure when unwrapping

The following also compiles and runs without issue, demonstrating the erasure when moving from Swift to Objective-C:

let swiftPromise = Promise<Int>.pending()
let objCStringPromise: FBLPromise<NSString> = swiftPromise.asObjCPromise()
let objCPromisePromise: FBLPromise<Promise<Int>> = swiftPromise.asObjCPromise()
let objCDateFormatterPromise: FBLPromise<DateFormatter> = swiftPromise.asObjCPromise()

objCStringPromise.__onQueue(.main, then: { value in
  print(value)
  return value
})

swiftPromise.fulfill(2)

All derived Objective-C Promises can be interacted with as if they are the declared types, and, again, fail at runtime when interacted with in a way that the real underlying type doesn't understand.

As in the previous section, fulfilling in the other direction with incorrect types also compiles and runs:

let swiftPromise = Promise<Int>.pending()
let objCStringPromise: FBLPromise<NSString> = swiftPromise.asObjCPromise()

swiftPromise.then(on: .main) { _ in }
objCPromise.__fulfill("A value")

Again, this snippet fails at runtime regardless of the contents of the then block, as the coercion back to Int fails.

Root Cause

The concrete type used to populate a generic type parameter in a function is resolved at the call site, and function-level generic type parameters are unrelated to the type parameters of the enclosing class.

Within a given Promise instance, there is a Value symbol Promise<Value> that is usually what is being interacted with. In the two critical methods, init and asObjCPromise, they are being shadowed. Concretely, those methods look like Promise<Value>.init<Value> and Promise<Value>.asObjCPromise<Value>, which, to stress the difference in parameters, are entirely equivalent to Promise<T>.init<U> and Promise<T>.asObjCPromise<V> (U and V intentionally varied from each other).

As a result, the Value on the Objective-C FBLPromise is entirely erased from the perspective of the Swift Promise when wrapped in init. Likewise, the Value on the Swift Promise is entirely erased from the perspective of the Objective-C FBLPromise when unwrapping through asObjCPromise.

Possible Solution

The two Value parameters can be unified by way of exposing them in an extension that applies the necessary Value: AnyObject constraint on the Swift Promise's Value:

extension Promise where Value: AnyObject {
  public convenience init(_ objCPromise: FBLPromise<Value>) {
    /* ... */
  }

  public func asObjCPromise() -> FBLPromise<Value> {
    /* ... */
  }
}

This has a few immediate drawbacks:

The latter issue may break a number of existing clients in cases that were actually fine.

shoumikhin commented 5 years ago

Hi Scott,

Thanks a lot for the super detailed report! It's awesome!

Just to make sure that we have this covered, it's not supposed to import the FBLPromises module in Swift directly, and normally nobody would ever create and invoke methods on FBLPromise objects directly in Swift. When such an instance of FBLPromise is returned from an @objc func, it's recommended to wrap it with the Swift Promise type immediately.

But I believe, you used FBLPromise in the examples for brevity only to demonstrate your point, and the code is absolutely correct: we can safely assume the objCPromise objects in the examples are simply coming from the Objective-C world, or are passed back using asObjCPromise() result.

I think the root cause of the issue resides in the fact that Objective-C generics are too lightweight. Even though you explicitly specify the generic type, you can still fulfill a promise with any value (or even an error) in Objective-C and the compiler won't object. Well, maybe only issue a warning in some obvious cases. Thus, Objective-C itself is somewhat error prone on the matter.

The real type checks happen on the Swift side only (both, in compile and run time), that's why when you wrap an FBLPromise<NSString> with a Swift Promise<Int>, and then, in fact, fulfill the underlying Objective-C promise with an integer value, everything works well. The compiler simply ignored the FBLPromise generic type completely until there's still a type match on the Swift side.

In the second example, when you fulfill an Objective-C promise with something the Swift counterpart doesn't really expect, a runtime failure happens due to an assert we have. And that assertion is even possible, because Swift treats generic types more seriously than Objective-C.

As you've mentioned, making the generic Swift Value conform to AnyObject has its downsides, since not every object you want a promise to resolve with is an AnyObject, so that limitation seems less than ideal.

The current recommendation is that the generic promise types in Objective-C and Swift must match. When that doesn't happen, the client either get a runtime assertion failure in Swift (when Swift detects an Objective-C was resolved with a value of a different type than the generic one), or everything just works (when Objective-C promise was resolved with a value of the correct type, despite the lightweight generic type was different).

Hope that addresses your concerns.

BTW, we had an even more subtle issue with interoperability related to Hashable containers that you may find interesting.

Thanks.

SaidinWoT commented 5 years ago

Thanks for the detailed response!

While it's true that lightweight generics are all stripped down to id values in Objective-C, the compiler is still able to do a reasonable amount of type-checking based on them. FBLPromise.h has opted out of some of that type safety by accepting id in resolvedWith: and fulfill:, but if it did use the Value parameter, both clang and the Swift compiler would complain about fulfilling a FBLPromise<NSString> with, e.g., an NSNumber.

I also fully appreciate that internal type erasure is an absolute necessity for making a useful Swift wrapper - Objective-C lightweight generics are brought in as T: AnyObject, and, of course, not everything in Swift is an AnyObject. However, on Darwin (the only platform the Swift team expects to have to support Objective-C interop), everything in Swift may be coerced to an AnyObject (for non-class-based values, they are wrapped up in a SwiftValue). This fact is why the internal type erasure through asAnyObject works at all for types such as Int.

The proposed "possible solution" makes use of this to give real type safety in the public API. Internal type erasure does not necessitate publicly visible type erasure - the Swift compiler will enforce matching types at the wrapping/unwrapping boundary, and there is then a strong guarantee that type coercions will not fail. In particular, today I can write:

func makeIntPromise<T: AnyObject>(_ objCPromise: FBLPromise<T>) -> Promise<Int> {
  return Promise<Int>(objCPromise)
}

func unwrapStringPromise<T>(_ swiftPromise: Promise<T>) -> FBLPromise<NSString> {
  return swiftPromise.asObjCPromise()
}

While guarding that particular init and asObjCPromise in an extension with the appropriate where clause would make these illegal.

The Hashable issue is really interesting, but also fixable. This also addresses the "potential downside" mentioned in the original post.

extension Promise where Value: _ObjectiveCBridgeable {
  public static func fromBridgedPromise(_ bridgedObjCPromise: FBLPromise<Value.__ObjectiveCType) -> Promise<Value> {
    // Using the type-erasing init, which is totally fine if it's an
    // internal implementation detail
    return Promise<Value>.init(bridgedObjCPromise)
  }

  public func asBridgedObjCPromise() -> FBLPromise<Value._ObjectiveCType> {
    guard let promise = objCPromise as? FBLPromise<Value._ObjectiveCType> else {
      preconditionFailure("Cannot cast \(type(of: objCPromise)) to \(FBLPromise<Value._ObjectiveCType>.self)")
    }
    return promise
  }
}

With such an extension, the following code compiles and executes without issue:

let swiftPromise = Promise<[String: String]>.pending()
let objCPromise: FBLPromise<NSDictionary> = swiftPromise.asBridgedObjCPromise()

objCPromise.__onQueue(.main, then: { dictionary in
  XCTAssertEqual(dictionary?["a"] as! String, "b")
}
swiftPromise.fulfill(["a": "b", "c": "d"])

I've also tested this with the TestClass from the issue you linked, and no issues are encountered. Unfortunately, NSDictionary itself is brought in as having no generics, so some type safety will always be lost in that transition. The same method enables seamlessly bridging Promise<String> to FBLPromise<NSString>.

A similar approach can be used with ReferenceConvertible to cover only the set of types that don't require using a protocol that Swift wishes could be private.

ghost commented 5 years ago

Hey Scott, really appreciate the detailed feedback and the proposed solutions for improving the type safety and addressing the hashable issue. Do you mind putting those into one or more PRs so we can see the full implementations? Also, have you tested the type safety improvements / bridging with Promises containing closure types?

In the future, we would like to remove the dependency on Darwin, which will require refactoring the common layer to a C core and/or making PromisesSwift it's own standalone library with some extensions for providing interoperability support with ObjC. As of right now, it's fine to make the Darwin assumption, but just wanted to mention that as it's something we are currently looking into.

Thank you!

ghost commented 5 years ago

Hey Scott, please feel free to submit a PR (even if it's not fully functional) with the changes that you had in mind, and we can continue the discussion from there. Thank you!