From: Chris Morgan Date: Tue, 25 Jan 2022 02:48:23 +0000 (+1100) Subject: Refactor to avoid a spurious compatibility warning X-Git-Tag: 1.0.0-beta.1~20 X-Git-Url: https://git.chrismorgan.info/anymap/commitdiff_plain/521fbfe6bcbf60e7abc8d8ab33bf73349fc4b38d Refactor to avoid a spurious compatibility warning Explained in the SAFETY comment. I’m not happy about *doing* this, but it will make *using* this crate easier, since future-compatibility lints make noise on bin crate builds, so this was polluting other people’s code and making life harder for users. I have traded one evil (a spurious warning) for another (unsafe code). --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 04b57ee..72f1270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ - Implement `Default` on `Map` (not just on `RawMap`) +- Worked around the spurious `where_clauses_object_safety` future-compatibility lint that has been raised since mid-2018. + If you put `#![allow(where_clauses_object_safety)]` on your binary crates for this reason, you can remove it. + I don’t plan for there to be any real changes from 0.12.1; it should be just a bit of housecleaning and a version bump. diff --git a/README.md b/README.md index c4bbe6b..9c2905c 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,8 @@ This library uses a fair bit of unsafe code for several reasons: - In the interests of performance, skipping various checks that are unnecessary because of the invariants of the data structure (no need to check the type ID when it’s been statically ensured by being used as the hash map key) and simplifying hashing (type IDs are already good hashes, no need to mangle them through SipHash). +- In the `Clone` implementation of `dyn CloneAny` with `Send` and/or `Sync` auto traits added, an unsafe block is used where safe code used to be used in order to avoid a [spurious future-compatibility lint](https://github.com/rust-lang/rust/issues/51443#issuecomment-421988013). + It’s not possible to remove all unsafety from this library without also removing some of the functionality. Still, at the cost of the `CloneAny` functionality, the raw interface and maybe the concurrency support, you can definitely remove all unsafe code. Here’s how you could do it: - Remove the genericness of it all; diff --git a/src/any.rs b/src/any.rs index e9cef36..8404cd4 100644 --- a/src/any.rs +++ b/src/any.rs @@ -10,15 +10,6 @@ use std::any::Any as StdAny; pub trait CloneToAny { /// Clone `self` into a new `Box` object. fn clone_to_any(&self) -> Box; - - /// Clone `self` into a new `Box` object. - fn clone_to_any_send(&self) -> Box where Self: Send; - - /// Clone `self` into a new `Box` object. - fn clone_to_any_sync(&self) -> Box where Self: Sync; - - /// Clone `self` into a new `Box` object. - fn clone_to_any_send_sync(&self) -> Box where Self: Send + Sync; } impl CloneToAny for T { @@ -26,29 +17,27 @@ impl CloneToAny for T { fn clone_to_any(&self) -> Box { Box::new(self.clone()) } - - #[inline] - fn clone_to_any_send(&self) -> Box where Self: Send { - Box::new(self.clone()) - } - - #[inline] - fn clone_to_any_sync(&self) -> Box where Self: Sync { - Box::new(self.clone()) - } - - #[inline] - fn clone_to_any_send_sync(&self) -> Box where Self: Send + Sync { - Box::new(self.clone()) - } } macro_rules! impl_clone { - ($t:ty, $method:ident) => { + ($t:ty) => { impl Clone for Box<$t> { #[inline] fn clone(&self) -> Box<$t> { - (**self).$method() + // SAFETY: this dance is to reapply any Send/Sync marker. I’m not happy about this + // approach, given that I used to do it in safe code, but then came a dodgy + // future-compatibility warning where_clauses_object_safety, which is spurious for + // auto traits but still super annoying (future-compatibility lints seem to mean + // your bin crate needs a corresponding allow!). Although I explained my plight¹ + // and it was all explained and agreed upon, no action has been taken. So I finally + // caved and worked around it by doing it this way, which matches what’s done for + // std::any², so it’s probably not *too* bad. + // + // ¹ https://github.com/rust-lang/rust/issues/51443#issuecomment-421988013 + // ² https://github.com/rust-lang/rust/blob/e7825f2b690c9a0d21b6f6d84c404bb53b151b38/library/alloc/src/boxed.rs#L1613-L1616 + let clone: Box = (**self).clone_to_any(); + let raw: *mut dyn CloneAny = Box::into_raw(clone); + unsafe { Box::from_raw(raw as *mut $t) } } } } @@ -144,7 +133,7 @@ implement!(CloneAny,); implement!(CloneAny, + Send); implement!(CloneAny, + Sync); implement!(CloneAny, + Send + Sync); -impl_clone!(dyn CloneAny, clone_to_any); -impl_clone!(dyn CloneAny + Send, clone_to_any_send); -impl_clone!(dyn CloneAny + Sync, clone_to_any_sync); -impl_clone!(dyn CloneAny + Send + Sync, clone_to_any_send_sync); +impl_clone!(dyn CloneAny); +impl_clone!(dyn CloneAny + Send); +impl_clone!(dyn CloneAny + Sync); +impl_clone!(dyn CloneAny + Send + Sync);