Make TypeIdHasher safe, bump MSRV
authorChris Morgan <me@chrismorgan.info>
committerChris Morgan <me@chrismorgan.info>
Wait a few years and nice things stabilise!

• u64::from_ne_bytes([u8; 8]) is stable in 1.32.0
• TryFrom<&[u8]> for [u8; 8] is stable in 1.34.0

(There are other things I’m touching today that also require a more mild
MSRV bump, but this is the most I *need* at this time.)
.travis.yml
CHANGELOG.md
Cargo.toml
README.md
src/raw.rs

index 0b0690253f4705b3f9404fb40544cadc3a15a4c8..d6e378bb8c2d619ad0041f088cee4961e5e700f8 100644 (file)
@@ -1,7 +1,7 @@
 language: rust
 rust:
+  - 1.34.0
   - nightly
-  - beta
   - stable
 script:
   - if [[ "$(rustc --version)" =~ -(dev|nightly) ]]; then cargo bench; fi
index 72f127020926f3e816388048681452d5fb4a3e1a..04ff219208e06f46ac0d873d6326813ac0bc7e82 100644 (file)
@@ -2,6 +2,8 @@
 
 - Relicensed from MIT/Apache-2.0 to BlueOak-1.0.0/MIT/Apache-2.0.
 
+- Increased the minimum supported version of Rust from 1.7.0 to 1.34.0.
+
 - Remove `bench` Cargo feature (by shifting benchmarks out of `src/lib.rs` into
   `benches/bench.rs`; it still won’t run on anything but nightly, but that
   don’t signify). Technically a [breaking-change], but it was something for
index b9ce2e1082024a950cb47d9ba78c1fa01ea893c1..be51bde1de9026e373677513d29e8a6e278e8e75 100644 (file)
@@ -2,6 +2,7 @@
 name = "anymap"
 version = "0.12.1"
 authors = ["Chris Morgan <rust@chrismorgan.info>"]
+rust-version = "1.34"
 description = "A safe and convenient store for one value of each type"
 repository = "https://github.com/chris-morgan/anymap"
 keywords = ["container", "any", "map"]
index 9c2905c0df02d09a656d448263eb3422f8d51468..91471de2c988f7dddca14bbf536b74a6af8d1610 100644 (file)
--- a/README.md
+++ b/README.md
@@ -16,18 +16,17 @@ This library uses a fair bit of unsafe code for several reasons:
 
 - To support Any and CloneAny, unsafe code is required (because of how the `downcast` methods are defined in `impl dyn Any` rather than being trait methods; I think this is kind of a historical detail of the structure of `std::any::Any`); if you wanted to ditch `Clone` support this unsafety could be removed.
 
-- 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 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).
 
 - 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:
+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 and the raw interface, you can definitely remove all unsafe code. Here’s how you could do it:
 
 - Remove the genericness of it all;
 - Merge `anymap::raw` into the normal interface, flattening it;
-- Change things like `.map(|any| unsafe { any.downcast_unchecked() })` to `.and_then(|any| any.downcast())` (performance cost: one extra superfluous type ID comparison, indirect);
-- Ditch the `TypeIdHasher` since transmuting a `TypeId` is right out (cost: SIP mangling of a u64 on every access).
+- Change things like `.map(|any| unsafe { any.downcast_unchecked() })` to `.and_then(|any| any.downcast())` (performance cost: one extra superfluous type ID comparison, indirect).
 
-Yeah, the performance costs of going safe are quite small. The more serious matters are the loss of `Clone` and maybe `Send + Sync`.
+Yeah, the performance costs of going safe are quite small. The more serious matter is the loss of `Clone`.
 
 But frankly, if you wanted to do all this it’d be easier and faster to write it from scratch. The core of the library is actually really simple and perfectly safe, as can be seen in [`src/lib.rs` in the first commit](https://github.com/chris-morgan/anymap/tree/a294948f57dee47bb284d6a3ae1b8f61a902a03c/src/lib.rs) (note that that code won’t run without a few syntactic alterations; it was from well before Rust 1.0 and has things like `Any:'static` where now we have `Any + 'static`).
 
index aa3d6dc7dd83b1d3e159339c5abd5c2abdee86d2..0094b3412726fc99517f70868677e39b59f3d5cc 100644 (file)
@@ -5,12 +5,12 @@
 use std::any::TypeId;
 use std::borrow::Borrow;
 use std::collections::hash_map::{self, HashMap};
+use std::convert::TryInto;
 use std::hash::Hash;
 use std::hash::{Hasher, BuildHasherDefault};
 #[cfg(test)]
 use std::mem;
 use std::ops::{Index, IndexMut};
-use std::ptr;
 
 use any::{Any, UncheckedAnyExt};
 
@@ -22,11 +22,13 @@ struct TypeIdHasher {
 impl Hasher for TypeIdHasher {
     #[inline]
     fn write(&mut self, bytes: &[u8]) {
-        // This expects to receive one and exactly one 64-bit value
-        debug_assert!(bytes.len() == 8);
-        unsafe {
-            ptr::copy_nonoverlapping(&bytes[0] as *const u8 as *const u64, &mut self.value, 1)
-        }
+        // This expects to receive exactly one 64-bit value, and there’s no realistic chance of
+        // that changing, but I don’t want to depend on something that isn’t expressly part of the
+        // contract for safety. But I’m OK with release builds putting everything in one bucket
+        // if it *did* change (and debug builds panicking).
+        debug_assert_eq!(bytes.len(), 8);
+        let _ = bytes.try_into()
+            .map(|array| self.value = u64::from_ne_bytes(array));
     }
 
     #[inline]