Encapsulate cfg(feature = "track_location") in a type. (#17602)

# Objective

Eliminate the need to write `cfg(feature = "track_location")` every time
one uses an API that may use location tracking. It's verbose, and a
little intimidating. And it requires code outside of `bevy_ecs` that
wants to use location tracking needs to either unconditionally enable
the feature, or include conditional compilation of its own. It would be
good for users to be able to log locations when they are available
without needing to add feature flags to their own crates.

Reduce the number of cases where code compiles with the `track_location`
feature enabled, but not with it disabled, or vice versa. It can be hard
to remember to test it both ways!

Remove the need to store a `None` in `HookContext` when the
`track_location` feature is disabled.

## Solution

Create an `MaybeLocation<T>` type that contains a `T` if the
`track_location` feature is enabled, and is a ZST if it is not. The
overall API is similar to `Option`, but whether the value is `Some` or
`None` is set at compile time and is the same for all values.

Default `T` to `&'static Location<'static>`, since that is the most
common case.

Remove all `cfg(feature = "track_location")` blocks outside of the
implementation of that type, and instead call methods on it.

When `track_location` is disabled, `MaybeLocation` is a ZST and all
methods are `#[inline]` and empty, so they should be entirely removed by
the compiler. But the code will still be visible to the compiler and
checked, so if it compiles with the feature disabled then it should also
compile with it enabled, and vice versa.

## Open Questions

Where should these types live? I put them in `change_detection` because
that's where the existing `MaybeLocation` types were, but we now use
these outside of change detection.

While I believe that the compiler should be able to remove all of these
calls, I have not actually tested anything. If we want to take this
approach, what testing is required to ensure it doesn't impact
performance?

## Migration Guide

Methods like `Ref::changed_by()` that return a `&'static
Location<'static>` will now be available even when the `track_location`
feature is disabled, but they will return a new `MaybeLocation` type.
`MaybeLocation` wraps a `&'static Location<'static>` when the feature is
enabled, and is a ZST when the feature is disabled.

Existing code that needs a `&Location` can call `into_option().unwrap()`
to recover it. Many trait impls are forwarded, so if you only need
`Display` then no changes will be necessary.

If that code was conditionally compiled, you may instead want to use the
methods on `MaybeLocation` to remove the need for conditional
compilation.

Code that constructs a `Ref`, `Mut`, `Res`, or `ResMut` will now need to
provide location information unconditionally. If you are creating them
from existing Bevy types, you can obtain a `MaybeLocation` from methods
like `Table::get_changed_by_slice_for()` or
`ComponentSparseSet::get_with_ticks`. Otherwise, you will need to store
a `MaybeLocation` next to your data and use methods like `as_ref()` or
`as_mut()` to obtain wrapped references.

Author: chescock

crates/bevy_ecs/src/bundle.rs

@@ -9,6 +9,7 @@ use crate::{
         Archetype, ArchetypeAfterBundleInsert, ArchetypeId, Archetypes, BundleComponentStatus,
         ComponentStatus, SpawnBundleStatus,
     },
+    change_detection::MaybeLocation,
     component::{
         Component, ComponentId, Components, RequiredComponentConstructor, RequiredComponents,
         StorageType, Tick,
@@ -24,8 +25,6 @@ use alloc::{boxed::Box, vec, vec::Vec};
 use bevy_platform_support::collections::{HashMap, HashSet};
 use bevy_ptr::{ConstNonNull, OwningPtr};
 use bevy_utils::TypeIdMap;
-#[cfg(feature = "track_location")]
-use core::panic::Location;
 use core::{any::TypeId, ptr::NonNull};
 use variadics_please::all_tuples;
 
@@ -623,7 +622,7 @@ impl BundleInfo {
         change_tick: Tick,
         bundle: T,
         insert_mode: InsertMode,
-        #[cfg(feature = "track_location")] caller: &'static Location<'static>,
+        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"
@@ -638,20 +637,12 @@ impl BundleInfo {
                     // the target table contains the component.
                     let column = table.get_column_mut(component_id).debug_checked_unwrap();
                     match (status, insert_mode) {
-                        (ComponentStatus::Added, _) => column.initialize(
-                            table_row,
-                            component_ptr,
-                            change_tick,
-                            #[cfg(feature = "track_location")]
-                            caller,
-                        ),
-                        (ComponentStatus::Existing, InsertMode::Replace) => column.replace(
-                            table_row,
-                            component_ptr,
-                            change_tick,
-                            #[cfg(feature = "track_location")]
-                            caller,
-                        ),
+                        (ComponentStatus::Added, _) => {
+                            column.initialize(table_row, component_ptr, change_tick, caller);
+                        }
+                        (ComponentStatus::Existing, InsertMode::Replace) => {
+                            column.replace(table_row, component_ptr, change_tick, caller);
+                        }
                         (ComponentStatus::Existing, InsertMode::Keep) => {
                             if let Some(drop_fn) = table.get_drop_for(component_id) {
                                 drop_fn(component_ptr);
@@ -664,13 +655,7 @@ impl BundleInfo {
                         // SAFETY: If component_id is in self.component_ids, BundleInfo::new ensures that
                         // a sparse set exists for the component.
                         unsafe { sparse_sets.get_mut(component_id).debug_checked_unwrap() };
-                    sparse_set.insert(
-                        entity,
-                        component_ptr,
-                        change_tick,
-                        #[cfg(feature = "track_location")]
-                        caller,
-                    );
+                    sparse_set.insert(entity, component_ptr, change_tick, caller);
                 }
             }
             bundle_component += 1;
@@ -683,7 +668,6 @@ impl BundleInfo {
                 change_tick,
                 table_row,
                 entity,
-                #[cfg(feature = "track_location")]
                 caller,
             );
         }
@@ -712,7 +696,7 @@ impl BundleInfo {
         component_id: ComponentId,
         storage_type: StorageType,
         component_ptr: OwningPtr,
-        #[cfg(feature = "track_location")] caller: &'static Location<'static>,
+        caller: MaybeLocation,
     ) {
         {
             match storage_type {
@@ -721,26 +705,14 @@ impl BundleInfo {
                         // SAFETY: If component_id is in required_components, BundleInfo::new requires that
                         // the target table contains the component.
                         unsafe { table.get_column_mut(component_id).debug_checked_unwrap() };
-                    column.initialize(
-                        table_row,
-                        component_ptr,
-                        change_tick,
-                        #[cfg(feature = "track_location")]
-                        caller,
-                    );
+                    column.initialize(table_row, component_ptr, change_tick, caller);
                 }
                 StorageType::SparseSet => {
                     let sparse_set =
                         // SAFETY: If component_id is in required_components, BundleInfo::new requires that
                         // a sparse set exists for the component.
                         unsafe { sparse_sets.get_mut(component_id).debug_checked_unwrap() };
-                    sparse_set.insert(
-                        entity,
-                        component_ptr,
-                        change_tick,
-                        #[cfg(feature = "track_location")]
-                        caller,
-                    );
+                    sparse_set.insert(entity, component_ptr, change_tick, caller);
                 }
             }
         }
@@ -1127,7 +1099,7 @@ impl<'w> BundleInserter<'w> {
         location: EntityLocation,
         bundle: T,
         insert_mode: InsertMode,
-        #[cfg(feature = "track_location")] caller: &'static Location<'static>,
+        caller: MaybeLocation,
     ) -> (EntityLocation, T::Effect) {
         let bundle_info = self.bundle_info.as_ref();
         let archetype_after_insert = self.archetype_after_insert.as_ref();
@@ -1145,15 +1117,13 @@ impl<'w> BundleInserter<'w> {
                         ON_REPLACE,
                         entity,
                         archetype_after_insert.iter_existing(),
-                        #[cfg(feature = "track_location")]
                         caller,
                     );
                 }
                 deferred_world.trigger_on_replace(
                     archetype,
                     entity,
                     archetype_after_insert.iter_existing(),
-                    #[cfg(feature = "track_location")]
                     caller,
                 );
             }
@@ -1183,7 +1153,6 @@ impl<'w> BundleInserter<'w> {
                     self.change_tick,
                     bundle,
                     insert_mode,
-                    #[cfg(feature = "track_location")]
                     caller,
                 );
 
@@ -1225,7 +1194,6 @@ impl<'w> BundleInserter<'w> {
                     self.change_tick,
                     bundle,
                     insert_mode,
-                    #[cfg(feature = "track_location")]
                     caller,
                 );
 
@@ -1308,7 +1276,6 @@ impl<'w> BundleInserter<'w> {
                     self.change_tick,
                     bundle,
                     insert_mode,
-                    #[cfg(feature = "track_location")]
                     caller,
                 );
 
@@ -1327,15 +1294,13 @@ impl<'w> BundleInserter<'w> {
                 new_archetype,
                 entity,
                 archetype_after_insert.iter_added(),
-                #[cfg(feature = "track_location")]
                 caller,
             );
             if new_archetype.has_add_observer() {
                 deferred_world.trigger_observers(
                     ON_ADD,
                     entity,
                     archetype_after_insert.iter_added(),
-                    #[cfg(feature = "track_location")]
                     caller,
                 );
             }
@@ -1346,15 +1311,13 @@ impl<'w> BundleInserter<'w> {
                         new_archetype,
                         entity,
                         archetype_after_insert.iter_inserted(),
-                        #[cfg(feature = "track_location")]
                         caller,
                     );
                     if new_archetype.has_insert_observer() {
                         deferred_world.trigger_observers(
                             ON_INSERT,
                             entity,
                             archetype_after_insert.iter_inserted(),
-                            #[cfg(feature = "track_location")]
                             caller,
                         );
                     }
@@ -1366,15 +1329,13 @@ impl<'w> BundleInserter<'w> {
                         new_archetype,
                         entity,
                         archetype_after_insert.iter_added(),
-                        #[cfg(feature = "track_location")]
                         caller,
                     );
                     if new_archetype.has_insert_observer() {
                         deferred_world.trigger_observers(
                             ON_INSERT,
                             entity,
                             archetype_after_insert.iter_added(),
-                            #[cfg(feature = "track_location")]
                             caller,
                         );
                     }
@@ -1456,7 +1417,7 @@ impl<'w> BundleSpawner<'w> {
         &mut self,
         entity: Entity,
         bundle: T,
-        #[cfg(feature = "track_location")] caller: &'static Location<'static>,
+        caller: MaybeLocation,
     ) -> (EntityLocation, T::Effect) {
         // SAFETY: We do not make any structural changes to the archetype graph through self.world so these pointers always remain valid
         let bundle_info = self.bundle_info.as_ref();
@@ -1481,7 +1442,6 @@ impl<'w> BundleSpawner<'w> {
                 self.change_tick,
                 bundle,
                 InsertMode::Replace,
-                #[cfg(feature = "track_location")]
                 caller,
             );
             entities.set(entity.index(), location);
@@ -1499,31 +1459,27 @@ impl<'w> BundleSpawner<'w> {
                 archetype,
                 entity,
                 bundle_info.iter_contributed_components(),
-                #[cfg(feature = "track_location")]
                 caller,
             );
             if archetype.has_add_observer() {
                 deferred_world.trigger_observers(
                     ON_ADD,
                     entity,
                     bundle_info.iter_contributed_components(),
-                    #[cfg(feature = "track_location")]
                     caller,
                 );
             }
             deferred_world.trigger_on_insert(
                 archetype,
                 entity,
                 bundle_info.iter_contributed_components(),
-                #[cfg(feature = "track_location")]
                 caller,
             );
             if archetype.has_insert_observer() {
                 deferred_world.trigger_observers(
                     ON_INSERT,
                     entity,
                     bundle_info.iter_contributed_components(),
-                    #[cfg(feature = "track_location")]
                     caller,
                 );
             }
@@ -1538,18 +1494,11 @@ impl<'w> BundleSpawner<'w> {
     pub unsafe fn spawn<T: Bundle>(
         &mut self,
         bundle: T,
-        #[cfg(feature = "track_location")] caller: &'static Location<'static>,
+        caller: MaybeLocation,
     ) -> (Entity, T::Effect) {
         let entity = self.entities().alloc();
         // SAFETY: entity is allocated (but non-existent), `T` matches this BundleInfo's type
-        let (_, after_effect) = unsafe {
-            self.spawn_non_existent(
-                entity,
-                bundle,
-                #[cfg(feature = "track_location")]
-                caller,
-            )
-        };
+        let (_, after_effect) = unsafe { self.spawn_non_existent(entity, bundle, caller) };
         (entity, after_effect)
     }