bevy_reflect: Introduce reflect_clone_and_take. (#19944)

# Objective

There is a pattern that appears in multiple places, involving
`reflect_clone`, followed by `take`, followed by `map_err` that produces
a `FailedDowncast` in a particular form.

## Solution

Introduces `reflect_clone_and_take`, which factors out the repeated
code.

## Testing

`cargo run -p ci`

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

crates/bevy_reflect/compile_fail/tests/reflect_remote/invalid_definition_fail.rs

@@ -9,6 +9,7 @@ mod structs {
 
     #[reflect_remote(external_crate::TheirStruct)]
     //~^ ERROR: `?` operator has incompatible types
+    //~| ERROR: mismatched types
     struct MyStruct {
         // Reason: Should be `u32`
         pub value: bool,
@@ -25,6 +26,7 @@ mod tuple_structs {
 
     #[reflect_remote(external_crate::TheirStruct)]
     //~^ ERROR: `?` operator has incompatible types
+    //~| ERROR: mismatched types
     struct MyStruct(
         // Reason: Should be `u32`
         pub bool,
@@ -48,6 +50,7 @@ mod enums {
     //~| ERROR: variant `enums::external_crate::TheirStruct::Unit` has no field named `0`
     //~| ERROR: `?` operator has incompatible types
     //~| ERROR: `?` operator has incompatible types
+    //~| ERROR: mismatched types
     enum MyStruct {
         // Reason: Should be unit variant
         Unit(i32),
@@ -57,6 +60,7 @@ mod enums {
         // Reason: Should be `usize`
         Struct { value: String },
         //~^ ERROR: mismatched types
+        //~| ERROR: mismatched types
     }
 }
 

crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs

@@ -26,8 +26,8 @@ mod incorrect_inner_type {
     //~| ERROR: `TheirInner<T>` does not implement `PartialReflect` so cannot be introspected
     //~| ERROR: `TheirInner<T>` does not implement `PartialReflect` so cannot be introspected
     //~| ERROR: `TheirInner<T>` does not implement `TypePath` so cannot provide dynamic type path information
-    //~| ERROR: `TheirInner<T>` does not implement `TypePath` so cannot provide dynamic type path information
     //~| ERROR: `?` operator has incompatible types
+    //~| ERROR: mismatched types
     struct MyOuter<T: FromReflect + GetTypeRegistration> {
         // Reason: Should not use `MyInner<T>` directly
         pub inner: MyInner<T>,

crates/bevy_reflect/compile_fail/tests/reflect_remote/type_mismatch_fail.rs

@@ -77,6 +77,7 @@ mod enums {
 
     #[reflect_remote(external_crate::TheirBar)]
     //~^ ERROR: `?` operator has incompatible types
+    //~| ERROR: mismatched types
     enum MyBar {
         // Reason: Should use `i32`
         Value(u32),

crates/bevy_reflect/derive/src/derive_data.rs

@@ -722,18 +722,7 @@ impl<'a> ReflectStruct<'a> {
                         }
                     } else {
                         quote! {
-                            #bevy_reflect_path::PartialReflect::reflect_clone(#accessor)?
+                            <#field_ty as #bevy_reflect_path::PartialReflect>::reflect_clone_and_take(#accessor)?
-                                .take()
-                                .map_err(|value| #bevy_reflect_path::ReflectCloneError::FailedDowncast {
-                                    expected: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(
-                                        <#field_ty as #bevy_reflect_path::TypePath>::type_path()
-                                    ),
-                                    received: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Owned(
-                                        #bevy_reflect_path::__macro_exports::alloc_utils::ToString::to_string(
-                                            #bevy_reflect_path::DynamicTypePath::reflect_type_path(&*value)
-                                        )
-                                    ),
-                                })?
                         }
                     };
 

crates/bevy_reflect/derive/src/enum_utility.rs

@@ -316,9 +316,7 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> {
 
     fn construct_field(&self, field: VariantField) -> TokenStream {
         let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path();
-
         let field_ty = field.field.reflected_type();
-
         let alias = field.alias;
         let alias = match &field.field.attrs.remote {
             Some(wrapper_ty) => {
@@ -332,18 +330,7 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> {
         match &field.field.attrs.clone {
             CloneBehavior::Default => {
                 quote! {
-                    #bevy_reflect_path::PartialReflect::reflect_clone(#alias)?
+                    <#field_ty as #bevy_reflect_path::PartialReflect>::reflect_clone_and_take(#alias)?
-                        .take()
-                        .map_err(|value| #bevy_reflect_path::ReflectCloneError::FailedDowncast {
-                            expected: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(
-                                <#field_ty as #bevy_reflect_path::TypePath>::type_path()
-                            ),
-                            received: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Owned(
-                                #bevy_reflect_path::__macro_exports::alloc_utils::ToString::to_string(
-                                    #bevy_reflect_path::DynamicTypePath::reflect_type_path(&*value)
-                                )
-                            ),
-                        })?
                 }
             }
             CloneBehavior::Trait => {

crates/bevy_reflect/src/impls/alloc/collections/btree/map.rs

@@ -9,7 +9,6 @@ use crate::{
     type_registry::{FromType, GetTypeRegistration, ReflectFromPtr, TypeRegistration},
     utility::GenericTypeInfoCell,
 };
-use alloc::borrow::Cow;
 use alloc::vec::Vec;
 use bevy_platform::prelude::*;
 use bevy_reflect_derive::impl_type_path;
@@ -144,21 +143,8 @@ where
     fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
         let mut map = Self::new();
         for (key, value) in self.iter() {
-            let key =
+            let key = key.reflect_clone_and_take()?;
-                key.reflect_clone()?
+            let value = value.reflect_clone_and_take()?;
-                    .take()
-                    .map_err(|_| ReflectCloneError::FailedDowncast {
-                        expected: Cow::Borrowed(<Self as TypePath>::type_path()),
-                        received: Cow::Owned(key.reflect_type_path().to_string()),
-                    })?;
-            let value =
-                value
-                    .reflect_clone()?
-                    .take()
-                    .map_err(|_| ReflectCloneError::FailedDowncast {
-                        expected: Cow::Borrowed(<Self as TypePath>::type_path()),
-                        received: Cow::Owned(value.reflect_type_path().to_string()),
-                    })?;
             map.insert(key, value);
         }
 

crates/bevy_reflect/src/impls/macros/list.rs

@@ -113,14 +113,7 @@ macro_rules! impl_reflect_for_veclike {
                 fn reflect_clone(&self) -> Result<bevy_platform::prelude::Box<dyn $crate::reflect::Reflect>, $crate::error::ReflectCloneError> {
                     Ok(bevy_platform::prelude::Box::new(
                         self.iter()
-                            .map(|value| {
+                            .map(|value| value.reflect_clone_and_take())
-                                value.reflect_clone()?.take().map_err(|_| {
-                                    $crate::error::ReflectCloneError::FailedDowncast {
-                                        expected: alloc::borrow::Cow::Borrowed(<T as $crate::type_path::TypePath>::type_path()),
-                                        received: alloc::borrow::Cow::Owned(alloc::string::ToString::to_string(value.reflect_type_path())),
-                                    }
-                                })
-                            })
                             .collect::<Result<Self, $crate::error::ReflectCloneError>>()?,
                     ))
                 }

crates/bevy_reflect/src/impls/macros/map.rs

@@ -146,18 +146,8 @@ macro_rules! impl_reflect_for_hashmap {
                 fn reflect_clone(&self) -> Result<bevy_platform::prelude::Box<dyn $crate::reflect::Reflect>, $crate::error::ReflectCloneError> {
                     let mut map = Self::with_capacity_and_hasher(self.len(), S::default());
                     for (key, value) in self.iter() {
-                        let key = key.reflect_clone()?.take().map_err(|_| {
+                        let key = key.reflect_clone_and_take()?;
-                            $crate::error::ReflectCloneError::FailedDowncast {
+                        let value = value.reflect_clone_and_take()?;
-                                expected: alloc::borrow::Cow::Borrowed(<K as $crate::type_path::TypePath>::type_path()),
-                                received: alloc::borrow::Cow::Owned(alloc::string::ToString::to_string(key.reflect_type_path())),
-                            }
-                        })?;
-                        let value = value.reflect_clone()?.take().map_err(|_| {
-                            $crate::error::ReflectCloneError::FailedDowncast {
-                                expected: alloc::borrow::Cow::Borrowed(<V as $crate::type_path::TypePath>::type_path()),
-                                received: alloc::borrow::Cow::Owned(alloc::string::ToString::to_string(value.reflect_type_path())),
-                            }
-                        })?;
                         map.insert(key, value);
                     }
 

crates/bevy_reflect/src/impls/macros/set.rs

@@ -129,12 +129,7 @@ macro_rules! impl_reflect_for_hashset {
                 fn reflect_clone(&self) -> Result<bevy_platform::prelude::Box<dyn $crate::reflect::Reflect>, $crate::error::ReflectCloneError> {
                     let mut set = Self::with_capacity_and_hasher(self.len(), S::default());
                     for value in self.iter() {
-                        let value = value.reflect_clone()?.take().map_err(|_| {
+                        let value = value.reflect_clone_and_take()?;
-                            $crate::error::ReflectCloneError::FailedDowncast {
-                                expected: alloc::borrow::Cow::Borrowed(<V as $crate::type_path::TypePath>::type_path()),
-                                received: alloc::borrow::Cow::Owned(alloc::string::ToString::to_string(value.reflect_type_path())),
-                            }
-                        })?;
                         set.insert(value);
                     }
 

crates/bevy_reflect/src/impls/smallvec.rs

@@ -4,7 +4,7 @@ use crate::{
     ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypeParamInfo, TypePath, TypeRegistration,
     Typed,
 };
-use alloc::{borrow::Cow, boxed::Box, string::ToString, vec::Vec};
+use alloc::{boxed::Box, vec::Vec};
 use bevy_reflect::ReflectCloneError;
 use bevy_reflect_derive::impl_type_path;
 use core::any::Any;
@@ -137,16 +137,11 @@ where
 
     fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
         Ok(Box::new(
-            self.iter()
+            // `(**self)` avoids getting `SmallVec<T> as List::iter`, which
-                .map(|value| {
+            // would give us the wrong item type.
-                    value
+            (**self)
-                        .reflect_clone()?
+                .iter()
-                        .take()
+                .map(PartialReflect::reflect_clone_and_take)
-                        .map_err(|_| ReflectCloneError::FailedDowncast {
-                            expected: Cow::Borrowed(<T::Item as TypePath>::type_path()),
-                            received: Cow::Owned(value.reflect_type_path().to_string()),
-                        })
-                })
                 .collect::<Result<Self, ReflectCloneError>>()?,
         ))
     }

crates/bevy_reflect/src/reflect.rs

@@ -313,6 +313,24 @@ where
         })
     }
 
+    /// For a type implementing [`PartialReflect`], combines `reflect_clone` and
+    /// `take` in a useful fashion, automatically constructing an appropriate
+    /// [`ReflectCloneError`] if the downcast fails.
+    ///
+    /// This is an associated function, rather than a method, because methods
+    /// with generic types prevent dyn-compatibility.
+    fn reflect_clone_and_take<T: 'static>(&self) -> Result<T, ReflectCloneError>
+    where
+        Self: TypePath + Sized,
+    {
+        self.reflect_clone()?
+            .take()
+            .map_err(|_| ReflectCloneError::FailedDowncast {
+                expected: Cow::Borrowed(<Self as TypePath>::type_path()),
+                received: Cow::Owned(self.reflect_type_path().to_string()),
+            })
+    }
+
     /// Returns a hash of the value (which includes the type).
     ///
     /// If the underlying type does not support hashing, returns `None`.