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 Nov 1, 2023
1 parent 99dbabd commit cfdb66b
Show file tree
Hide file tree
Showing 43 changed files with 838 additions and 831 deletions.
22 changes: 0 additions & 22 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ members = [
"fixtures/keywords/swift",
"fixtures/metadata",
"fixtures/proc-macro",
"fixtures/reexport-scaffolding-macro",
"fixtures/regressions/enum-without-i32-helpers",
"fixtures/regressions/fully-qualified-types",
"fixtures/regressions/kotlin-experimental-unsigned-types",
Expand Down
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 @@ -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)
}
}

Expand Down
24 changes: 0 additions & 24 deletions fixtures/reexport-scaffolding-macro/Cargo.toml

This file was deleted.

197 changes: 0 additions & 197 deletions fixtures/reexport-scaffolding-macro/src/lib.rs

This file was deleted.

Loading

0 comments on commit cfdb66b

Please sign in to comment.