Skip to content

Commit

Permalink
WIP experiment with using raw pointers for objects.
Browse files Browse the repository at this point in the history
  • Loading branch information
rfk committed Dec 27, 2020
1 parent 803bb3d commit 8933531
Show file tree
Hide file tree
Showing 17 changed files with 91 additions and 53 deletions.
6 changes: 3 additions & 3 deletions examples/rondpoint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn switcheroo(b: bool) -> bool {
// Even if roundtripping works, it's possible we have
// symmetrical errors that cancel each other out.
#[derive(Debug, Clone)]
struct Retourneur;
pub struct Retourneur;
impl Retourneur {
fn new() -> Self {
Retourneur
Expand Down Expand Up @@ -129,7 +129,7 @@ impl Retourneur {
}

#[derive(Debug, Clone)]
struct Stringifier;
pub struct Stringifier;

#[allow(dead_code)]
impl Stringifier {
Expand Down Expand Up @@ -175,7 +175,7 @@ impl Stringifier {
}

#[derive(Debug, Clone)]
struct Optionneur;
pub struct Optionneur;
impl Optionneur {
fn new() -> Self {
Optionneur
Expand Down
2 changes: 1 addition & 1 deletion examples/sprites/tests/test_generated_bindings.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
uniffi_macros::build_foreign_language_testcases!(
"src/sprites.udl",
[
// "tests/bindings/test_sprites.py",
"tests/bindings/test_sprites.py",
"tests/bindings/test_sprites.kts",
"tests/bindings/test_sprites.swift",
]
Expand Down
2 changes: 1 addition & 1 deletion examples/todolist/tests/test_generated_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ uniffi_macros::build_foreign_language_testcases!(
[
"tests/bindings/test_todolist.kts",
"tests/bindings/test_todolist.swift",
// "tests/bindings/test_todolist.py"
"tests/bindings/test_todolist.py"
]
);
1 change: 1 addition & 0 deletions uniffi_bindgen/src/bindings/gecko_js/gen_gecko_js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ mod filters {
FFIType::UInt64 => "uint64_t".into(),
FFIType::Float32 => "float".into(),
FFIType::Float64 => "double".into(),
FFIType::Pointer(_) => "void*".into(), // TODO: name the type
FFIType::RustCString => "const char*".into(),
FFIType::RustBuffer => context.ffi_rustbuffer_type(),
FFIType::RustError => context.ffi_rusterror_type(),
Expand Down
1 change: 1 addition & 0 deletions uniffi_bindgen/src/bindings/kotlin/gen_kotlin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ mod filters {
FFIType::Int64 | FFIType::UInt64 => "Long".to_string(),
FFIType::Float32 => "Float".to_string(),
FFIType::Float64 => "Double".to_string(),
FFIType::Pointer(_) => "Pointer".to_string(), // TODO: name the type?
FFIType::RustCString => "Pointer".to_string(),
FFIType::RustBuffer => "RustBuffer.ByValue".to_string(),
FFIType::RustError => "RustError".to_string(),
Expand Down
12 changes: 6 additions & 6 deletions uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@
// This would be a good candidate for isolating in its own ffi-support lib.

abstract class FFIObject(
private val handle: AtomicLong
private val pointer: AtomicReference<Pointer?>
) {
open fun destroy() {
this.handle.set(0L)
this.pointer.set(null)
}

internal inline fun <R> callWithHandle(block: (handle: Long) -> R) =
this.handle.get().let { handle ->
if (handle != 0L) {
block(handle)
internal inline fun <R> callWithPointer(block: (pointer: Pointer) -> R) =
this.pointer.get().let { pointer ->
if (pointer != null) {
block(pointer)
} else {
throw IllegalStateException("${this.javaClass.simpleName} object has already been destroyed")
}
Expand Down
16 changes: 8 additions & 8 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ public interface {{ obj.name()|class_name_kt }}Interface {
}

class {{ obj.name()|class_name_kt }}(
handle: Long
) : FFIObject(AtomicLong(handle)), {{ obj.name()|class_name_kt }}Interface {
ptr: Pointer
) : FFIObject(AtomicReference(ptr)), {{ obj.name()|class_name_kt }}Interface {

{%- for cons in obj.constructors() %}
constructor({% call kt::arg_list_decl(cons) -%}) :
Expand All @@ -19,16 +19,16 @@ class {{ obj.name()|class_name_kt }}(

/**
* Disconnect the object from the underlying Rust object.
*
*
* It can be called more than once, but once called, interacting with the object
* causes an `IllegalStateException`.
*
*
* Clients **must** call this method once done with the object, or cause a memory leak.
*/
override fun destroy() {
try {
callWithHandle {
super.destroy() // poison the handle so no-one else can use it before we tell rust.
callWithPointer {
super.destroy() // poison the pointer so no-one else can use it before we tell rust.
rustCall(InternalError.ByReference()) { err ->
_UniFFILib.INSTANCE.{{ obj.ffi_object_free().name() }}(it, err)
}
Expand All @@ -43,15 +43,15 @@ class {{ obj.name()|class_name_kt }}(

{%- when Some with (return_type) -%}
override fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_protocol(meth) %}): {{ return_type|type_kt }} =
callWithHandle {
callWithPointer {
{%- call kt::to_ffi_call_with_prefix("it", meth) %}
}.let {
{{ "it"|lift_kt(return_type) }}
}

{%- when None -%}
override fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_protocol(meth) %}) =
callWithHandle {
callWithPointer {
{%- call kt::to_ffi_call_with_prefix("it", meth) %}
}
{% endmatch %}
Expand Down
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 @@ -23,7 +23,7 @@ import com.sun.jna.Pointer
import com.sun.jna.Structure
import java.nio.ByteBuffer
import java.nio.ByteOrder
import java.util.concurrent.atomic.AtomicLong
import java.util.concurrent.atomic.AtomicReference

{% include "RustBufferTemplate.kt" %}

Expand Down
1 change: 1 addition & 0 deletions uniffi_bindgen/src/bindings/python/gen_python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ mod filters {
FFIType::UInt64 => "ctypes.c_uint64".to_string(),
FFIType::Float32 => "ctypes.c_float".to_string(),
FFIType::Float64 => "ctypes.c_double".to_string(),
FFIType::Pointer(_) => "ctypes.c_voidp".to_string(), // TODO: name the type?
FFIType::RustCString => "ctypes.c_voidp".to_string(),
FFIType::RustBuffer => "RustBuffer".to_string(),
FFIType::RustError => "ctypes.POINTER(RustError)".to_string(),
Expand Down
1 change: 1 addition & 0 deletions uniffi_bindgen/src/bindings/swift/gen_swift.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ mod filters {
FFIType::UInt64 => "uint64_t".into(),
FFIType::Float32 => "float".into(),
FFIType::Float64 => "double".into(),
FFIType::Pointer(_) => "void*_Nonnull".into(), // TODO: name the type?
FFIType::RustCString => "const char*_Nonnull".into(),
FFIType::RustBuffer => "RustBuffer".into(),
FFIType::RustError => "NativeRustError".into(),
Expand Down
10 changes: 5 additions & 5 deletions uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ public protocol {{ obj.name() }}Protocol {
}

public class {{ obj.name() }}: {{ obj.name() }}Protocol {
private let handle: UInt64
private let pointer: UnsafeMutableRawPointer

{%- for cons in obj.constructors() %}
public init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} {
self.handle = {% call swift::to_ffi_call(cons) %}
self.pointer = {% call swift::to_ffi_call(cons) %}
}
{%- endfor %}

deinit {
try! rustCall(InternalError.unknown()) { err in
{{ obj.ffi_object_free().name() }}(handle, err)
{{ obj.ffi_object_free().name() }}(pointer, err)
}
}

Expand All @@ -30,13 +30,13 @@ public class {{ obj.name() }}: {{ obj.name() }}Protocol {

{%- when Some with (return_type) -%}
public func {{ meth.name()|fn_name_swift }}({% call swift::arg_list_decl(meth) %}) {% call swift::throws(meth) %} -> {{ return_type|type_swift }} {
let _retval = {% call swift::to_ffi_call_with_prefix("self.handle", meth) %}
let _retval = {% call swift::to_ffi_call_with_prefix("self.pointer", meth) %}
return {% call swift::try(meth) %} {{ "_retval"|lift_swift(return_type) }}
}

{%- when None -%}
public func {{ meth.name()|fn_name_swift }}({% call swift::arg_list_decl(meth) %}) {% call swift::throws(meth) %} {
{% call swift::to_ffi_call_with_prefix("self.handle", meth) %}
{% call swift::to_ffi_call_with_prefix("self.pointer", meth) %}
}
{%- endmatch %}
{% endfor %}
Expand Down
18 changes: 13 additions & 5 deletions uniffi_bindgen/src/interface/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ impl APIConverter<Enum> for weedle::EnumDefinition<'_> {
///
/// In WebIDL these correspond to the `interface` keyword.
///
/// TODO: these docs are wrong for raw pointer objects.
/// At the FFI layer, objects are represented by an opaque integer handle and a set of functions
/// a common prefix. The object's constuctors are functions that return new objects by handle,
/// and its methods are functions that take a handle as first argument. The foreign language
Expand Down Expand Up @@ -755,8 +756,8 @@ impl Object {
fn derive_ffi_funcs(&mut self, ci_prefix: &str) -> Result<()> {
self.ffi_func_free.name = format!("ffi_{}_{}_object_free", ci_prefix, self.name);
self.ffi_func_free.arguments = vec![FFIArgument {
name: "handle".to_string(),
type_: FFIType::UInt64,
name: "obj".to_string(),
type_: FFIType::Pointer(self.name.clone()),
}];
self.ffi_func_free.return_type = None;
for cons in self.constructors.iter_mut() {
Expand Down Expand Up @@ -795,7 +796,9 @@ impl APIConverter<Object> for weedle::InterfaceDefinition<'_> {
for member in &self.members.body {
match member {
weedle::interface::InterfaceMember::Constructor(t) => {
object.constructors.push(t.convert(ci)?);
let mut cons = t.convert(ci)?;
cons.object_name.push_str(object.name.as_str());
object.constructors.push(cons);
}
weedle::interface::InterfaceMember::Operation(t) => {
let mut method = t.convert(ci)?;
Expand All @@ -814,11 +817,13 @@ impl APIConverter<Object> for weedle::InterfaceDefinition<'_> {

// Represents a constructor for an object type.
//
// TODO: update handle docs for raw pointers.
// In the FFI, this will be a function that returns a handle for an instance
// of the corresponding object type.
#[derive(Debug, Clone)]
pub struct Constructor {
name: String,
object_name: String,
arguments: Vec<Argument>,
ffi_func: FFIFunction,
attributes: Attributes,
Expand Down Expand Up @@ -848,7 +853,7 @@ impl Constructor {
self.ffi_func.name.push_str("_");
self.ffi_func.name.push_str(&self.name);
self.ffi_func.arguments = self.arguments.iter().map(FFIArgument::from).collect();
self.ffi_func.return_type = Some(FFIType::UInt64);
self.ffi_func.return_type = Some(FFIType::Pointer(self.object_name.clone()));
Ok(())
}
}
Expand All @@ -871,6 +876,7 @@ impl Default for Constructor {
fn default() -> Self {
Constructor {
name: String::from("new"),
object_name: Default::default(),
arguments: Vec::new(),
ffi_func: Default::default(),
attributes: Attributes(Vec::new()),
Expand All @@ -882,6 +888,7 @@ impl APIConverter<Constructor> for weedle::interface::ConstructorInterfaceMember
fn convert(&self, ci: &mut ComponentInterface) -> Result<Constructor> {
Ok(Constructor {
name: String::from("new"), // TODO: get the name from an attribute maybe?
object_name: Default::default(),
arguments: self.args.body.list.convert(ci)?,
ffi_func: Default::default(),
attributes: match &self.attributes {
Expand All @@ -894,6 +901,7 @@ impl APIConverter<Constructor> for weedle::interface::ConstructorInterfaceMember

// Represents an instance method for an object type.
//
// TODO: update handle docs for raw pointers.
// The in FFI, this will be a function whose first argument is a handle for an
// instance of the corresponding object type.
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -925,7 +933,7 @@ impl Method {

pub fn first_argument(&self) -> Argument {
Argument {
name: "handle".to_string(),
name: "obj".to_string(),
// TODO: ideally we'd get this via `ci.resolve_type_expression` so that it
// is contained in the proper `TypeUniverse`, but this works for now.
type_: Type::Object(self.object_name.clone()),
Expand Down
6 changes: 4 additions & 2 deletions uniffi_bindgen/src/interface/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ pub enum FFIType {
Int64,
Float32,
Float64,
/// A pointer to a named struct type.
Pointer(String),
/// A `char*` pointer belonging to a rust-owned CString.
/// If you've got one of these, you must call the appropriate rust function to free it.
/// This is currently only used for error messages, and may go away in future.
Expand Down Expand Up @@ -124,8 +126,8 @@ impl From<&Type> for FFIType {
// Strings are always owned rust values.
// We might add a separate type for borrowed strings in future.
Type::String => FFIType::RustBuffer,
// Objects are passed as opaque integer handles.
Type::Object(_) => FFIType::UInt64,
// Objects are passed as raw pointers.
Type::Object(name) => FFIType::Pointer(name.into()),
// Enums are passed as integers.
Type::Enum(_) => FFIType::UInt32,
// Errors have their own special type.
Expand Down
1 change: 1 addition & 0 deletions uniffi_bindgen/src/scaffolding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ mod filters {
FFIType::UInt64 => "u64".into(),
FFIType::Float32 => "f32".into(),
FFIType::Float64 => "f64".into(),
FFIType::Pointer(name) => format!("Option<Box<{}>>", name),
FFIType::RustCString => "*mut std::os::raw::c_char".into(),
FFIType::RustBuffer => "uniffi::RustBuffer".into(),
FFIType::RustError => "uniffi::deps::ffi_support::ExternError".into(),
Expand Down
39 changes: 25 additions & 14 deletions uniffi_bindgen/src/templates/ObjectTemplate.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,39 @@
// For each Object definition, we assume the caller has provided an appropriately-shaped `struct`
// with an `impl` for each method on the object. We create a `ConcurrentHandleMap` for safely handing
// out references to these structs to foreign language code, and we provide a `pub extern "C"` function
// corresponding to each method.
// with an `impl` for each method on the object. We generate some boxing code in order to represent
// instances of this struct as raw pointers in the FFI, and some static assertions to ensure this
// is safe (well...TBD exactly the circumstances under which this is safe, but it's a start).
// We provide a `pub extern "C"` function corresponding to each method on the object, which takes
// an instance pointer as first argument.
//
// If the caller's implementation of the struct does not match with the methods or types specified
// in the UDL, then the rust compiler will complain with a (hopefully at least somewhat helpful!)
// error message when processing this generated code.
{% let handle_map = format!("UNIFFI_HANDLE_MAP_{}", obj.name().to_uppercase()) %}
uniffi::deps::lazy_static::lazy_static! {
static ref {{ handle_map }}: uniffi::deps::ffi_support::ConcurrentHandleMap<{{ obj.name() }}> = uniffi::deps::ffi_support::ConcurrentHandleMap::new();

// Pass objects across the FFI as pointers, in using a box.
//
// Per the `Box` docs:
// "So long as T: Sized, a Box<T> is guaranteed to be represented as a single pointer and is also ABI-compatible with C pointers".
//
// Note that implementing `IntoFfi` for `T` asserts that `T: Sized`.

unsafe impl uniffi::deps::ffi_support::IntoFfi for {{ obj.name() }} {
type Value = Option<Box<{{ obj.name() }}>>;
fn ffi_default() -> Self::Value { None }
fn into_ffi_value(self) -> Self::Value { Some(Box::new(self)) }
}

{% let ffi_free = obj.ffi_object_free() -%}
#[no_mangle]
pub extern "C" fn {{ ffi_free.name() }}(handle: u64) {
let _ = {{ handle_map }}.delete_u64(handle);
}
#[no_mangle]
pub extern "C" fn {{ obj.ffi_object_free().name() }}(obj : Option<Box<{{ obj.name() }}>>) {
drop(obj);
}

{%- for cons in obj.constructors() %}
#[allow(clippy::all)]
#[no_mangle]
pub extern "C" fn {{ cons.ffi_func().name() }}(
{%- call rs::arg_list_ffi_decl(cons.ffi_func()) %}) -> u64 {
uniffi::deps::log::debug!("{{ cons.ffi_func().name() }}");
{%- call rs::arg_list_ffi_decl(cons.ffi_func()) %}
) -> Option<Box<{{ obj.name() }}>> {
uniffi::deps::log::debug!("{{ cons.ffi_func().name() }} - raw pointer version");
// If the constructor does not have the same signature as declared in the UDL, then
// this attempt to call it will fail with a (somewhat) helpful compiler error.
{% call rs::to_rs_constructor_call(obj, cons) %}
Expand All @@ -35,7 +46,7 @@ uniffi::deps::lazy_static::lazy_static! {
pub extern "C" fn {{ meth.ffi_func().name() }}(
{%- call rs::arg_list_ffi_decl(meth.ffi_func()) %}
) -> {% call rs::return_type_func(meth) %} {
uniffi::deps::log::debug!("{{ meth.ffi_func().name() }}");
uniffi::deps::log::debug!("{{ meth.ffi_func().name() }} - raw pointer version");
// If the method does not have the same signature as declared in the UDL, then
// this attempt to call it will fail with a (somewhat) helpful compiler error.
{% call rs::to_rs_method_call(obj, meth) %}
Expand Down
Loading

0 comments on commit 8933531

Please sign in to comment.