Skip to content

Commit

Permalink
Review 3 comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jhugman committed Jan 21, 2021
1 parent 90e4743 commit 3f965b8
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 32 deletions.
2 changes: 1 addition & 1 deletion examples/callbacks/src/callbacks.udl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface RustGetters {
/// These objects are implemented by the foreign language and passed
/// to Rust. Rust then calls methods on it when it needs to.
/// Rust developers need to declare these traits extending `Send` so
/// They can be stored in Rust— i.e. not passed in as an argument to
/// they can be stored in Rust— i.e. not passed in as an argument to
/// be used immediately.
callback interface StoredForeignStringifier {
string from_simple_type(i32 value);
Expand Down
2 changes: 1 addition & 1 deletion examples/callbacks/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl Telephone {
let _ = call_responder.hello();
} else {
call_responder.busy();
call_responder.text_received("Pas maintenant!".into());
call_responder.text_received("Not now, I'm on another call!".into());
}
}
}
Expand Down
12 changes: 1 addition & 11 deletions uniffi/src/ffi/foreigncallbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,6 @@ pub struct ForeignCallbackInternals {
callback_ptr: AtomicUsize,
}

impl Default for ForeignCallbackInternals {
fn default() -> Self {
ForeignCallbackInternals {
callback_ptr: AtomicUsize::new(0),
}
}
}

