From e45f984c83984b45f86ea7a5d5bac6d5aea5f483 Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Thu, 3 Feb 2022 01:04:47 +1100 Subject: [PATCH] Revise README.md for accuracy and flow --- README.md | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index a0db1b1..798f925 100644 --- a/README.md +++ b/README.md @@ -57,27 +57,19 @@ anymap = { version = "1.0.0-beta.1", default-features = false, features = ["hash The `std` feature is enabled by default. The `hashbrown` feature overrides it. At least one of the two must be enabled. -**On stability:** hashbrown is still pre-1.0.0 and experiencing breaking changes. Because it’s useful for a small fraction of users, I am retaining it, but with *different compatibility guarantees to the typical SemVer ones*. Where possible, I will just widen the range for new releases of hashbrown (e.g. change `0.12` to `>=0.12, <0.14` when they release 0.13.0), but if an incompatible change occurs, I may drop support for older versions of hashbrown with a bump to the *minor* part of the anymap version number (e.g. 1.1.0, 1.2.0). Iff you’re using this feature, this is cause to *consider* using a tilde requirement like `"~1.0"` (or spell it out as `>=1, <1.1`). +**On stability:** hashbrown is still pre-1.0.0 and experiencing breaking changes. Because it’s useful for a small fraction of users, I am retaining it, but with *different compatibility guarantees to the typical SemVer ones*. Where possible, I will just widen the range for new releases of hashbrown, but if an incompatible change occurs, I may drop support for older versions of hashbrown with a bump to the *minor* part of the anymap version number (e.g. 1.1.0, 1.2.0). Iff you’re using this feature, this is cause to *consider* using a tilde requirement like `"~1.0"` (or spell it out as `>=1, <1.1`). ## Unsafe code in this library This library uses a fair bit of unsafe code for several reasons: -- To support `CloneAny`, unsafe code is required (because the downcast methods are defined on `dyn Any` rather than being trait methods); if you wanted to ditch `Clone` support this unsafety could be removed. +- To support `CloneAny`, unsafe code is currently required (because the downcast methods are defined on `dyn Any` rather than being trait methods, and upcasting is an incomplete feature in rustc); 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). +- For `dyn CloneAny + Send` and `dyn CloneAny + Send + Sync`’s `Clone` implementation, an unsafe block is used to attach the auto traits 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). -- 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). +- In the interests of performance, type ID checks are skipped as unnecessary because of the invariants of the data structure (though this does come at the cost of `Map::{as_raw_mut, into_raw}` being marked unsafe). -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 (choose `dyn Any`, `dyn Any + Send` or `dyn Any + Send + Sync` and stick with it); -- 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). - -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`). +It is possible to remove all unsafe code at the cost of only `CloneAny` functionality and a little performance. The `safe` branch in the Git repository contains a couple of commits demonstrating the concept. It’s quite straightforward; the core of this library is very simple and perfectly safe. ## Colophon -- 2.42.0