mirror of
https://github.com/bevyengine/bevy
synced 2024-12-22 19:13:08 +00:00
02688a99b8
# Objective Cloning a `WorldQuery` type's "fetch" struct was made unsafe in #5593, by adding the `unsafe fn clone_fetch` to `WorldQuery`. However, as that method's documentation explains, it is not the right place to put the safety invariant: > While calling this method on its own cannot cause UB it is marked `unsafe` as the caller must ensure that the returned value is not used in any way that would cause two `QueryItem<Self>` for the same `archetype_index` or `table_row` to be alive at the same time. You can clone a fetch struct all you want and it will never cause undefined behavior -- in order for something to go wrong, you need to improperly call `WorldQuery::fetch` with it (which is marked unsafe). Additionally, making it unsafe to clone a fetch struct does not even prevent undefined behavior, since there are other ways to incorrectly use a fetch struct. For example, you could just call fetch more than once for the same entity, which is not currently forbidden by any documented invariants. ## Solution Document a safety invariant on `WorldQuery::fetch` that requires the caller to not create aliased `WorldQueryItem`s for mutable types. Remove the `clone_fetch` function, and add the bound `Fetch: Clone` instead. --- ## Changelog - Removed the associated function `WorldQuery::clone_fetch`, and added a `Clone` bound to `WorldQuery::Fetch`. ## Migration Guide ### `fetch` invariants The function `WorldQuery::fetch` has had the following safety invariant added: > If this type does not implement `ReadOnlyWorldQuery`, then the caller must ensure that it is impossible for more than one `Self::Item` to exist for the same entity at any given time. This invariant was always required for soundness, but was previously undocumented. If you called this function manually anywhere, you should check to make sure that this invariant is not violated. ### Removed `clone_fetch` The function `WorldQuery::clone_fetch` has been removed. The associated type `WorldQuery::Fetch` now has the bound `Clone`. Before: ```rust struct MyFetch<'w> { ... } unsafe impl WorldQuery for MyQuery { ... type Fetch<'w> = MyFetch<'w> unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> { MyFetch { field1: fetch.field1, field2: fetch.field2.clone(), ... } } } ``` After: ```rust #[derive(Clone)] struct MyFetch<'w> { ... } unsafe impl WorldQuery for MyQuery { ... type Fetch<'w> = MyFetch<'w>; } ``` --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|---|---|---|
| .. | ||
| src | ||
| Cargo.toml | ||