Rename UncheckedAnyExt, fix Extend, tweak things
authorChris Morgan <me@chrismorgan.info>
committerChris Morgan <me@chrismorgan.info>
The *name* UncheckedAnyExt was ending up visible in docs, but it had
become an increasingly unpleasant name. “Downcast” is suitable, though,
being private, it’s not still not perfect. But there’s no point in
making it public, as people generally can’t implement it because of
coherence rules (I tried).

Plus documentation and style changes.

As for Extend, eh, that should ideally be in a different commit, but
it’s here now, and I’m the only one working on this code base in
general, so I permit myself to be slightly lazy from time to time.
Trouble was Downcast should never have had an Any supertrait, as it was
grossly misleading, leading to type_id giving `dyn Any`’s TypeId rather
than the underlying type’s.
CHANGELOG.md
src/any.rs
src/lib.rs

index b91e2bfc53ebaf974cf2b2f3c6aebe6ad44f80a0..8dd437eee0e7ac2c298a7708c5ea86bdc32a3066 100644 (file)
@@ -3,6 +3,10 @@
 Planned once the dust of 1.0.0-beta.1 settles, since 1.0.0-beta.1 ended up
 being bigger than I’d earlier intended.
 
+# 1.0.0-beta.2 (unreleased)
+
+- Fixed the broken `Extend` implementation added in 1.0.0-beta.1.
+
 # 1.0.0-beta.1 (2022-01-25)
 
 - Removed `anymap::any::Any` in favour of just plain `core::any::Any`, since its
index 15b535b343433fb5ce83cc97e15d2f9838406833..636cb922bc83252310f288a57aad0f18458d695a 100644 (file)
@@ -1,5 +1,5 @@
 use core::fmt;
-use core::any::Any;
+use core::any::{Any, TypeId};
 #[cfg(not(feature = "std"))]
 use alloc::boxed::Box;
 
@@ -47,23 +47,61 @@ macro_rules! impl_clone {
     }
 }
 
