Skip to content

Commit

Permalink
Add/fix track_caller attribute on panicking entity accessor methods (
Browse files Browse the repository at this point in the history
…#8951)

# Objective

`World::entity`, `World::entity_mut` and `Commands::entity` should be
marked with `track_caller` to display where (in user code) the call with
the invalid `Entity` was made. `Commands::entity` already has the
attibute, but it does nothing due to the call to `unwrap_or_else`.

## Solution

- Apply the `track_caller` attribute to the `World::entity_mut` and
`World::entity`.
- Remove the call to `unwrap_or_else` which makes the `track_caller`
attribute useless (because `unwrap_or_else` is not `track_caller`
itself). The avoid eager evaluation of the panicking branch it is never
inlined.

---------

Co-authored-by: Giacomo Stevanato <giaco.stevanato@gmail.com>
  • Loading branch information
MrGunflame and SkiFire13 authored Jun 26, 2023
1 parent 1e73312 commit 15be0d1
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
14 changes: 11 additions & 3 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,19 @@ impl<'w, 's> Commands<'w, 's> {
#[inline]
#[track_caller]
pub fn entity<'a>(&'a mut self, entity: Entity) -> EntityCommands<'w, 's, 'a> {
self.get_entity(entity).unwrap_or_else(|| {
#[inline(never)]
#[cold]
#[track_caller]
fn panic_no_entity(entity: Entity) -> ! {
panic!(
"Attempting to create an EntityCommands for entity {entity:?}, which doesn't exist.",
)
})
);
}

match self.get_entity(entity) {
Some(entity) => entity,
None => panic_no_entity(entity),
}
}

/// Returns the [`EntityCommands`] for the requested [`Entity`], if it exists.
Expand Down
30 changes: 24 additions & 6 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,19 @@ impl World {
/// assert_eq!(position.x, 0.0);
/// ```
#[inline]
#[track_caller]
pub fn entity(&self, entity: Entity) -> EntityRef {
// Lazily evaluate panic!() via unwrap_or_else() to avoid allocation unless failure
self.get_entity(entity)
.unwrap_or_else(|| panic!("Entity {entity:?} does not exist"))
#[inline(never)]
#[cold]
#[track_caller]
fn panic_no_entity(entity: Entity) -> ! {
panic!("Entity {entity:?} does not exist");
}

match self.get_entity(entity) {
Some(entity) => entity,
None => panic_no_entity(entity),
}
}

/// Retrieves an [`EntityMut`] that exposes read and write operations for the given `entity`.
Expand All @@ -267,10 +276,19 @@ impl World {
/// position.x = 1.0;
/// ```
#[inline]
#[track_caller]
pub fn entity_mut(&mut self, entity: Entity) -> EntityMut {
// Lazily evaluate panic!() via unwrap_or_else() to avoid allocation unless failure
self.get_entity_mut(entity)
.unwrap_or_else(|| panic!("Entity {entity:?} does not exist"))
#[inline(never)]
#[cold]
#[track_caller]
fn panic_no_entity(entity: Entity) -> ! {
panic!("Entity {entity:?} does not exist");
}

match self.get_entity_mut(entity) {
Some(entity) => entity,
None => panic_no_entity(entity),
}
}

/// Returns the components of an [`Entity`](crate::entity::Entity) through [`ComponentInfo`](crate::component::ComponentInfo).
Expand Down

0 comments on commit 15be0d1

Please # to comment.