Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A strawfox for #417 to allow an interface to have a method which take… #418

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions examples/threadsafe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use std::sync::atomic::{AtomicBool, AtomicI32, Ordering};
use std::sync::Arc;

/// Simulation of a task doing something to keep a thread busy.
/// Up until now, everything has been synchronous and blocking, so
Expand Down Expand Up @@ -53,6 +54,7 @@ impl Counter {
// It relies on its own locking mechanisms, and uses the `Threadsafe`
// attribute to tell uniffi to use the `ArcHandleMap`, so avoid the default
// locking strategy.
#[derive(Debug)]
struct ThreadsafeCounter {
is_busy: AtomicBool,
count: AtomicI32,
Expand All @@ -62,6 +64,7 @@ struct ThreadsafeCounter {
// `Send` and `Sync`.
static_assertions::assert_impl_all!(ThreadsafeCounter: Send, Sync);


impl ThreadsafeCounter {
fn new() -> Self {
Self {
Expand All @@ -83,6 +86,11 @@ impl ThreadsafeCounter {
self.count.load(Ordering::SeqCst)
}
}

fn cloneable(self: Arc<Self>) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be clear, I think we want a new example rather than abusing this, but this is OK for demo purposes

// `self` is already an Arc::clone() so can be used directly.
println!("Got {:?}", self)
}
}

include!(concat!(env!("OUT_DIR"), "/threadsafe.uniffi.rs"));
4 changes: 4 additions & 0 deletions examples/threadsafe/src/threadsafe.udl
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@ interface Counter {
interface ThreadsafeCounter {
void busy_wait(i32 ms);
i32 increment_if_busy();
// A function that wants a signature of `fn cloneable(self: Arc<Self>)`
// so that it's capable of sending an Arc::clone somewhere else.
[SomethingSomethingArc] // probably want a better attribute? ;)
void cloneable();
};

10 changes: 5 additions & 5 deletions uniffi/src/ffi/handle_maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> Result<R, E>,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> Result<R, E>,
ExternError: From<E>,
R: IntoFfi,
{
Expand All @@ -175,7 +175,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
let obj = map.get(h)?;
Arc::clone(&obj)
};
Ok(callback(&*obj)?)
Ok(callback(obj)?)
})
}

Expand All @@ -187,7 +187,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> R,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> R,
R: IntoFfi,
{
self.call_with_result(out_error, h, |r| -> Result<_, HandleError> {
Expand Down Expand Up @@ -313,7 +313,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> Result<R, E>,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> Result<R, E>,
ExternError: From<E>,
R: IntoFfi,
{
Expand All @@ -327,7 +327,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> R,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> R,
R: IntoFfi,
{
self.call_with_output(out_error, h, callback)
Expand Down
9 changes: 8 additions & 1 deletion uniffi_bindgen/src/interface/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ use anyhow::{bail, Result};
/// This is a convenience enum for parsing UDL attributes and erroring out if we encounter
/// any unsupported ones. These don't convert directly into parts of a `ComponentInterface`, but
/// may influence the properties of things like functions and arguments.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq)]
pub(super) enum Attribute {
ByRef,
Error,
Threadsafe,
Throws(String),
SomethingSomethingArc,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the need for this, so I'm not going to much help naming this.

}

impl Attribute {
Expand All @@ -50,6 +51,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttribute<'_>> for Attribute {
"ByRef" => Ok(Attribute::ByRef),
"Error" => Ok(Attribute::Error),
"Threadsafe" => Ok(Attribute::Threadsafe),
"SomethingSomethingArc" => Ok(Attribute::SomethingSomethingArc),
_ => anyhow::bail!("ExtendedAttributeNoArgs not supported: {:?}", (attr.0).0),
},
// Matches assignment-style attributes like ["Throws=Error"]
Expand Down Expand Up @@ -145,6 +147,10 @@ impl FunctionAttributes {
_ => None,
})
}

pub(super) fn get_something_something_arc(&self) -> bool {
self.0.iter().any(|a| *a == Attribute::SomethingSomethingArc)
}
}

impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for FunctionAttributes {
Expand All @@ -154,6 +160,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for FunctionAttribut
) -> Result<Self, Self::Error> {
let attrs = parse_attributes(weedle_attributes, |attr| match attr {
Attribute::Throws(_) => Ok(()),
Attribute::SomethingSomethingArc => Ok(()), // XXX - not valid in ctors?
_ => bail!(format!(
"{:?} not supported for functions, methods or constructors",
attr
Expand Down
4 changes: 4 additions & 0 deletions uniffi_bindgen/src/interface/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,10 @@ impl Method {
self.attributes.get_throws_err()
}

pub fn something_something_arc(&self) -> bool {
self.attributes.get_something_something_arc()
}

pub fn derive_ffi_func(&mut self, ci_prefix: &str, obj_prefix: &str) -> Result<()> {
self.ffi_func.name.push_str(ci_prefix);
self.ffi_func.name.push_str("_");
Expand Down
8 changes: 8 additions & 0 deletions uniffi_bindgen/src/templates/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,20 @@ use uniffi::UniffiMethodCall;
{% match meth.throws() -%}
{% when Some with (e) -%}
{{ this_handle_map }}.method_call_with_result(err, {{ meth.first_argument().name() }}, |obj| -> Result<{% call return_type_func(meth) %}, {{e}}> {
{%- if meth.something_something_arc() -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("obj", meth) -%}?;
{%- else -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("&*obj", meth) -%}?;
{%- endif -%}
Comment on lines +69 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method_call_with_result is doing the work of choosing between the two handle_map implementations, so this else block is being seen by both Threadsafe and non-Threadsafe objects.

Ok({% call ret(meth) %})
})
{% else -%}
{{ this_handle_map }}.method_call_with_output(err, {{ meth.first_argument().name() }}, |obj| {
{%- if meth.something_something_arc() -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("obj", meth) -%};
{%- else -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("&*obj", meth) -%};
{%- endif -%}
{% call ret(meth) %}
})
{% endmatch -%}
Expand Down