Skip to content

We may need a Send/Sync wrapper instead of NonNull<T> #16

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

Closed
flier opened this issue Dec 1, 2021 · 3 comments
Closed

We may need a Send/Sync wrapper instead of NonNull<T> #16

flier opened this issue Dec 1, 2021 · 3 comments

Comments

@flier
Copy link
Contributor

flier commented Dec 1, 2021

As you known, when we declare a foreign type that implements Send and Sync

foreign_type! {
    /// A Foo.
    pub unsafe type Foo
        : Sync + Send // optional
    {
        type CType = foo_sys::FOO;
        fn drop = foo_sys::FOO_free;
        fn clone = foo_sys::FOO_duplicate; // optional
    }
}

It will generate a type that wrap the NonNull<T>.

pub struct FooRef(Opaque);

unsafe impl ForeignTypeRef for FooRef {
    type CType = foo_sys::FOO;
}

pub struct Foo(NonNull<foo_sys::FOO>);

unsafe impl Sync for FooRef {}
unsafe impl Send for FooRef {}

unsafe impl Sync for Foo {}
unsafe impl Send for Foo {}

Then the nightly clippy will give a non_send_fields_in_send_ty warning, because NonNull<T> is !Send and !Sync.

   |
13 | / foreign_type! {
14 | |     /// A compiled pattern database that can then be used to scan data.
15 | |     pub unsafe type Database<T>: Send + Sync {
16 | |         type CType = ffi::hs_database_t;
...  |
20 | |     }
21 | | }
   | |_^
   |
   = note: `#[warn(clippy::non_send_fields_in_send_ty)]` on by default
note: the type of field `0` is `!Send`

We need a Send/Sync wrapper instead of NonNull<T>, maybe change to use UnsafeCell<NonNull<T>>?

flier added a commit to flier/rust-hyperscan that referenced this issue Dec 1, 2021
flier added a commit to flier/rust-hyperscan that referenced this issue Dec 1, 2021
@sfackler
Copy link
Owner

sfackler commented Dec 1, 2021

That seems like a poorly designed lint, since it will by definition trigger any time someone unsafely implements Sync or Send: rust-lang/rust-clippy#8045.

@flier
Copy link
Contributor Author

flier commented Dec 2, 2021

That seems like a poorly designed lint, since it will by definition trigger any time someone unsafely implements Sync or Send:
rust-lang/rust-clippy#8045

I agree, use unsafe code carries with it a corresponding risk.

@avitex
Copy link
Contributor

avitex commented Feb 7, 2022

The lint was moved to the nursery in 1.58.1

@sfackler sfackler closed this as completed Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants