Skip to content

Implement Send for chunk types - #78

Merged
mgeier merged 2 commits into
mgeier:mainfrom
RamType0:impl_Send_for_chunks
Mar 3, 2022
Merged

mgeier merged 2 commits into
mgeier:mainfrom
RamType0:impl_Send_for_chunks

Conversation

@RamType0

Copy link
Copy Markdown
Contributor

#4

@RamType0

Copy link
Copy Markdown
Contributor Author

This clippy error is really weird

error: this implementation is unsound, as some fields in Producer<T> are !Send
--> src/lib.rs:291:1
|
291 | unsafe impl<T: Send> Send for Producer {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: -D clippy::non-send-fields-in-send-ty implied by -D warnings
note: the type of field buffer is !Send
--> src/lib.rs:278:5
|
278 | buffer: Arc<RingBuffer>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: add bounds on type parameter T that satisfy Arc<RingBuffer<T>>: Send
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty

error: this implementation is unsound, as some fields in Consumer<T> are !Send
--> src/lib.rs:496:1
|
496 | unsafe impl<T: Send> Send for Consumer {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field buffer is !Send
--> src/lib.rs:483:5
|
483 | buffer: Arc<RingBuffer>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: add bounds on type parameter T that satisfy Arc<RingBuffer<T>>: Send
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty

error: could not compile rtrb due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: build failed
Error: Process completed with exit code 101.

rust-lang/rust-clippy#8045

@mgeier

mgeier commented Jan 18, 2022

Copy link
Copy Markdown
Owner

Yeah, that's strange.
AFAIU, this lint will be disabled in the next point release which comes out within the next few days.

Interestingly, this lint is not triggered by #75.

@mgeier

mgeier commented Jan 22, 2022 •

Copy link
Copy Markdown
Owner

Rust 1.58.1 has been released, I've restarted CI and now it passes.

Comment thread src/chunks.rs
producer: &'a Producer<T>,
}

unsafe impl<T: Send> Send for WriteChunkUninit<'_, T> {}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What about adding a comment regarding safety?

Maybe something like this:

// WriteChunkUninit only exists while a unique reference to the Producer is held.
// It is therefore safe to move it to another thread.

@mgeier mgeier left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I've added comments as suggestions, what do you think about them?

Comment thread src/chunks.rs
Comment thread src/chunks.rs
Co-authored-by: Matthias Geier <Matthias.Geier@gmail.com>
@mgeier
mgeier merged commit 7279fc1 into mgeier:main Mar 3, 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

Successfully merging this pull request may close these issues.

2 participants