Use VTables for callback interfaces

Instead of registering a single method to implement a callback
interface, foreign code now registers a VTable.  VTable sounds fancy,
but it's just a repr(C) struct where each field is a callback function.

This gives us some more flexibility with the method signatures.
Before, all arguments were passed using a RustBuffer, but not all FFI
types can be written to a RustBuffer.  In particular, I want to be able
to pass callback function pointers.

This also makes the callback interface FFI closer to the Rust one.  I
wanted to make it match exactly, but it didn't work out.  Unfortunately,
we can't directly return the return value on Python because of an old
ctypes bug (https://bugs.python.org/issue5710). Instead, input an out
param for the return type.  The other main possibility would be to
change `RustBuffer` to be a simple `*mut u8` (mozilla#1779), which would then
be returnable by Python.  However, it seems bad to restrict ourselves
from ever returning a struct in the future. Eventually, we want to stop
using `RustBuffer` for all complex data types and that probably means
using a struct instead in some cases.

Renamed `CALL_PANIC` to `CALL_UNEXPECTED_ERROR` in the foreign bindings
templates.  This matches the name in the Rust code and makes more sense
for foreign trait implementations.

Switched the keywords fixture tests to use traits rather than callback
interfaces.  The old tests were testing unsupported things like
returning callback interfaces which just happened to pass.

Removed the reexport-scaffolding-macro fixture. I don't think this one
is giving us a lot of value anymore and I don't want to keep updating it
when the FFI changes.

Author: bendk

fixtures/keywords/kotlin/src/keywords.udl

@@ -7,7 +7,8 @@ interface break {
     void object(u8? class);
 };
 
-callback interface continue {
+[Trait]
+interface continue {
     return return(return v);
     continue? continue();
     record<u8, break> break(break? v);

fixtures/keywords/kotlin/src/lib.rs

@@ -15,9 +15,9 @@ impl r#break {
 }
 
 #[allow(non_camel_case_types)]
-trait r#continue {
+trait r#continue: Send + Sync {
     fn r#return(&self, v: r#return) -> r#return;
-    fn r#continue(&self) -> Option<Box<dyn r#continue>>;
+    fn r#continue(&self) -> Option<Arc<dyn r#continue>>;
     fn r#break(&self, _v: Option<Arc<r#break>>) -> HashMap<u8, Arc<r#break>>;
     fn r#while(&self, _v: Vec<r#while>) -> r#while;
     fn class(&self, _v: HashMap<u8, Vec<class>>) -> Option<HashMap<u8, Vec<class>>>;

fixtures/keywords/rust/src/keywords.udl

@@ -13,7 +13,8 @@ interface break {
     void async(u8? yield);
 };
 
-callback interface continue {
+[Trait]
+interface continue {
     return return(return v);
     continue? continue();
     record<u8, break> break(break? v);

fixtures/keywords/rust/src/lib.rs

@@ -16,15 +16,15 @@ impl r#break {
     pub fn r#break(&self, v: HashMap<u8, Arc<r#break>>) -> Option<HashMap<u8, Arc<r#break>>> {
         Some(v)
     }
-    fn r#continue(&self, _v: Vec<Box<dyn r#continue>>) {}
+    fn r#continue(&self, _v: Vec<Arc<dyn r#continue>>) {}
     pub fn r#yield(&self, _async: u8) {}
     pub fn r#async(&self, _yield: Option<u8>) {}
 }
 
 #[allow(non_camel_case_types)]
-pub trait r#continue {
+pub trait r#continue: Send + Sync {
     fn r#return(&self, v: r#return) -> r#return;
-    fn r#continue(&self) -> Option<Box<dyn r#continue>>;
+    fn r#continue(&self) -> Option<Arc<dyn r#continue>>;
     fn r#break(&self, _v: Option<Arc<r#break>>) -> HashMap<u8, Arc<r#break>>;
     fn r#while(&self, _v: Vec<r#while>) -> r#while;
     fn r#yield(&self, _v: HashMap<u8, Vec<r#yield>>) -> Option<HashMap<u8, Vec<r#yield>>>;

fixtures/proc-macro/tests/bindings/test_proc_macro.swift

@@ -79,8 +79,7 @@ class SwiftTestCallbackInterface : TestCallbackInterface {
     }
 
     func callbackHandler(h: Object) -> UInt32 {
-        var v = h.takeError(e: BasicError.InvalidInput)
-        return v
+        return h.takeError(e: BasicError.InvalidInput)
     }
 }
 

uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs

@@ -300,26 +300,55 @@ impl KotlinCodeOracle {
 
     /// Get the idiomatic Kotlin rendering of a variable name.
     fn var_name(&self, nm: &str) -> String {
-        format!("`{}`", nm.to_string().to_lower_camel_case())
+        format!("`{}`", self.var_name_raw(nm))
+    }
+
+    /// `var_name` without the backticks.  Useful for using in `@Structure.FieldOrder`.
+    pub fn var_name_raw(&self, nm: &str) -> String {
+        nm.to_string().to_lower_camel_case()
     }
 
     /// Get the idiomatic Kotlin rendering of an individual enum variant.
     fn enum_variant_name(&self, nm: &str) -> String {
         nm.to_string().to_shouty_snake_case()
     }
 
-    /// Get the idiomatic Python rendering of an FFI callback function
+    /// Get the idiomatic Kotlin rendering of an FFI callback function name
     fn ffi_callback_name(&self, nm: &str) -> String {
         format!("Uniffi{}", nm.to_upper_camel_case())
     }
 
+    /// Get the idiomatic Kotlin rendering of an FFI struct name
+    fn ffi_struct_name(&self, nm: &str) -> String {
+        format!("Uniffi{}", nm.to_upper_camel_case())
+    }
+
     fn ffi_type_label_by_value(&self, ffi_type: &FfiType) -> String {
         match ffi_type {
             FfiType::RustBuffer(_) => format!("{}.ByValue", self.ffi_type_label(ffi_type)),
             _ => self.ffi_type_label(ffi_type),
         }
     }
 
+    fn ffi_type_label_by_reference(&self, ffi_type: &FfiType) -> String {
+        match ffi_type {
+            FfiType::Int8
+            | FfiType::UInt8
+            | FfiType::Int16
+            | FfiType::UInt16
+            | FfiType::Int32
+            | FfiType::UInt32
+            | FfiType::Int64
+            | FfiType::UInt64
+            | FfiType::Float32
+            | FfiType::Float64 => format!("{}ByReference", self.ffi_type_label(ffi_type)),
+            FfiType::RustArcPtr(_) => "PointerByReference".to_owned(),
+            // JNA structs default to ByReference
+            FfiType::RustBuffer(_) | FfiType::Struct(_) => self.ffi_type_label(ffi_type),
+            _ => panic!("{ffi_type:?} by reference is not implemented"),
+        }
+    }
+
     fn ffi_type_label(&self, ffi_type: &FfiType) -> String {
         match ffi_type {
             // Note that unsigned integers in Kotlin are currently experimental, but java.nio.ByteBuffer does not
@@ -337,8 +366,9 @@ impl KotlinCodeOracle {
             }
             FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(),
             FfiType::Callback(name) => self.ffi_callback_name(name),
-            FfiType::ForeignCallback => "ForeignCallback".to_string(),
-            FfiType::RustFutureHandle => "Pointer".to_string(),
+            FfiType::Struct(name) => self.ffi_struct_name(name),
+            FfiType::Reference(inner) => self.ffi_type_label_by_reference(inner),
+            FfiType::VoidPointer | FfiType::RustFutureHandle => "Pointer".to_string(),
             FfiType::RustFutureContinuationData => "USize".to_string(),
         }
     }
@@ -493,6 +523,11 @@ mod filters {
         Ok(KotlinCodeOracle.var_name(nm))
     }
 
+    /// Get the idiomatic Kotlin rendering of a variable name.
+    pub fn var_name_raw(nm: &str) -> Result<String, askama::Error> {
+        Ok(KotlinCodeOracle.var_name_raw(nm))
+    }
+
     /// Get a String representing the name used for an individual enum variant.
     pub fn variant_name(v: &Variant) -> Result<String, askama::Error> {
         Ok(KotlinCodeOracle.enum_variant_name(v.name()))
@@ -508,6 +543,11 @@ mod filters {
         Ok(KotlinCodeOracle.ffi_callback_name(nm))
     }
 
+    /// Get the idiomatic Kotlin rendering of an FFI struct name
+    pub fn ffi_struct_name(nm: &str) -> Result<String, askama::Error> {
+        Ok(KotlinCodeOracle.ffi_struct_name(nm))
+    }
+
     pub fn object_names(
         obj: &Object,
         ci: &ComponentInterface,

uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt

@@ -1,107 +1,70 @@
 {% if self.include_once_check("CallbackInterfaceRuntime.kt") %}{% include "CallbackInterfaceRuntime.kt" %}{% endif %}
 
-// Implement the foreign callback handler for {{ interface_name }}
-internal class {{ callback_handler_class }} : ForeignCallback {
-    @Suppress("TooGenericExceptionCaught")
-    override fun invoke(handle: Handle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int {
-        val cb = {{ ffi_converter_name }}.handleMap.get(handle)
-        return when (method) {
-            IDX_CALLBACK_FREE -> {
-                {{ ffi_converter_name }}.handleMap.remove(handle)
+{%- let trait_impl=format!("uniffiCallbackInterface{}", name) %}
 
-                // Successful return
-                // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs`
-                UNIFFI_CALLBACK_SUCCESS
-            }
-            {% for meth in methods.iter() -%}
-            {% let method_name = format!("invoke_{}", meth.name())|fn_name -%}
-            {{ loop.index }} -> {
-                // Call the method, write to outBuf and return a status code
-                // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` for info
-                try {
-                    this.{{ method_name }}(cb, argsData, argsLen, outBuf)
-                } catch (e: Throwable) {
-                    // Unexpected error
-                    try {
-                        // Try to serialize the error into a string
-                        outBuf.setValue({{ Type::String.borrow()|ffi_converter_name }}.lower(e.toString()))
-                    } catch (e: Throwable) {
-                        // If that fails, then it's time to give up and just return
-                    }
-                    UNIFFI_CALLBACK_UNEXPECTED_ERROR
-                }
-            }
-            {% endfor %}
-            else -> {
-                // An unexpected error happened.
-                // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs`
-                try {
-                    // Try to serialize the error into a string
-                    outBuf.setValue({{ Type::String.borrow()|ffi_converter_name }}.lower("Invalid Callback index"))
-                } catch (e: Throwable) {
-                    // If that fails, then it's time to give up and just return
-                }
-                UNIFFI_CALLBACK_UNEXPECTED_ERROR
+// Put the implementation in an object so we don't pollute the top-level namespace
+internal object {{ trait_impl }} {
+    {%- for (ffi_callback, meth) in vtable_methods.iter() %}
+    internal object {{ meth.name()|var_name }}: {{ ffi_callback.name()|ffi_callback_name }} {
+        override fun callback(
+            {%- for arg in ffi_callback.arguments() -%}
+            {{ arg.name().borrow()|var_name }}: {{ arg.type_().borrow()|ffi_type_name_by_value }},
+            {%- endfor -%}
+            {%- if ffi_callback.has_rust_call_status_arg() -%}
+            uniffiCallStatus: RustCallStatus,
+            {%- endif -%}
+        )
+        {%- match ffi_callback.return_type() %}
+        {%- when Some(return_type) %}: {{ return_type|ffi_type_name_by_value }},
+        {%- when None %}
+        {%- endmatch %} {
+            val uniffiObj = {{ ffi_converter_name }}.handleMap.get(uniffiHandle)
+            val makeCall = { ->
+                uniffiObj.{{ meth.name()|fn_name() }}(
+                    {%- for arg in meth.arguments() %}
+                    {{ arg|lift_fn }}({{ arg.name()|var_name }}),
+                    {%- endfor %}
+                )
             }
-        }
-    }
 
-    {% for meth in methods.iter() -%}
-    {% let method_name = format!("invoke_{}", meth.name())|fn_name %}
-    @Suppress("UNUSED_PARAMETER")
-    private fun {{ method_name }}(kotlinCallbackInterface: {{ interface_name }}, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int {
-        {%- if meth.arguments().len() > 0 %}
-        val argsBuf = argsData.getByteBuffer(0, argsLen.toLong()).also {
-            it.order(ByteOrder.BIG_ENDIAN)
-        }
-        {%- endif %}
+            {%- match meth.return_type() %}
+            {%- when Some(return_type) %}
+            val writeReturn = { value: {{ return_type|type_name(ci) }} -> uniffiOutReturn.setValue({{ return_type|lower_fn }}(value)) }
+            {%- when None %}
+            val writeReturn = { _: Unit -> Unit }
+            {%- endmatch %}
 
-        {%- match meth.return_type() %}
-        {%- when Some with (return_type) %}
-        fun makeCall() : Int {
-            val returnValue = kotlinCallbackInterface.{{ meth.name()|fn_name }}(
-                {%- for arg in meth.arguments() %}
-                {{ arg|read_fn }}(argsBuf)
-                {% if !loop.last %}, {% endif %}
-                {%- endfor %}
-            )
-            outBuf.setValue({{ return_type|ffi_converter_name }}.lowerIntoRustBuffer(returnValue))
-            return UNIFFI_CALLBACK_SUCCESS
-        }
-        {%- when None %}
-        fun makeCall() : Int {
-            kotlinCallbackInterface.{{ meth.name()|fn_name }}(
-                {%- for arg in meth.arguments() %}
-                {{ arg|read_fn }}(argsBuf)
-                {%- if !loop.last %}, {% endif %}
-                {%- endfor %}
+            {%- match meth.throws_type() %}
+            {%- when None %}
+            uniffiTraitInterfaceCall(uniffiCallStatus, makeCall, writeReturn)
+            {%- when Some(error_type) %}
+            uniffiTraitInterfaceCallWithError(
+                uniffiCallStatus,
+                makeCall,
+                writeReturn,
+                { e: {{error_type|type_name(ci) }} -> {{ error_type|lower_fn }}(e) }
             )
-            return UNIFFI_CALLBACK_SUCCESS
+            {%- endmatch %}
         }
-        {%- endmatch %}
+    }
+    {%- endfor %}
 
-        {%- match meth.throws_type() %}
-        {%- when None %}
-        fun makeCallAndHandleError() : Int = makeCall()
-        {%- when Some(error_type) %}
-        fun makeCallAndHandleError()  : Int = try {
-            makeCall()
-        } catch (e: {{ error_type|type_name(ci) }}) {
-            // Expected error, serialize it into outBuf
-            outBuf.setValue({{ error_type|ffi_converter_name }}.lowerIntoRustBuffer(e))
-            UNIFFI_CALLBACK_ERROR
+    internal object uniffiFree: {{ "CallbackInterfaceFree"|ffi_callback_name }} {
+        override fun callback(handle: Long) {
+            {{ ffi_converter_name }}.handleMap.remove(handle)
         }
-        {%- endmatch %}
-
-        return makeCallAndHandleError()
     }
-    {% endfor %}
+
+    internal var vtable = {{ vtable|ffi_type_name_by_value }}(
+        {%- for (ffi_callback, meth) in vtable_methods.iter() %}
+        {{ meth.name()|var_name() }},
+        {%- endfor %}
+        uniffiFree
+    )
 
     // Registers the foreign callback with the Rust side.
     // This method is generated for each callback interface.
     internal fun register(lib: _UniFFILib) {
-        lib.{{ ffi_init_callback.name() }}(this)
+        lib.{{ ffi_init_callback.name() }}(vtable)
     }
 }
-
-internal val {{ callback_handler_obj }} = {{ callback_handler_class }}()

uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt

@@ -32,10 +32,6 @@ internal class ConcurrentHandleMap<T>(
         }
 }
 
-interface ForeignCallback : com.sun.jna.Callback {
-    public fun invoke(handle: Handle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int
-}
-
 // 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

uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceTemplate.kt

@@ -1,10 +1,10 @@
 {%- let cbi = ci|get_callback_interface_definition(name) %}
-{%- let callback_handler_class = format!("UniffiCallbackInterface{}", name) %}
-{%- let callback_handler_obj = format!("uniffiCallbackInterface{}", name) %}
 {%- let ffi_init_callback = cbi.ffi_init_callback() %}
 {%- let interface_name = cbi|type_name(ci) %}
-{%- let methods = cbi.methods() %}
 {%- let interface_docstring = cbi.docstring() %}
+{%- let methods = cbi.methods() %}
+{%- let vtable = cbi.vtable() %}
+{%- let vtable_methods = cbi.vtable_methods() %}
 
 {% include "Interface.kt" %}
 {% include "CallbackInterfaceImpl.kt" %}