impl ForeignCallbackInternals {
pub const fn new() -> Self {
ForeignCallbackInternals {
Expand All @@ -57,9 +49,7 @@ impl ForeignCallbackInternals {
.compare_and_swap(0, as_usize, Ordering::SeqCst);
if old_ptr != 0 {
// This is an internal bug, the other side of the FFI should ensure
// it sets this only once. Note that this is actually going to be
// before logging is initialized in practice, so there's not a lot
// we can actually do here.
// it sets this only once.
panic!("Bug: call set_callback multiple times. This is likely a uniffi bug");
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% let type_name = obj.name()|class_name_kt %}
{% let type_name = cbi.name()|class_name_kt %}
public interface {{ type_name }} {
{% for meth in obj.methods() -%}
{% for meth in cbi.methods() -%}
fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_decl(meth) %})
{%- match meth.return_type() -%}
{%- when Some with (return_type) %}: {{ return_type|type_kt -}}
Expand All @@ -9,7 +9,7 @@ public interface {{ type_name }} {
{% endfor %}
}

{% let canonical_type_name = format!("CallbackInterface{}", type_name) %}
{% let canonical_type_name = cbi.type_().canonical_name()|class_name_kt %}
{% let callback_internals = format!("{}Internals", canonical_type_name) -%}
{% let callback_interface_impl = format!("{}FFI", canonical_type_name) -%}

Expand All @@ -19,7 +19,7 @@ internal class {{ callback_interface_impl }} : ForeignCallback {
return {{ callback_internals }}.handleMap.callWithResult(handle) { cb ->
when (method) {
IDX_CALLBACK_FREE -> {{ callback_internals }}.drop(handle)
{% for meth in obj.methods() -%}
{% for meth in cbi.methods() -%}
{% let method_name = format!("invoke_{}", meth.name())|fn_name_kt -%}
{{ loop.index }} -> this.{{ method_name }}(cb, args)
{% endfor %}
Expand All @@ -28,7 +28,7 @@ internal class {{ callback_interface_impl }} : ForeignCallback {
}
}

{% for meth in obj.methods() -%}
{% for meth in cbi.methods() -%}
{% let method_name = format!("invoke_{}", meth.name())|fn_name_kt %}
private fun {{ method_name }}(kotlinCallbackInterface: {{ type_name }}, args: RustBuffer.ByValue): RustBuffer.ByValue =
try {
Expand Down Expand Up @@ -69,7 +69,7 @@ internal object {{ callback_internals }}: CallbackInternals<{{ type_name }}>(
) {
override fun register(lib: _UniFFILib) {
rustCall(InternalError.ByReference()) { err ->
lib.{{ obj.ffi_init_callback().name() }}(this.foreignCallback, err)
lib.{{ cbi.ffi_init_callback().name() }}(this.foreignCallback, err)
}
}
}
2 changes: 1 addition & 1 deletion uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ import kotlin.concurrent.withLock
{% endfor %}

// Callback Interfaces
{% for obj in ci.iter_callback_interface_definitions() %}
{% for cbi in ci.iter_callback_interface_definitions() %}
{% include "CallbackInterfaceTemplate.kt" %}
{% endfor %}

Expand Down
4 changes: 4 additions & 0 deletions uniffi_bindgen/src/interface/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,10 @@ impl CallbackInterface {
&self.name
}

pub fn type_(&self) -> Type {
Type::CallbackInterface(self.name.clone())
}

pub fn methods(&self) -> Vec<&Method> {
self.methods.iter().collect()
}
Expand Down
15 changes: 4 additions & 11 deletions uniffi_bindgen/src/templates/CallbackInterfaceTemplate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@
// - for each method, arguments will be packed into a `RustBuffer` and sent over the `ForeignCallback` to be
// unpacked and called. The return value is packed into another `RustBuffer` and sent back to Rust.
// - a `Drop` `impl`, which tells the foreign language to forget about the real callback object.
// - a `Send` `impl` so `Object`s can store callbacks.
#}
{% let trait_name = obj.name() -%}
{% let trait_name = cbi.name() -%}
{% let trait_impl = format!("{}Proxy", trait_name) -%}
{% let foreign_callback_internals = format!("foreign_callback_{}_internals", trait_name)|upper -%}

// Register a foreign callback for getting across the FFI.
static {{ foreign_callback_internals }}: uniffi::ForeignCallbackInternals = uniffi::ForeignCallbackInternals::new();

#[no_mangle]
pub extern "C" fn {{ obj.ffi_init_callback().name() }}(callback: uniffi::ForeignCallback) {
pub extern "C" fn {{ cbi.ffi_init_callback().name() }}(callback: uniffi::ForeignCallback) {
{{ foreign_callback_internals }}.set_callback(callback);
}

Expand All @@ -29,12 +28,6 @@ struct {{ trait_impl }} {
handle: u64
}

// We need Send so we can stash the callbacks in objects we're sharing with foreign languages
// i.e. in the handle map.
// Prepared for our target trait to declare:
// `trait {{ trait_name }}: Send + std::fmt::Debug`
unsafe impl Send for {{ trait_impl }} {}

impl Drop for {{ trait_impl }} {
fn drop(&mut self) {
let callback = {{ foreign_callback_internals }}.get_callback().unwrap();
Expand All @@ -43,7 +36,7 @@ impl Drop for {{ trait_impl }} {
}

impl {{ trait_name }} for {{ trait_impl }} {
{%- for meth in obj.methods() %}
{%- for meth in cbi.methods() %}

{#- Method declaration #}
fn {{ meth.name() -}}
Expand All @@ -53,7 +46,7 @@ impl {{ trait_name }} for {{ trait_impl }} {
{% else -%}
{%- endmatch -%} {
{#- Method body #}
uniffi::deps::log::debug!("{{ obj.name() }}.{{ meth.name() }}");
uniffi::deps::log::debug!("{{ cbi.name() }}.{{ meth.name() }}");

{#- Packing args into a RustBuffer #}
{% if meth.arguments().len() == 0 -%}
Expand Down
2 changes: 1 addition & 1 deletion uniffi_bindgen/src/templates/scaffolding_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
{% endfor %}

// Callback Interface defitions, corresponding to UDL `callback interface` definitions.
{% for obj in ci.iter_callback_interface_definitions() %}
{% for cbi in ci.iter_callback_interface_definitions() %}
{% include "CallbackInterfaceTemplate.rs" %}
{% endfor %}

Expand Down

0 comments on commit 3f965b8

Please sign in to comment.