Skip to content

Commit

Permalink
Ensure structs and traits exposed via procmacros also staticallly ass…
Browse files Browse the repository at this point in the history
…ert Send+Sync (mozilla#1727)
  • Loading branch information
mhammond authored Sep 7, 2023
1 parent 1eabe86 commit 4fc37dc
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 32 deletions.
20 changes: 20 additions & 0 deletions fixtures/uitests/tests/ui/interface_not_sync_and_send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,23 @@ impl Counter {
new_value
}
}

#[derive(uniffi::Object)]
pub struct ProcMacroCounter {
// This will fail to compile, because `Cell` is not `Sync`.
value: Cell<u32>,
}

#[uniffi::export]
impl ProcMacroCounter {
#[uniffi::constructor]
pub fn new() -> std::sync::Arc<Self> {
std::sync::Arc::new(Self { value: Cell::new(0) })
}

pub fn increment(&self) -> u32 {
let new_value = self.value.get() + 1;
self.value.set(new_value);
new_value
}
}
32 changes: 26 additions & 6 deletions fixtures/uitests/tests/ui/interface_not_sync_and_send.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: `Cell<u32>` cannot be shared between threads safely
--> $OUT_DIR[uniffi_uitests]/counter.uniffi.rs
|
| uniffi::deps::static_assertions::assert_impl_all!(r#Counter: Sync, Send);
| ^^^^^^^^^ `Cell<u32>` cannot be shared between threads safely
| struct r#Counter { }
| ^^^^^^^^^ `Cell<u32>` cannot be shared between threads safely
|
= help: within `Counter`, the trait `Sync` is not implemented for `Cell<u32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU32` instead
Expand All @@ -11,9 +11,29 @@ note: required because it appears within the type `Counter`
|
9 | pub struct Counter {
| ^^^^^^^
note: required by a bound in `assert_impl_all`
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> $OUT_DIR[uniffi_uitests]/counter.uniffi.rs
|
| uniffi::deps::static_assertions::assert_impl_all!(r#Counter: Sync, Send);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the macro `uniffi::deps::static_assertions::assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)
| #[::uniffi::expand_interface_support(tag = crate::UniFfiTag)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the macro `uniffi::deps::static_assertions::assert_impl_all` which comes from the expansion of the attribute macro `::uniffi::expand_interface_support` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `Cell<u32>` cannot be shared between threads safely
--> tests/ui/interface_not_sync_and_send.rs:27:12
|
27 | pub struct ProcMacroCounter {
| ^^^^^^^^^^^^^^^^ `Cell<u32>` cannot be shared between threads safely
|
= help: within `ProcMacroCounter`, the trait `Sync` is not implemented for `Cell<u32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU32` instead
note: required because it appears within the type `ProcMacroCounter`
--> tests/ui/interface_not_sync_and_send.rs:27:12
|
27 | pub struct ProcMacroCounter {
| ^^^^^^^^^^^^^^^^
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> tests/ui/interface_not_sync_and_send.rs:26:10
|
26 | #[derive(uniffi::Object)]
| ^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the macro `uniffi::deps::static_assertions::assert_impl_all` which comes from the expansion of the derive macro `uniffi::Object` (in Nightly builds, run with -Z macro-backtrace for more info)
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@ fn main() { /* empty main required by `trybuild` */}
// This will fail to compile, because the trait is not explicit Send+Sync
pub trait Trait {
}

// This will fail to compile, because the trait is not explicit Send+Sync
#[uniffi::export]
pub trait ProcMacroTrait {
}
52 changes: 40 additions & 12 deletions fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr
Original file line number Diff line number Diff line change
@@ -1,27 +1,55 @@
error[E0277]: `dyn Trait` cannot be shared between threads safely
--> $OUT_DIR[uniffi_uitests]/trait.uniffi.rs
|
| uniffi::deps::static_assertions::assert_impl_all!(dyn r#Trait: Sync, Send);
| ^^^^^^^^^^^ `dyn Trait` cannot be shared between threads safely
| ::uniffi::expand_trait_interface_support!(r#Trait);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `dyn Trait` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `dyn Trait`
note: required by a bound in `assert_impl_all`
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> $OUT_DIR[uniffi_uitests]/trait.uniffi.rs
|
| uniffi::deps::static_assertions::assert_impl_all!(dyn r#Trait: Sync, Send);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the macro `uniffi::deps::static_assertions::assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)
| ::uniffi::expand_trait_interface_support!(r#Trait);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the macro `::uniffi::expand_trait_interface_support` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `dyn Trait` cannot be sent between threads safely
--> $OUT_DIR[uniffi_uitests]/trait.uniffi.rs
|
| uniffi::deps::static_assertions::assert_impl_all!(dyn r#Trait: Sync, Send);
| ^^^^^^^^^^^ `dyn Trait` cannot be sent between threads safely
| ::uniffi::expand_trait_interface_support!(r#Trait);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `dyn Trait` cannot be sent between threads safely
|
= help: the trait `Send` is not implemented for `dyn Trait`
note: required by a bound in `assert_impl_all`
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> $OUT_DIR[uniffi_uitests]/trait.uniffi.rs
|
| uniffi::deps::static_assertions::assert_impl_all!(dyn r#Trait: Sync, Send);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the macro `uniffi::deps::static_assertions::assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)
| ::uniffi::expand_trait_interface_support!(r#Trait);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the macro `::uniffi::expand_trait_interface_support` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `dyn ProcMacroTrait` cannot be shared between threads safely
--> tests/ui/interface_trait_not_sync_and_send.rs:11:1
|
11 | #[uniffi::export]
| ^^^^^^^^^^^^^^^^^ `dyn ProcMacroTrait` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `dyn ProcMacroTrait`
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> tests/ui/interface_trait_not_sync_and_send.rs:11:1
|
11 | #[uniffi::export]
| ^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `dyn ProcMacroTrait` cannot be sent between threads safely
--> tests/ui/interface_trait_not_sync_and_send.rs:11:1
|
11 | #[uniffi::export]
| ^^^^^^^^^^^^^^^^^ `dyn ProcMacroTrait` cannot be sent between threads safely
|
= help: the trait `Send` is not implemented for `dyn ProcMacroTrait`
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> tests/ui/interface_trait_not_sync_and_send.rs:11:1
|
11 | #[uniffi::export]
| ^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info)
10 changes: 2 additions & 8 deletions uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,12 @@

{%- match obj.imp() -%}
{%- when ObjectImpl::Trait %}
::uniffi::scaffolding_ffi_converter_trait_interface!(r#{{ obj.name() }});
::uniffi::expand_trait_interface_support!(r#{{ obj.name() }});
{% else %}
#[::uniffi::ffi_converter_interface(tag = crate::UniFfiTag)]
#[::uniffi::expand_interface_support(tag = crate::UniFfiTag)]
struct {{ obj.rust_name() }} { }
{% endmatch %}

// All Object structs must be `Sync + Send`. The generated scaffolding will fail to compile
// if they are not, but unfortunately it fails with an unactionably obscure error message.
// By asserting the requirement explicitly, we help Rust produce a more scrutable error message
// and thus help the user debug why the requirement isn't being met.
uniffi::deps::static_assertions::assert_impl_all!({{ obj.rust_name() }}: Sync, Send);

{% let ffi_free = obj.ffi_object_free() -%}
#[doc(hidden)]
#[no_mangle]
Expand Down
6 changes: 6 additions & 0 deletions uniffi_macros/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ pub(crate) fn ffi_converter_trait_impl(trait_ident: &Ident, tag: Option<&Path>)
};

quote! {
// All traits must be `Sync + Send`. The generated scaffolding will fail to compile
// if they are not, but unfortunately it fails with an unactionably obscure error message.
// By asserting the requirement explicitly, we help Rust produce a more scrutable error message
// and thus help the user debug why the requirement isn't being met.
uniffi::deps::static_assertions::assert_impl_all!(dyn #trait_ident: Sync, Send);

unsafe #impl_spec {
type FfiType = *const ::std::os::raw::c_void;
type ReturnType = Self::FfiType;
Expand Down
12 changes: 7 additions & 5 deletions uniffi_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,24 +216,26 @@ pub fn ffi_converter_error(attrs: TokenStream, input: TokenStream) -> TokenStrea
.into()
}

/// Generate the FfiConverter implementation for an Interface
/// Generate various support elements, including the FfiConverter implementation,
/// for an Interface
///
/// This is used by the Askama scaffolding code. It this inputs an struct/enum definition, but
/// only outputs the `FfiConverter` implementation, not the item.
#[doc(hidden)]
#[proc_macro_attribute]
pub fn ffi_converter_interface(attrs: TokenStream, input: TokenStream) -> TokenStream {
object::expand_ffi_converter_interface(
pub fn expand_interface_support(attrs: TokenStream, input: TokenStream) -> TokenStream {
object::expand_interface_support(
syn::parse_macro_input!(attrs),
syn::parse_macro_input!(input),
)
.into()
}

/// Generate the FfiConverter implementation for an trait interface for the scaffolding code
/// Generate various support elements, including the FfiConverter implementation,
/// for a trait interface for the scaffolding code
#[doc(hidden)]
#[proc_macro]
pub fn scaffolding_ffi_converter_trait_interface(tokens: TokenStream) -> TokenStream {
pub fn expand_trait_interface_support(tokens: TokenStream) -> TokenStream {
export::ffi_converter_trait_impl(
&syn::parse_macro_input!(tokens),
Some(&syn::parse_quote!(crate::UniFfiTag)),
Expand Down
8 changes: 7 additions & 1 deletion uniffi_macros/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub fn expand_object(input: DeriveInput, module_path: String) -> TokenStream {
}
}

pub(crate) fn expand_ffi_converter_interface(attr: CommonAttr, input: DeriveInput) -> TokenStream {
pub(crate) fn expand_interface_support(attr: CommonAttr, input: DeriveInput) -> TokenStream {
interface_impl(&input.ident, attr.tag.as_ref())
}

Expand All @@ -57,6 +57,12 @@ pub(crate) fn interface_impl(ident: &Ident, tag: Option<&Path>) -> TokenStream {
};

quote! {
// All Object structs must be `Sync + Send`. The generated scaffolding will fail to compile
// if they are not, but unfortunately it fails with an unactionably obscure error message.
// By asserting the requirement explicitly, we help Rust produce a more scrutable error message
// and thus help the user debug why the requirement isn't being met.
uniffi::deps::static_assertions::assert_impl_all!(#ident: Sync, Send);

#[doc(hidden)]
#[automatically_derived]
/// Support for passing reference-counted shared objects via the FFI.
Expand Down

0 comments on commit 4fc37dc

Please sign in to comment.