Mitigate stack overflow on large Bundle inserts

crates/bevy_ecs/compile_fail/tests/ui/bundle_on_drop_impl.rs

@@ -0,0 +1,17 @@
+use bevy_ecs::prelude::*;
+
+#[derive(Component, Debug)]
+pub struct A(usize);
+
+// this should fail since destructuring T: Drop cannot be split.
+#[derive(Bundle, Debug)]
+//~^ E0509
+pub struct DropBundle {
+    component_a: A,
+}
+
+impl Drop for DropBundle {
+    fn drop(&mut self) {
+        // Just need the impl
+    }
+}

crates/bevy_ecs/macros/src/lib.rs

@@ -115,20 +115,23 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
 
     let mut active_field_types = Vec::new();
     let mut active_field_tokens = Vec::new();
+    let mut active_field_alias: Vec<proc_macro2::TokenStream> = Vec::new();
     let mut inactive_field_tokens = Vec::new();
     for (((i, field_type), field_kind), field) in field_type
         .iter()
         .enumerate()
         .zip(field_kind.iter())
         .zip(field.iter())
     {
+        let field_alias = format_ident!("field_{}", i).to_token_stream();
         let field_tokens = match field {
             Some(field) => field.to_token_stream(),
             None => Index::from(i).to_token_stream(),
         };
         match field_kind {
             BundleFieldKind::Component => {
                 active_field_types.push(field_type);
+                active_field_alias.push(field_alias);
                 active_field_tokens.push(field_tokens);
             }
 
@@ -163,16 +166,46 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
     };
 
     let dynamic_bundle_impl = quote! {
-        #[allow(deprecated)]
+        #[doc(hidden)]
+        #[expect(dead_code, reason = "This is a static assertion.")]
+        impl #impl_generics #struct_name #ty_generics #where_clause {
+            // Types that implement `Drop` cannot have their fields moved out. The implementation in
+            // `get_componments` avoids this with pointers, so there needs to be a static assertion
+            // that this is a sound thing to do. See https://doc.rust-lang.org/error_codes/E0509.html
+            // for more information.
+            fn check_no_bundle_drop(self) {
+                // This has no effect, but we need to make sure the compiler doesn't optimize it out
+                // black_box is used to do this
+                #( core::hint::black_box(self.#active_field_tokens); )*
+            }
+        }
+
         impl #impl_generics #ecs_path::bundle::DynamicBundle for #struct_name #ty_generics #where_clause {
             type Effect = ();
             #[allow(unused_variables)]
             #[inline]
-            fn get_components(
-                self,
+            unsafe fn get_components(
+                ptr: #ecs_path::ptr::MovingPtr<'_, Self>,
                 func: &mut impl FnMut(#ecs_path::component::StorageType, #ecs_path::ptr::OwningPtr<'_>)
             ) {
-                #(<#active_field_types as #ecs_path::bundle::DynamicBundle>::get_components(self.#active_field_tokens, &mut *func);)*
+                use #ecs_path::__macro_exports::DebugCheckedUnwrap;
+
+                #( let #active_field_alias = ptr.move_field(|ptr| &raw mut (*ptr).#active_field_tokens); )*
+                core::mem::forget(ptr);
+                #(
+                    <#active_field_types as #ecs_path::bundle::DynamicBundle>::get_components(
+                        #active_field_alias.try_into().debug_checked_unwrap(),
+                        func
+                    );
+                )*
+            }
+
+            #[allow(unused_variables)]
+            #[inline]
+            unsafe fn apply_effect(
+                ptr: #ecs_path::ptr::MovingPtr<'_, core::mem::MaybeUninit<Self>>,
+                func: &mut #ecs_path::world::EntityWorldMut<'_>,
+            ) {
             }
         }
     };

crates/bevy_ecs/src/bundle/impls.rs

@@ -1,11 +1,13 @@
 use core::any::TypeId;
 
