Skip to content

Draft: Remove Send and Sync bounds from BufferedReader

KarelPeeters requested to merge flaghacker/sequoia:remove-sync into main

See #1057 for previous discussion. I'm not sure where best to continue this, I'll stay here for now.

Never mind my previous comment about assert_send_and_sync, it looks like I didn't notice the macro also adds extra Send + Sync bounds to the generic parameters. It does make sense after all!

I was indeed proposing to push the Send + Sync bounds to users, including equoia-openpgp. Sadly it seems that Box<dyn BufferedReader + Send + Sync doesn't work yet (see https://github.com/rust-lang/rfcs/issues/2035), so indeed this would require introducing a new collection trait like this:

/// A version of the [BufferedReader] trait that is also [Send] and [Sync].
/// This trait is automatically implemented for types that fulfill the requirements.
///
/// This trait mainly exists as a workaround for https://github.com/rust-lang/rfcs/issues/2035,
/// so `Box<dyn BufferedReaderSync<C>>` can be used instead of `Box<dyn BufferedReader<C> + Send + Sync>`
pub trait BufferedReaderSync<C: fmt::Debug>: BufferedReader<C> + Send + Sync {}

impl<C: fmt::Debug, T: BufferedReader<C> + Send + Sync> BufferedReaderSync<C> for T {}

I've pushed my current WIP changes in [PR], although that doesn't yet change over all users. This seems to mostly be a mechanical operation, replacing dyn BufferedReader<C> with BufferedReaderSync<C> in about 120 places. I can continue doing this if you this is an acceptable solution.

An alternative is to keep BufferedReader as Send + Sync by default, and introduce a new subtrait that doesn't require those bounds. This would still be a breaking change, but now implementors would have to change, instead of users. I'm not sure which is better if any.

Merge request reports