Add callback interfaces for Swift (mozilla#1066) r=travis

* Added swift callback support * Temporary fix for running callbacks in another thread * Generate all the required swift code * Make run_uniffi_bindgen_test build * [wip] add tests for callbacks (mozilla#1) Co-authored-by: Paul Griffin <[email protected]> * Fix nits * Fit new Swift callbacks into the new Unit of Code structure Re-work to fit in to the new structure Change to FfiConverter language cargo fmt Add docs to include swift * Backfill Kotlin callbacks to closer match the Swift implementation Kotlin relies on Handle instead of Long (or ULong) Align kotlin implementation with swift * Fixup markdown weirdness * Address self-review * Address reviewer comments Co-authored-by: Paul Griffin <[email protected]> Co-authored-by: Paul Griffin <[email protected]>
saks · Oct 8, 2021 · 62a817b · 62a817b
1 parent 8deaa05
commit 62a817b
Show file tree

Hide file tree

Showing 18 changed files with 422 additions and 74 deletions.
diff --git a/docs/manual/src/tutorial/callback_interfaces.md b/docs/manual/src/tutorial/callback_interfaces.md
@@ -11,34 +11,35 @@ in Rust.
 
 # Using callback interfaces
 
-1. Define a Rust trait. 
+## 1. Define a Rust trait.
 
 This toy example defines a way of Rust accessing a key-value store exposed
 by the host operating system (e.g. the key chain).
 
-```
+```rust,no_run
 trait Keychain: Send {
   pub fn get(key: String) -> Option<String>
   pub fn put(key: String, value: String)
 }
 ```
 
-2. Define a callback interface in the UDL
+## 2. Define a callback interface in the UDL.
 
-```
+```webidl
 callback interface Keychain {
     string? get(string key);
     void put(string key, string data);
 };
 ```
 
-3. And allow it to be passed into Rust.
+## 3. And allow it to be passed into Rust.
 
 Here, we define a constructor to pass the keychain to rust, and then another method
 which may use it.
 
 In UDL:
-```
+
+```webidl
 object Authenticator {
     constructor(Keychain keychain);
     void login();
@@ -47,7 +48,7 @@ object Authenticator {
 
 In Rust:
 
-```
+```rust,no_run
 struct Authenticator {
   keychain: Box<dyn Keychain>,
 }
@@ -62,12 +63,13 @@ impl Authenticator {
   }
 }
 ```
-4. Create an foreign language implementation of the callback interface.
+
+## 4. Create an foreign language implementation of the callback interface.
 
 In this example, here's a Kotlin implementation.
 
-```
-class AndroidKeychain: Keychain {
+```kotlin
+class KotlinKeychain: Keychain {
     override fun get(key: String): String? {
         // … elide the implementation.
         return value
@@ -77,17 +79,42 @@ class AndroidKeychain: Keychain {
     }
 }
 ```
-5. Pass the implementation to Rust.
+
+…and Swift:
+
+```swift
+class SwiftKeychain: Keychain {
+    func get(key: String) -> String? {
+        // … elide the implementation.
+        return value
+    }
+    func put(key: String) {
+        // … elide the implementation.
+    }
+}
+```
+
+Note: in Swift, this must be a `class`.
+
+## 5. Pass the implementation to Rust.
 
 Again, in Kotlin
 
+```kt
+val authenticator = Authenticator(KotlinKeychain())
+// later on:
+authenticator.login()
 ```
-val authenticator = Authenticator(AndroidKeychain())
+
+and in Swift:
+
+```swift
+let authenticator = Authenticator(SwiftKeychain())
 // later on:
 authenticator.login()
 ```
 
-Care is taken to ensure that once `Box<dyn Keychain>` is dropped in Rust, then it is cleaned up in Kotlin.
+Care is taken to ensure that once `Box<dyn Keychain>` is dropped in Rust, then it is cleaned up in the foreign language.
 
 Also note, that storing the `Box<dyn Keychain>` in the `Authenticator` required that all implementations
-*must* implement `Send`.
+*must* implement `Send`.
diff --git a/examples/callbacks/tests/bindings/test_callbacks.swift b/examples/callbacks/tests/bindings/test_callbacks.swift
@@ -0,0 +1,50 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#if canImport(callbacks)
+    import callbacks
+#endif
+
+// Simple example just to see it work.
+// Pass in a string, get a string back.
+// Pass in nothing, get unit back.
+class OnCallAnsweredImpl : OnCallAnswered {
+    var yesCount: Int = 0
+    var busyCount: Int = 0
+    var stringReceived = ""
+
+    func hello() -> String {
+        yesCount += 1
+        return "Hi hi \(yesCount)"
+    }
+
+    func busy() {
+        busyCount += 1
+    }
+
+    func textReceived(text: String) {
+        stringReceived = text
+    }
+}
+
+let cbObject = OnCallAnsweredImpl()
+let telephone = Telephone()
+
+telephone.call(domestic: true, callResponder: cbObject)
+assert(cbObject.busyCount == 0, "yesCount=\(cbObject.busyCount) (should be 0)")
+assert(cbObject.yesCount == 1, "yesCount=\(cbObject.yesCount) (should be 1)")
+
+telephone.call(domestic: true, callResponder: cbObject)
+assert(cbObject.busyCount == 0, "yesCount=\(cbObject.busyCount) (should be 0)")
+assert(cbObject.yesCount == 2, "yesCount=\(cbObject.yesCount) (should be 2)")
+
+telephone.call(domestic: false, callResponder: cbObject)
+assert(cbObject.busyCount == 1, "yesCount=\(cbObject.busyCount) (should be 1)")
+assert(cbObject.yesCount == 2, "yesCount=\(cbObject.yesCount) (should be 2)")
+
+let cbObject2 = OnCallAnsweredImpl()
+telephone.call(domestic: true, callResponder: cbObject2)
+assert(cbObject2.busyCount == 0, "yesCount=\(cbObject2.busyCount) (should be 0)")
+assert(cbObject2.yesCount == 1, "yesCount=\(cbObject2.yesCount) (should be 1)")
+
diff --git a/examples/callbacks/tests/test_generated_bindings.rs b/examples/callbacks/tests/test_generated_bindings.rs
@@ -2,7 +2,7 @@ uniffi_macros::build_foreign_language_testcases!(
     "src/callbacks.udl",
     [
         "tests/bindings/test_callbacks.kts",
-        //"tests/bindings/test_callbacks.swift",
-        //"tests/bindings/test_callbacks.py",
+        "tests/bindings/test_callbacks.swift",
+        //"tests/bindings/test_callbacks.py", // see https://github.com/mozilla/uniffi-rs/pull/1068
     ]
 );
diff --git a/examples/callbacks/uniffi.toml b/examples/callbacks/uniffi.toml
@@ -1,3 +1,6 @@
 [bindings.kotlin]
 package_name = "uniffi.callbacks"
 cdylib_name = "callbacks"
+
+[bindings.swift]
+cdylib_name = "callbacks"
diff --git a/fixtures/callbacks/tests/bindings/test_callbacks.swift b/fixtures/callbacks/tests/bindings/test_callbacks.swift
@@ -0,0 +1,77 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#if canImport(callbacks)
+    import callbacks
+#endif
+
+// A bit more systematic in testing, but this time in English.
+//
+// 1. Pass in the callback as arguments.
+// Make the callback methods use multiple aruments, with a variety of types, and
+// with a variety of return types.
+let rustGetters = RustGetters()
+class SwiftGetters: ForeignGetters {
+    func getBool(v: Bool, arg2: Bool) -> Bool { v != arg2 }
+    func getString(v: String, arg2: Bool) -> String { arg2 ? "1234567890123" : v }
+    func getOption(v: String?, arg2: Bool) -> String? { arg2 ? v?.uppercased() : v }
+    func getList(v: [Int32], arg2: Bool) -> [Int32] { arg2 ? v : [] }
+}
+
+func test() {
+    let callback = SwiftGetters()
+    [true, false].forEach { v in
+        let flag = true
+        let expected = callback.getBool(v: v, arg2: flag)
+        let observed = rustGetters.getBool(callback: callback, v: v, arg2: flag)
+        assert(expected == observed, "roundtripping through callback: \(String(describing: expected)) != \(String(describing: observed))")
+    }
+
+    [[Int32(1), Int32(2)], [Int32(0), Int32(1)]].forEach { v in
+        let flag = true
+        let expected = callback.getList(v: v, arg2: flag)
+        let observed = rustGetters.getList(callback: callback, v: v, arg2: flag)
+        assert(expected == observed, "roundtripping through callback: \(String(describing: expected)) != \(String(describing: observed))")
+    }
+
+    ["Hello", "world"].forEach { v in
+        let flag = true
+        let expected = callback.getString(v: v, arg2: flag)
+        let observed = rustGetters.getString(callback: callback, v: v, arg2: flag)
+        assert(expected == observed, "roundtripping through callback: \(String(describing: expected)) != \(String(describing: observed))")
+    }
+
+    ["Some", nil].forEach { v in
+        let flag = false
+        let expected = callback.getOption(v: v, arg2: flag)
+        let observed = rustGetters.getOption(callback: callback, v: v, arg2: flag)
+        assert(expected == observed, "roundtripping through callback: \(String(describing: expected)) != \(String(describing: observed))")
+    }
+
+    assert(rustGetters.getStringOptionalCallback(callback: callback, v: "TestString", arg2: false) == "TestString")
+    assert(rustGetters.getStringOptionalCallback(callback: nil, v: "TestString", arg2: false) == nil)
+
+    // rustGetters.destroy()
+
+    // 2. Pass the callback in as a constructor argument, to be stored on the Object struct.
+    // This is crucial if we want to configure a system at startup,
+    // then use it without passing callbacks all the time.
+
+    class StoredSwiftStringifier: StoredForeignStringifier {
+        func fromSimpleType(value: Int32) -> String { "swift: \(value)" }
+        // We don't test this, but we're checking that the arg type is included in the minimal list of types used
+        // in the UDL.
+        // If this doesn't compile, then look at TypeResolver.
+        func fromComplexType(values: [Double?]?) -> String { "swift: \(String(describing: values))" }
+    }
+
+    let swiftStringifier = StoredSwiftStringifier()
+    let rustStringifier = RustStringifier(callback: swiftStringifier)
+    ([1, 2] as [Int32]).forEach { v in
+        let expected = swiftStringifier.fromSimpleType(value: v)
+        let observed = rustStringifier.fromSimpleType(value: v)
+        assert(expected == observed, "callback is sent on construction: \(expected) != \(observed)")
+    }
+
+}
diff --git a/fixtures/callbacks/tests/test_generated_bindings.rs b/fixtures/callbacks/tests/test_generated_bindings.rs
@@ -1,4 +1,8 @@
 uniffi_macros::build_foreign_language_testcases!(
     "src/callbacks.udl",
-    ["tests/bindings/test_callbacks.kts"]
+    [
+        "tests/bindings/test_callbacks.kts",
+        "tests/bindings/test_callbacks.swift",
+        //"tests/bindings/test_callbacks.py", // see https://github.com/mozilla/uniffi-rs/pull/1068
+    ]
 );
diff --git a/uniffi/src/testing.rs b/uniffi/src/testing.rs
@@ -139,13 +139,9 @@ pub fn ensure_compiled_cdylib(pkg_dir: &str) -> Result<String> {
 /// who are working on uniffi itself and want to test out their changes to the bindings generator.
 #[cfg(not(feature = "builtin-bindgen"))]
 fn run_uniffi_bindgen_test(out_dir: &str, udl_files: &[&str], test_file: &str) -> Result<()> {
-    let udl_files = udl_files
-        .into_iter()
-        .map(|&x| x)
-        .collect::<Vec<&str>>()
-        .join("\n");
+    let udl_files = udl_files.join("\n");
     let status = Command::new("uniffi-bindgen")
-        .args(&["test", out_dir, &udl_files, test_file])
+        .args(&["test", out_dir, udl_files, test_file])
         .status()?;
     if !status.success() {
         bail!("Error while running tests: {}",);

diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/callback_interface.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/callback_interface.rs
@@ -18,8 +18,8 @@ impl CallbackInterfaceCodeType {
         Self { id }
     }
 
-    fn internals(&self, oracle: &dyn CodeOracle) -> String {
-        format!("{}Internals", self.canonical_name(oracle))
+    fn ffi_converter_name(&self, oracle: &dyn CodeOracle) -> String {
+        format!("FfiConverter{}", self.canonical_name(oracle))
     }
 }
 
@@ -37,7 +37,11 @@ impl CodeType for CallbackInterfaceCodeType {
     }
 
     fn lower(&self, oracle: &dyn CodeOracle, nm: &dyn fmt::Display) -> String {
-        format!("{}.lower({})", self.internals(oracle), oracle.var_name(nm))
+        format!(
+            "{}.lower({})",
+            self.ffi_converter_name(oracle),
+            oracle.var_name(nm)
+        )
     }
 
     fn write(
@@ -48,18 +52,18 @@ impl CodeType for CallbackInterfaceCodeType {
     ) -> String {
         format!(
             "{}.write({}, {})",
-            self.internals(oracle),
+            self.ffi_converter_name(oracle),
             oracle.var_name(nm),
             target
         )
     }
 
     fn lift(&self, oracle: &dyn CodeOracle, nm: &dyn fmt::Display) -> String {
-        format!("{}.lift({})", self.internals(oracle), nm)
+        format!("{}.lift({})", self.ffi_converter_name(oracle), nm)
     }
 
     fn read(&self, oracle: &dyn CodeOracle, nm: &dyn fmt::Display) -> String {
-        format!("{}.read({})", self.internals(oracle), nm)
+        format!("{}.read({})", self.ffi_converter_name(oracle), nm)
     }
 
     fn helper_code(&self, oracle: &dyn CodeOracle) -> Option<String> {
@@ -88,7 +92,10 @@ impl KotlinCallbackInterface {
 impl CodeDeclaration for KotlinCallbackInterface {
     fn initialization_code(&self, oracle: &dyn CodeOracle) -> Option<String> {
         let code_type = CallbackInterfaceCodeType::new(self.inner.name().into());
-        Some(format!("{}.register(lib)", code_type.internals(oracle)))
+        Some(format!(
+            "{}.register(lib)",
+            code_type.ffi_converter_name(oracle)
+        ))
     }
 
     fn definition_code(&self, _oracle: &dyn CodeOracle) -> Option<String> {

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt
@@ -17,13 +17,6 @@ internal class ConcurrentHandleMap<T>(
                     }
             }
 
-    fun <R> callWithResult(handle: Handle, fn: (T) -> R): R =
-        lock.withLock {
-            leftMap[handle] ?: throw RuntimeException("Panic: handle not in handlemap")
-        }.let { obj ->
-            fn.invoke(obj)
-        }
-
     fun get(handle: Handle) = lock.withLock {
         leftMap[handle]
     }
@@ -42,14 +35,14 @@ internal class ConcurrentHandleMap<T>(
 }
 
 interface ForeignCallback : com.sun.jna.Callback {
-    public fun invoke(handle: Long, method: Int, args: RustBuffer.ByValue): RustBuffer.ByValue
+    public fun invoke(handle: Handle, method: Int, args: RustBuffer.ByValue): RustBuffer.ByValue
 }
 
 // Magic number for the Rust proxy to call using the same mechanism as every other method,
 // to free the callback once it's dropped by Rust.
 internal const val IDX_CALLBACK_FREE = 0
 
-internal abstract class CallbackInternals<CallbackInterface>(
+internal abstract class FfiConverterCallbackInterface<CallbackInterface>(
     protected val foreignCallback: ForeignCallback
 ) {
     val handleMap = ConcurrentHandleMap<CallbackInterface>()
@@ -58,11 +51,11 @@ internal abstract class CallbackInternals<CallbackInterface>(
     // This method is generated for each callback interface.
     abstract fun register(lib: _UniFFILib)
 
-    fun drop(handle: Long): RustBuffer.ByValue {
+    fun drop(handle: Handle): RustBuffer.ByValue {
         return handleMap.remove(handle).let { RustBuffer.ByValue() }
     }
 
-    fun lift(n: Long) = handleMap.get(n)
+    fun lift(n: Handle) = handleMap.get(n)
 
     fun read(buf: ByteBuffer) = lift(buf.getLong())