-use bevy_ptr::OwningPtr;
-use variadics_please::all_tuples;
+use bevy_ptr::{MovingPtr, OwningPtr};
+use core::mem::MaybeUninit;
+use variadics_please::all_tuples_enumerated;
 
 use crate::{
-    bundle::{Bundle, BundleEffect, BundleFromComponents, DynamicBundle, NoBundleEffect},
+    bundle::{Bundle, BundleFromComponents, DynamicBundle, NoBundleEffect},
     component::{Component, ComponentId, Components, ComponentsRegistrator, StorageType},
+    query::DebugCheckedUnwrap,
     world::EntityWorldMut,
 };
 
@@ -40,13 +42,19 @@ unsafe impl<C: Component> BundleFromComponents for C {
 impl<C: Component> DynamicBundle for C {
     type Effect = ();
     #[inline]
-    fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect {
-        OwningPtr::make(self, |ptr| func(C::STORAGE_TYPE, ptr));
+    unsafe fn get_components(
+        ptr: MovingPtr<'_, Self>,
+        func: &mut impl FnMut(StorageType, OwningPtr<'_>),
+    ) -> Self::Effect {
+        func(C::STORAGE_TYPE, OwningPtr::from(ptr));
     }
+
+    #[inline]
+    unsafe fn apply_effect(_ptr: MovingPtr<'_, MaybeUninit<Self>>, _entity: &mut EntityWorldMut) {}
 }
 
 macro_rules! tuple_impl {
-    ($(#[$meta:meta])* $($name: ident),*) => {
+    ($(#[$meta:meta])* $(($index:tt, $name: ident, $alias: ident)),*) => {
         #[expect(
             clippy::allow_attributes,
             reason = "This is a tuple-related macro; as such, the lints below may not always apply."
@@ -125,59 +133,52 @@ macro_rules! tuple_impl {
                 reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case."
             )]
             #[inline(always)]
-            fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect {
-                #[allow(
-                    non_snake_case,
-                    reason = "The names of these variables are provided by the caller, not by us."
-                )]
-                let ($(mut $name,)*) = self;
-                ($(
-                    $name.get_components(&mut *func),
-                )*)
+            unsafe fn get_components(ptr: MovingPtr<'_, Self>, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) {
+                // SAFETY:
+                // - All of the `move_field` calls all fetch distinct and valid fields within `Self`.
+                // - If a field is `NoBundleEffect`, it's `apply_effect` is a no-op
+                //   and cannot move any value out of an invalid instance after this call.
+                // - If a field is `!NoBundleEffect`, it must be valid since a safe
+                //   implementation of `DynamicBundle` only moves the value out only
+                //   once between `get_components` and `apply_effect`.
+                bevy_ptr::deconstruct_moving_ptr!(ptr => ($($index => $alias,)*));
+                // SAFETY:
+                // - If `ptr` is aligned, then field_ptr is aligned properly. Rust tuples cannot be `repr(packed)`.
+                $( $name::get_components($alias.try_into().debug_checked_unwrap(), func); )*
             }
-        }
-    }
-}
-
-all_tuples!(
-    #[doc(fake_variadic)]
-    tuple_impl,
-    0,
-    15,
-    B
-);
 
-macro_rules! after_effect_impl {
-    ($(#[$meta:meta])* $($after_effect: ident),*) => {
-        #[expect(
-            clippy::allow_attributes,
-            reason = "This is a tuple-related macro; as such, the lints below may not always apply."
-        )]
-        $(#[$meta])*
-        impl<$($after_effect: BundleEffect),*> BundleEffect for ($($after_effect,)*) {
             #[allow(
                 clippy::unused_unit,
-                reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case.")
-            ]
-            fn apply(self, _entity: &mut EntityWorldMut) {
-                #[allow(
-                    non_snake_case,
-                    reason = "The names of these variables are provided by the caller, not by us."
-                )]
-                let ($($after_effect,)*) = self;
-                $($after_effect.apply(_entity);)*
+                reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case."
+            )]
+            #[inline(always)]
+            unsafe fn apply_effect(ptr: MovingPtr<'_, MaybeUninit<Self>>, entity: &mut EntityWorldMut) {
+                // SAFETY:
+                // - All of the `move_field` calls all fetch distinct and valid fields within `Self`.
+                // - If a field is `NoBundleEffect`, it's `apply_effect` is a no-op
+                //   and cannot move any value out of an invalid instance.
+                // - If a field is `!NoBundleEffect`, it must be valid since a safe
+                //   implementation of `DynamicBundle` only moves the value out only
+                //   once between `get_components` and `apply_effect`.
+                bevy_ptr::deconstruct_moving_ptr!(ptr: MaybeUninit => (
+                    $($index => $alias,)*
+                ));
+                // SAFETY:
+                // - If `ptr` is aligned, then field_ptr is aligned properly. Rust tuples cannot be `repr(packed)`.
+                $( $name::apply_effect($alias.try_into().debug_checked_unwrap(), entity); )*
             }
         }
 
         $(#[$meta])*
-        impl<$($after_effect: NoBundleEffect),*> NoBundleEffect for ($($after_effect,)*) { }
+        impl<$($name: NoBundleEffect),*> NoBundleEffect for ($($name,)*) {}
     }
 }
 
-all_tuples!(
+all_tuples_enumerated!(
     #[doc(fake_variadic)]
-    after_effect_impl,
+    tuple_impl,
     0,
     15,
-    P
+    B,
+    field_
 );

crates/bevy_ecs/src/bundle/info.rs

@@ -3,7 +3,7 @@ use bevy_platform::{
     collections::{HashMap, HashSet},
     hash::FixedHasher,
 };
-use bevy_ptr::OwningPtr;
+use bevy_ptr::{MovingPtr, OwningPtr};
 use bevy_utils::TypeIdMap;
 use core::{any::TypeId, ptr::NonNull};
 use indexmap::{IndexMap, IndexSet};
@@ -228,8 +228,14 @@ impl BundleInfo {
     /// which removes the need to look up the [`ArchetypeAfterBundleInsert`](crate::archetype::ArchetypeAfterBundleInsert)
     /// in the archetype graph, which requires ownership of the entity's current archetype.
     ///
+    /// Regardless of how this is used, [`apply_effect`] must be called at most once on `bundle` after this function is
+    /// called if `T::Effect: !NoBundleEffect` before returning to user-space safe code before returning to user-space safe code.
+    /// This is currently only doable via use of [`MovingPtr::partial_move`].
+    ///
     /// `table` must be the "new" table for `entity`. `table_row` must have space allocated for the
     /// `entity`, `bundle` must match this [`BundleInfo`]'s type
+    ///
+    /// [`apply_effect`]: crate::bundle::DynamicBundle::apply_effect
     #[inline]
     pub(super) unsafe fn write_components<'a, T: DynamicBundle, S: BundleComponentStatus>(
         &self,
@@ -240,14 +246,14 @@ impl BundleInfo {
         entity: Entity,
         table_row: TableRow,
         change_tick: Tick,
-        bundle: T,
+        bundle: MovingPtr<'_, T>,
         insert_mode: InsertMode,
         caller: MaybeLocation,
-    ) -> T::Effect {
+    ) {
         // NOTE: get_components calls this closure on each component in "bundle order".
         // bundle_info.component_ids are also in "bundle order"
         let mut bundle_component = 0;
-        let after_effect = bundle.get_components(&mut |storage_type, component_ptr| {
+        T::get_components(bundle, &mut |storage_type, component_ptr| {
             let component_id = *self
                 .contributed_component_ids
                 .get_unchecked(bundle_component);
@@ -303,8 +309,6 @@ impl BundleInfo {
                 caller,
             );
         }
-
-        after_effect
     }
 
     /// Internal method to initialize a required component from an [`OwningPtr`]. This should ultimately be called

crates/bevy_ecs/src/bundle/insert.rs

@@ -1,5 +1,5 @@
 use alloc::vec::Vec;
-use bevy_ptr::ConstNonNull;
+use bevy_ptr::{ConstNonNull, MovingPtr};
 use core::ptr::NonNull;
 
 use crate::{
@@ -138,18 +138,25 @@ impl<'w> BundleInserter<'w> {
     }
 
     /// # Safety
-    /// `entity` must currently exist in the source archetype for this inserter. `location`
-    /// must be `entity`'s location in the archetype. `T` must match this [`BundleInfo`]'s type
+    /// - `entity` must currently exist in the source archetype for this inserter.
+    /// - `location` must be `entity`'s location in the archetype.
+    /// - `T` must match this [`BundleInserter`] type used to create
+    /// - If `T::Effect: !NoBundleEffect.`, then [`apply_effect`] must be called at most once on
+    ///   `bundle` after this function before returning to user-space safe code.
+    /// - The value pointed to by `bundle` must not be accessed for anything other than [`apply_effect`]
+    ///   or dropped.
+    ///
+    /// [`apply_effect`]: crate::bundle::DynamicBundle::apply_effect
     #[inline]
     pub(crate) unsafe fn insert<T: DynamicBundle>(
         &mut self,
         entity: Entity,
         location: EntityLocation,
-        bundle: T,
+        bundle: MovingPtr<'_, T>,
         insert_mode: InsertMode,
         caller: MaybeLocation,
         relationship_hook_mode: RelationshipHookMode,
-    ) -> (EntityLocation, T::Effect) {
+    ) -> EntityLocation {
         let bundle_info = self.bundle_info.as_ref();
         let archetype_after_insert = self.archetype_after_insert.as_ref();
         let archetype = self.archetype.as_ref();
@@ -188,15 +195,15 @@ impl<'w> BundleInserter<'w> {
         // so this reference can only be promoted from shared to &mut down here, after they have been ran
         let archetype = self.archetype.as_mut();
 
-        let (new_archetype, new_location, after_effect) = match &mut self.archetype_move_type {
+        let (new_archetype, new_location) = match &mut self.archetype_move_type {
             ArchetypeMoveType::SameArchetype => {
                 // SAFETY: Mutable references do not alias and will be dropped after this block
                 let sparse_sets = {
                     let world = self.world.world_mut();
                     &mut world.storages.sparse_sets
                 };
 
-                let after_effect = bundle_info.write_components(
+                bundle_info.write_components(
                     table,
                     sparse_sets,
                     archetype_after_insert,
@@ -209,7 +216,7 @@ impl<'w> BundleInserter<'w> {
                     caller,
                 );
 
-                (archetype, location, after_effect)
+                (archetype, location)
             }
             ArchetypeMoveType::NewArchetypeSameTable { new_archetype } => {
                 let new_archetype = new_archetype.as_mut();
@@ -237,7 +244,7 @@ impl<'w> BundleInserter<'w> {
                 }
                 let new_location = new_archetype.allocate(entity, result.table_row);
                 entities.set(entity.index(), Some(new_location));
-                let after_effect = bundle_info.write_components(
+                bundle_info.write_components(
                     table,
                     sparse_sets,
                     archetype_after_insert,
@@ -250,7 +257,7 @@ impl<'w> BundleInserter<'w> {
                     caller,
                 );
 
-                (new_archetype, new_location, after_effect)
+                (new_archetype, new_location)
             }
             ArchetypeMoveType::NewArchetypeNewTable {
                 new_archetype,
@@ -319,7 +326,7 @@ impl<'w> BundleInserter<'w> {
                     }
                 }
 
-                let after_effect = bundle_info.write_components(
+                bundle_info.write_components(
                     new_table,
                     sparse_sets,
                     archetype_after_insert,
@@ -332,7 +339,7 @@ impl<'w> BundleInserter<'w> {
                     caller,
                 );
 
-                (new_archetype, new_location, after_effect)
+                (new_archetype, new_location)
             }
         };
 
@@ -411,7 +418,7 @@ impl<'w> BundleInserter<'w> {
             }
         }
 
-        (new_location, after_effect)
+        new_location
     }
 
     #[inline]