Rename UncheckedAnyExt, fix Extend, tweak things

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

@@ -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

src/any.rs

@@ -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);

src/lib.rs

@@ -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());
+    }
 }