Skip to content

bevy_ecs::world::entity_ref::EntityWorldMut::take(), a safe function, has an undocumented unsafe block #17345

@LikeLakers2

Description

@LikeLakers2

Whilst writing #17335, I came across an undocumented unsafe block:

#[allow(clippy::undocumented_unsafe_blocks)] // TODO: document why this is safe
unsafe {
Self::move_entity_from_remove::<false>(
entity,
&mut self.location,
old_location.archetype_id,
old_location,
entities,
archetypes,
storages,
new_archetype_id,
);
}

A safety comment should be provided. Having looked around, I found a similar call to this function, this time with a safety comment (though in an unsafe function):

// SAFETY: `new_archetype_id` is a subset of the components in `old_location.archetype_id`
// because it is created by removing a bundle from these components.
let mut new_location = location;
Self::move_entity_from_remove::<true>(
entity,
&mut new_location,
location.archetype_id,
location,
&mut world.entities,
&mut world.archetypes,
&mut world.storages,
new_archetype_id,
);

which could possibly be used as a starting point for figuring out what the safety requirements of the called function even are, because the safety documentation for the called function isn't that great:

/// # Safety
///
/// `new_archetype_id` must have the same or a subset of the components
/// in `old_archetype_id`. Probably more safety stuff too, audit a call to
/// this fn as if the code here was written inline
///
/// when DROP is true removed components will be dropped otherwise they will be forgotten
// We use a const generic here so that we are less reliant on
// inlining for rustc to optimize out the `match DROP`
unsafe fn move_entity_from_remove<const DROP: bool>(

Note: While this issue is similar to #11590, I consider it different enough to make a new issue. In particular, that issue is about undocumented unsafe blocks inside unsafe functions - while this is about an undocumented unsafe block inside a safe function.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-ECSEntities, components, systems, and eventsC-Code-QualityA section of code that is hard to understand or changeD-StraightforwardSimple bug fixes and API improvements, docs, test and examplesD-UnsafeTouches with unsafe code in some wayS-Ready-For-ImplementationThis issue is ready for an implementation PR. Go for it!

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions