From f5e887ef634cfcd5c354307ba105c616f4222c3d Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Mon, 23 Jan 2017 22:37:10 +0530 Subject: [PATCH] Add a note about unsafety. --- README.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/README.md b/README.md index 775052e..629a8e3 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,26 @@ Instructions Cargo all the way: it is `anymap` on crates.io. +Unsafe code in this library +--------------------------- + +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 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). + +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; +- 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). + +Yeah, the performance costs of going safe are quite small. The more serious matters are the loss of `Clone` and maybe `Send + Sync`. + +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`). + Author ------