-#[allow(missing_docs)]  // Bogus warning (it’s not public outside the crate), ☹
-pub trait UncheckedAnyExt: Any {
-    unsafe fn downcast_ref_unchecked<T: Any>(&self) -> &T;
-    unsafe fn downcast_mut_unchecked<T: Any>(&mut self) -> &mut T;
-    unsafe fn downcast_unchecked<T: Any>(self: Box<Self>) -> Box<T>;
+/// Methods for downcasting from an `Any`-like trait object.
+///
+/// This should only be implemented on trait objects for subtraits of `Any`, though you can
+/// implement it for other types and it’ll work fine, so long as your implementation is correct.
+pub trait Downcast {
+    /// Gets the `TypeId` of `self`.
+    fn type_id(&self) -> TypeId;
+
+    // Note the bound through these downcast methods is 'static, rather than the inexpressible
+    // concept of Self-but-as-a-trait (where Self is `dyn Trait`). This is sufficient, exceeding
+    // TypeId’s requirements. Sure, you *can* do CloneAny.downcast_unchecked::<NotClone>() and the
+    // type system won’t protect you, but that doesn’t introduce any unsafety: the method is
+    // already unsafe because you can specify the wrong type, and if this were exposing safe
+    // downcasting, CloneAny.downcast::<NotClone>() would just return an error, which is just as
+    // correct.
+    //
+    // Now in theory we could also add T: ?Sized, but that doesn’t play nicely with the common
+    // implementation, so I’m doing without it.
+
+    /// Downcast from `&Any` to `&T`, without checking the type matches.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `T` matches the trait object, on pain of *undefined behaviour*.
+    unsafe fn downcast_ref_unchecked<T: 'static>(&self) -> &T;
+
+    /// Downcast from `&mut Any` to `&mut T`, without checking the type matches.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `T` matches the trait object, on pain of *undefined behaviour*.
+    unsafe fn downcast_mut_unchecked<T: 'static>(&mut self) -> &mut T;
+
+    /// Downcast from `Box<Any>` to `Box<T>`, without checking the type matches.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `T` matches the trait object, on pain of *undefined behaviour*.
+    unsafe fn downcast_unchecked<T: 'static>(self: Box<Self>) -> Box<T>;
 }
 
-#[doc(hidden)]
 /// A trait for the conversion of an object into a boxed trait object.
-pub trait IntoBox<A: ?Sized + UncheckedAnyExt>: Any {
+pub trait IntoBox<A: ?Sized + Downcast>: Any {
     /// Convert self into the appropriate boxed form.
     fn into_box(self) -> Box<A>;
 }
 
 macro_rules! implement {
-    ($base:ident, $(+ $bounds:ident)*) => {
-        impl UncheckedAnyExt for dyn $base $(+ $bounds)* {
+    ($any_trait:ident $(+ $auto_traits:ident)*) => {
+        impl Downcast for dyn $any_trait $(+ $auto_traits)* {
+            #[inline]
+            fn type_id(&self) -> TypeId {
+                self.type_id()
+            }
+
             #[inline]
             unsafe fn downcast_ref_unchecked<T: 'static>(&self) -> &T {
                 &*(self as *const Self as *const T)
@@ -80,18 +118,18 @@ macro_rules! implement {
             }
         }
 
-        impl<T: $base $(+ $bounds)*> IntoBox<dyn $base $(+ $bounds)*> for T {
+        impl<T: $any_trait $(+ $auto_traits)*> IntoBox<dyn $any_trait $(+ $auto_traits)*> for T {
             #[inline]
-            fn into_box(self) -> Box<dyn $base $(+ $bounds)*> {
+            fn into_box(self) -> Box<dyn $any_trait $(+ $auto_traits)*> {
                 Box::new(self)
             }
         }
     }
 }
 
-implement!(Any,);
-implement!(Any, + Send);
-implement!(Any, + Send + Sync);
+implement!(Any);
+implement!(Any + Send);
+implement!(Any + Send + Sync);
 
 /// [`Any`], but with cloning.
 ///
@@ -99,9 +137,9 @@ implement!(Any, + Send + Sync);
 /// See [`core::any`] for more details on `Any` in general.
 pub trait CloneAny: Any + CloneToAny { }
 impl<T: Any + Clone> CloneAny for T { }
-implement!(CloneAny,);
-implement!(CloneAny, + Send);
-implement!(CloneAny, + Send + Sync);
+implement!(CloneAny);
+implement!(CloneAny + Send);
+implement!(CloneAny + Send + Sync);
 impl_clone!(dyn CloneAny);
 impl_clone!(dyn CloneAny + Send);
 impl_clone!(dyn CloneAny + Send + Sync);
index 17427011dfb8ffce8a868c5551a852299011a9ae..0fc29419faff02434400c3879125f12c00486e28 100644 (file)
@@ -20,7 +20,7 @@ extern crate alloc;
 #[cfg(not(feature = "std"))]
 use alloc::boxed::Box;
 
-use any::{UncheckedAnyExt, IntoBox};
+use any::{Downcast, IntoBox};
 pub use any::CloneAny;
 
 #[cfg(all(feature = "std", not(feature = "hashbrown")))]
@@ -108,12 +108,12 @@ pub type RawMap<A> = HashMap<TypeId, Box<A>, BuildHasherDefault<TypeIdHasher>>;
 ///
 /// Values containing non-static references are not permitted.
 #[derive(Debug)]
-pub struct Map<A: ?Sized + UncheckedAnyExt = dyn Any> {
+pub struct Map<A: ?Sized + Downcast = dyn Any> {
     raw: RawMap<A>,
 }
 
 // #[derive(Clone)] would want A to implement Clone, but in reality it’s only Box<A> that can.
-impl<A: ?Sized + UncheckedAnyExt> Clone for Map<A> where Box<A>: Clone {
+impl<A: ?Sized + Downcast> Clone for Map<A> where Box<A>: Clone {
     #[inline]
     fn clone(&self) -> Map<A> {
         Map {
@@ -129,14 +129,14 @@ impl<A: ?Sized + UncheckedAnyExt> Clone for Map<A> where Box<A>: Clone {
 /// It’s a bit sad, really. Ah well, I guess this approach will do.
 pub type AnyMap = Map<dyn Any>;
 
-impl<A: ?Sized + UncheckedAnyExt> Default for Map<A> {
+impl<A: ?Sized + Downcast> Default for Map<A> {
     #[inline]
     fn default() -> Map<A> {
         Map::new()
     }
 }
 
-impl<A: ?Sized + UncheckedAnyExt> Map<A> {
+impl<A: ?Sized + Downcast> Map<A> {
     /// Create an empty collection.
     #[inline]
     pub fn new() -> Map<A> {
@@ -336,17 +336,17 @@ impl<A: ?Sized + UncheckedAnyExt> Map<A> {
     }
 }
 
-impl<A: ?Sized + UncheckedAnyExt> Extend<Box<A>> for Map<A> {
+impl<A: ?Sized + Downcast> Extend<Box<A>> for Map<A> {
     #[inline]
     fn extend<T: IntoIterator<Item = Box<A>>>(&mut self, iter: T) {
         for item in iter {
-            let _ = self.raw.insert(item.type_id(), item);
+            let _ = self.raw.insert(Downcast::type_id(&*item), item);
         }
     }
 }
 
 /// A view into a single occupied location in an `Map`.
-pub struct OccupiedEntry<'a, A: ?Sized + UncheckedAnyExt, V: 'a> {
+pub struct OccupiedEntry<'a, A: ?Sized + Downcast, V: 'a> {
     #[cfg(all(feature = "std", not(feature = "hashbrown")))]
     inner: raw_hash_map::OccupiedEntry<'a, TypeId, Box<A>>,
     #[cfg(feature = "hashbrown")]
@@ -355,7 +355,7 @@ pub struct OccupiedEntry<'a, A: ?Sized + UncheckedAnyExt, V: 'a> {
 }
 
 /// A view into a single empty location in an `Map`.
-pub struct VacantEntry<'a, A: ?Sized + UncheckedAnyExt, V: 'a> {
+pub struct VacantEntry<'a, A: ?Sized + Downcast, V: 'a> {
     #[cfg(all(feature = "std", not(feature = "hashbrown")))]
     inner: raw_hash_map::VacantEntry<'a, TypeId, Box<A>>,
     #[cfg(feature = "hashbrown")]
@@ -364,14 +364,14 @@ pub struct VacantEntry<'a, A: ?Sized + UncheckedAnyExt, V: 'a> {
 }
 
 /// A view into a single location in an `Map`, which may be vacant or occupied.
-pub enum Entry<'a, A: ?Sized + UncheckedAnyExt, V: 'a> {
+pub enum Entry<'a, A: ?Sized + Downcast, V: 'a> {
     /// An occupied Entry
     Occupied(OccupiedEntry<'a, A, V>),
     /// A vacant Entry
     Vacant(VacantEntry<'a, A, V>),
 }
 
-impl<'a, A: ?Sized + UncheckedAnyExt, V: IntoBox<A>> Entry<'a, A, V> {
+impl<'a, A: ?Sized + Downcast, V: IntoBox<A>> Entry<'a, A, V> {
     /// Ensures a value is in the entry by inserting the default if empty, and returns
     /// a mutable reference to the value in the entry.
     #[inline]
@@ -421,7 +421,7 @@ impl<'a, A: ?Sized + UncheckedAnyExt, V: IntoBox<A>> Entry<'a, A, V> {
     // insert_entry(self, value: V) -> OccupiedEntry<'a, K, V>                             (1.59.0)
 }
 
-impl<'a, A: ?Sized + UncheckedAnyExt, V: IntoBox<A>> OccupiedEntry<'a, A, V> {
+impl<'a, A: ?Sized + Downcast, V: IntoBox<A>> OccupiedEntry<'a, A, V> {
     /// Gets a reference to the value in the entry
     #[inline]
     pub fn get(&self) -> &V {
@@ -454,7 +454,7 @@ impl<'a, A: ?Sized + UncheckedAnyExt, V: IntoBox<A>> OccupiedEntry<'a, A, V> {
     }
 }
 
-impl<'a, A: ?Sized + UncheckedAnyExt, V: IntoBox<A>> VacantEntry<'a, A, V> {
+impl<'a, A: ?Sized + Downcast, V: IntoBox<A>> VacantEntry<'a, A, V> {
     /// Sets the value of the entry with the VacantEntry's key,
     /// and returns a mutable reference to it
     #[inline]
@@ -645,4 +645,13 @@ mod tests {
         verify_hashing_with(TypeId::of::<&str>());
         verify_hashing_with(TypeId::of::<Vec<u8>>());
     }
+
+    #[test]
+    fn test_extend() {
+        let mut map = AnyMap::new();
+        map.extend([Box::new(123) as Box<dyn Any>, Box::new(456), Box::new(true)]);
+        assert_eq!(map.get(), Some(&456));
+        assert_eq!(map.get::<bool>(), Some(&true));
+        assert!(map.get::<Box<dyn Any>>().is_none());
+    }
 }