Skip to content

Commit

Permalink
Record metadata for a struct implementing a trait.
Browse files Browse the repository at this point in the history
```
#[uniffi::export]
impl MyTrait for MyObject { ... }
```

Previously worked as it ignored `MyTrait`. This adds new metadata to record it,
allowing foreign bindings to implement things like inheritance.

Includes Python generating an inheritance chain to reflect this relationship.

This will not generate correct code if a struct declares more than 1 trait,
and there's some undesirable re-wrapping when traits from these objects gets
passed back and forward, but seems to work surprisingly well.

On the path to mozilla#2196.
  • Loading branch information
mhammond committed Oct 4, 2024
1 parent 69ecfbd commit 317c1dd
Show file tree
Hide file tree
Showing 21 changed files with 306 additions and 31 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
- `uniffi.toml` of crates without a `lib` type where ignored in 0.28.1
- Python: Fixed a bug when enum/error names were not proper camel case (HTMLError instead of HtmlError).

### What's new?
- Proc-macros recognise when you are exporting an `impl Trait for Struct` block
so Python, Kotlin and Swift are all able to generate subclassing to reflect that.
[#2204](https://github.com/mozilla/uniffi-rs/pull/2204)

[All changes in [[UnreleasedUniFFIVersion]]](https://github.com/mozilla/uniffi-rs/compare/v0.28.1...HEAD).

## v0.28.1 (backend crates: v0.28.1) - (_2024-08-09_)
Expand Down
27 changes: 27 additions & 0 deletions docs/manual/src/proc_macro/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,33 @@ fn do_something(foo: MyFooRef) {
}
```

### Structs implementing traits.

You can declare that an object implements a trait. For example:

```
#[uniffi::export]
trait MyTrait {
// ...
}
#[derive(uniffi::Object)]
struct MyObject {}
#[uniffi::export]
impl MyObject {
// ... some methods
}
#[uniffi::export]
impl MyTrait for MyObject {
// ... the trait methods.
}
```

This will mean the bindings are able to use both the methods declared directly on `MyObject`
but also be able to be used whereever a `MyTrait` is required.

### Default values

Exported functions/methods can have default values using the `default` argument of the attribute macro that wraps them.
Expand Down
2 changes: 1 addition & 1 deletion fixtures/coverall/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ fn create_some_dict() -> SimpleDict {
float64: 0.0,
maybe_float64: Some(1.0),
coveralls: Some(Arc::new(Coveralls::new("some_dict".to_string()))),
test_trait: Some(Arc::new(traits::Trait2::default())),
test_trait: Some(Arc::new(traits::Node::default())),
}
}

Expand Down
60 changes: 51 additions & 9 deletions fixtures/coverall/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ use std::sync::{Arc, Mutex};

// namespace functions.
pub fn get_traits() -> Vec<Arc<dyn NodeTrait>> {
vec![Arc::new(Trait1::default()), Arc::new(Trait2::default())]
vec![
Arc::new(SimpleNodeImpl::default()),
Arc::new(Node::default()),
]
}

pub trait NodeTrait: Send + Sync + std::fmt::Debug {
Expand Down Expand Up @@ -163,13 +166,14 @@ pub fn test_getters(getters: Arc<dyn Getters>) {
assert!(result.is_err());
}

// An object only exposed as a `dyn T`
#[derive(Debug, Default)]
pub(crate) struct Trait1 {
pub(crate) struct SimpleNodeImpl {
// A reference to another trait.
parent: Mutex<Option<Arc<dyn NodeTrait>>>,
}

impl NodeTrait for Trait1 {
impl NodeTrait for SimpleNodeImpl {
fn name(&self) -> String {
"node-1".to_string()
}
Expand All @@ -183,21 +187,59 @@ impl NodeTrait for Trait1 {
}
}

#[derive(Debug, Default)]
pub(crate) struct Trait2 {
// An object exported and also able to be exposed as a `dyn T`
#[derive(uniffi::Object, Debug, Default)]
pub struct Node {
name: Option<String>,
parent: Mutex<Option<Arc<dyn NodeTrait>>>,
}
impl NodeTrait for Trait2 {

// The object methods.
#[uniffi::export]
impl Node {
#[uniffi::constructor]
fn new(name: String) -> Self {
Self {
parent: Mutex::new(Some(Arc::new(Node {
name: Some(format!("via {name}")),
parent: Mutex::new(None),
}))),
name: Some(name),
}
}

fn describe(&self) -> String {
format!(
"Node: name={:?}, parent={}",
self.name,
self.parent.lock().unwrap().is_some()
)
}

fn describe_parent(&self) -> String {
format!("{:?}", self.parent.lock().unwrap())
}
}

// The trait impl also exposed as methods
#[uniffi::export]
impl NodeTrait for Node {
fn name(&self) -> String {
"node-2".to_string()
self.name.clone().unwrap_or_else(|| "node-2".to_string())
}

fn set_parent(&self, parent: Option<Arc<dyn NodeTrait>>) {
*self.parent.lock().unwrap() = parent.map(|arc| Arc::clone(&arc))
*(self.parent.lock().unwrap()) = parent;
}

fn get_parent(&self) -> Option<Arc<dyn NodeTrait>> {
(*self.parent.lock().unwrap()).clone()
self.parent.lock().unwrap().clone()
}

// Even though the trait supplies a default impl, we must have one
// or the foreign code doesn't know it exists.
fn strong_count(self: Arc<Self>) -> u64 {
Arc::strong_count(&self) as u64
}
}

Expand Down
21 changes: 20 additions & 1 deletion fixtures/coverall/tests/bindings/test_coverall.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,28 @@ def test_path(self):
py_node.set_parent(None)
traits[0].set_parent(None)

def test_struct_traits(self):
# A struct, but also a trait.
node = Node("node")
# parent is a "raw" Rust object
self.assertEqual(node.describe(), "Node: name=Some(\"node\"), parent=true")
# Unfortunately, this trait object gets re-wrapped as it's passed back to Rust, because
# we can't easily know it's alreqdy the correct type.
# Nasty to rely on the repr of the inner mutex, but it neatly demonstrates the issue.
expected_desc = "Some(Node { name: Some(\"via node\"), parent: Mutex { data: None, poisoned: false, .. } })"
self.assertEqual(node.describe_parent(), expected_desc)

# Ideally it would pass the original `Arc<>` (ie, `expected_desc`), but we wrap it.
node.set_parent(node.get_parent())
# ie, this now acts like PyNode below rather than the "raw" parent.
# actual: Some(UniFFICallbackHandlerNodeTrait { handle: 18 })
#self.assertEqual(node.describe_parent(), expected_desc)

node.set_parent(PyNode())
self.assertTrue(node.describe_parent().startswith("Some(UniFFICallbackHandlerNodeTrait { handle: "))

def test_round_tripping(self):
rust_getters = make_rust_getters();
coveralls = Coveralls("test_round_tripping")
# Check that these don't cause use-after-free bugs
test_round_trip_through_rust(rust_getters)

Expand Down
34 changes: 34 additions & 0 deletions fixtures/metadata/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ pub trait Logger {
fn log(&self, message: String);
}

#[derive(uniffi::Object)]
pub struct RealLogger {}

#[uniffi::export]
impl Logger for RealLogger {
fn log(&self, _message: String) {}
}

pub use calc::Calculator;
pub use error::FlatError;
pub use person::Person;
Expand Down Expand Up @@ -861,4 +869,30 @@ mod test_function_metadata {
},
);
}

#[test]
fn test_trait_object_impl() {
check_metadata(
&UNIFFI_META_UNIFFI_FIXTURE_METADATA_INTERFACE_REALLOGGER,
ObjectMetadata {
module_path: "uniffi_fixture_metadata".into(),
name: "RealLogger".into(),
imp: ObjectImpl::Struct,
docstring: None,
},
);

check_metadata(
&UNIFFI_META_UNIFFI_FIXTURE_METADATA_AS_TRAIT_REALLOGGER_LOGGER,
AsTraitMetadata {
ty: Type::Object {
module_path: "uniffi_fixture_metadata".into(),
name: "RealLogger".into(),
imp: ObjectImpl::Struct,
},
trait_name: "Logger".into(),
tr_module_path: None,
},
);
}
}
21 changes: 21 additions & 0 deletions fixtures/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,27 @@ impl TraitWithForeign for RustTraitImpl {
}
}

// A public struct implementing a public trait.
#[derive(uniffi::Object)]
pub struct StructWithTrait {
name: String,
}

#[uniffi::export]
impl StructWithTrait {
#[uniffi::constructor]
fn new(name: String) -> Self {
Self { name }
}
}

#[uniffi::export]
impl Trait for StructWithTrait {
fn concat_strings(&self, a: &str, b: &str) -> String {
format!("{}: {a}{b}", self.name)
}
}

#[derive(uniffi::Object)]
pub struct Object;

Expand Down
2 changes: 2 additions & 0 deletions fixtures/proc-macro/tests/bindings/test_proc_macro.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
assert trait_impl.concat_strings("foo", "bar") == "foobar"
assert obj.get_trait(trait_impl).concat_strings("foo", "bar") == "foobar"
assert concat_strings_by_ref(trait_impl, "foo", "bar") == "foobar"
assert issubclass(StructWithTrait, Trait)
assert StructWithTrait("me").concat_strings("foo", "bar") == "me: foobar"

trait_impl2 = obj.get_trait_with_foreign(None)
assert trait_impl2.name() == "RustTraitImpl"
Expand Down
12 changes: 12 additions & 0 deletions uniffi_bindgen/src/bindings/python/gen_python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,18 @@ impl<'a> TypeRenderer<'a> {
.map(|(n, t)| (PythonCodeOracle.class_name(n), t))
.collect()
}

// Fetch and sort the objects to avoid forward references. For now
// just support simple things, so traits before everything else is ok.
fn iter_sorted_object_types(&self) -> impl Iterator<Item = &Type> {
let mut obs: Vec<&Type> = self
.ci
.iter_types()
.filter(|t| matches!(t, Type::Object { .. }))
.collect();
obs.sort_by_key(|t| !matches!(t, Type::Object { imp, .. } if imp.is_trait_interface()));
obs.into_iter()
}
}

#[derive(Template)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
{% if ci.is_name_used_as_error(name) %}
class {{ impl_name }}(Exception):
{%- else %}
class {{ impl_name }}:
class {{ impl_name }}({% for t in obj.as_trait_impls() %}{{ t.trait_name|class_name }},{% endfor %}):
{%- endif %}
{%- call py::docstring(obj, 4) %}
_pointer: ctypes.c_void_p
Expand Down
15 changes: 12 additions & 3 deletions uniffi_bindgen/src/bindings/python/templates/Types.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@
{%- when Type::Record { name, module_path } %}
{%- include "RecordTemplate.py" %}

{%- when Type::Object { name, module_path, imp } %}
{%- include "ObjectTemplate.py" %}

{%- when Type::Timestamp %}
{%- include "TimestampHelper.py" %}

Expand Down Expand Up @@ -98,6 +95,18 @@
{%- endmatch %}
{%- endfor %}

# objects.
{%- for type_ in self.iter_sorted_object_types() %}
{%- match type_ %}
{%- when Type::Object { name, module_path, imp } %}
{%- let type_name = type_|type_name %}
{%- let ffi_converter_name = type_|ffi_converter_name %}
{%- let canonical_type_name = type_|canonical_name %}
{%- include "ObjectTemplate.py" %}
{%- else %}
{%- endmatch %}
{%- endfor %}

{#-
Setup type aliases for our custom types, has complications due to
forward type references, #2067
Expand Down
19 changes: 17 additions & 2 deletions uniffi_bindgen/src/interface/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ pub use ffi::{
};
pub use uniffi_meta::Radix;
use uniffi_meta::{
ConstructorMetadata, LiteralMetadata, NamespaceMetadata, ObjectMetadata, TraitMethodMetadata,
UniffiTraitMetadata, UNIFFI_CONTRACT_VERSION,
AsTraitMetadata, ConstructorMetadata, LiteralMetadata, NamespaceMetadata, ObjectMetadata,
TraitMethodMetadata, UniffiTraitMetadata, UNIFFI_CONTRACT_VERSION,
};
pub type Literal = LiteralMetadata;

Expand Down Expand Up @@ -963,6 +963,21 @@ impl ComponentInterface {
Ok(())
}

pub(super) fn add_as_trait_impl(&mut self, trait_impl: AsTraitMetadata) -> Result<()> {
let object = trait_impl
.ty
.name()
.and_then(|n| get_object(&mut self.objects, n))
.ok_or_else(|| {
anyhow!(
"add_object_trait_impl: object {:?} not found",
&trait_impl.ty
)
})?;
object.as_trait_impls.push(trait_impl);
Ok(())
}

/// Perform global consistency checks on the declared interface.
///
/// This method checks for consistency problems in the declared interface
Expand Down
10 changes: 9 additions & 1 deletion uniffi_bindgen/src/interface/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
//! ```

use anyhow::Result;
use uniffi_meta::Checksum;
use uniffi_meta::{AsTraitMetadata, Checksum};

use super::callbacks;
use super::ffi::{FfiArgument, FfiCallbackFunction, FfiFunction, FfiStruct, FfiType};
Expand Down Expand Up @@ -91,6 +91,9 @@ pub struct Object {
// a regular method (albeit with a generated name)
// XXX - this should really be a HashSet, but not enough transient types support hash to make it worthwhile now.
pub(super) uniffi_traits: Vec<UniffiTrait>,
// These are traits described in our CI which this object has declared it implements.
// This allows foreign bindings to implement things like inheritance or whatever makes sense for them.
pub(super) as_trait_impls: Vec<AsTraitMetadata>,
// We don't include the FfiFuncs in the hash calculation, because:
// - it is entirely determined by the other fields,
// so excluding it is safe.
Expand Down Expand Up @@ -176,6 +179,10 @@ impl Object {
self.uniffi_traits.iter().collect()
}

pub fn as_trait_impls(&self) -> Vec<&AsTraitMetadata> {
self.as_trait_impls.iter().collect()
}

pub fn ffi_object_clone(&self) -> &FfiFunction {
&self.ffi_func_clone
}
Expand Down Expand Up @@ -316,6 +323,7 @@ impl From<uniffi_meta::ObjectMetadata> for Object {
constructors: Default::default(),
methods: Default::default(),
uniffi_traits: Default::default(),
as_trait_impls: Default::default(),
ffi_func_clone: FfiFunction {
name: ffi_clone_name,
..Default::default()
Expand Down
Loading

0 comments on commit 317c1dd

Please sign in to comment.