Skip to content

Commit

Permalink
Use VTables for callback interfaces
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bendk committed Dec 15, 2023
1 parent b5330c3 commit 6610dfc
Show file tree
Hide file tree
Showing 39 changed files with 837 additions and 631 deletions.
3 changes: 2 additions & 1 deletion fixtures/keywords/kotlin/src/keywords.udl
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions fixtures/keywords/kotlin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>>;
Expand Down
3 changes: 2 additions & 1 deletion fixtures/keywords/rust/src/keywords.udl
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions fixtures/keywords/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>>;
Expand Down
3 changes: 1 addition & 2 deletions fixtures/proc-macro/tests/bindings/test_proc_macro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ class SwiftTestCallbackInterface : TestCallbackInterface {
}

func callbackHandler(h: Object) -> UInt32 {
var v = h.takeError(e: BasicError.InvalidInput)
return v
return h.takeError(e: BasicError.InvalidInput)
}
}

Expand Down
48 changes: 44 additions & 4 deletions uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(),
}
}
Expand Down Expand Up @@ -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()))
Expand All @@ -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,
Expand Down
143 changes: 53 additions & 90 deletions uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt
Original file line number Diff line number Diff line change
@@ -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 }}()
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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" %}
Expand Down
Loading

0 comments on commit 6610dfc

Please sign in to comment.