Skip to content

Commit f89f7f3

Browse files
cBournhonesqueGingehBobG1983mnmaita
authored andcommitted
AnyOf soundness fix (#14013)
# Objective Fixes #13993 PR inspired by #14007 to accomplish the same thing, but maybe in a clearer fashion. @Gingeh feel free to take my changes and add them to your PR, I don't want to steal any credit --------- Co-authored-by: Gingeh <39150378+Gingeh@users.noreply.github.com> Co-authored-by: Bob Gardner <rgardner@inworld.ai> Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
1 parent 82f0156 commit f89f7f3

File tree

2 files changed

+62
-5
lines changed

2 files changed

+62
-5
lines changed

crates/bevy_ecs/src/query/fetch.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1862,24 +1862,28 @@ macro_rules! impl_anytuple_fetch {
18621862
}
18631863

18641864
fn update_component_access(state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {
1865-
let ($($name,)*) = state;
1866-
18671865
let mut _new_access = _access.clone();
1866+
1867+
// update the access (add the read/writes)
1868+
<($(Option<$name>,)*)>::update_component_access(state, _access);
1869+
1870+
// update the filters (Or<(With<$name>,)>)
1871+
let ($($name,)*) = state;
18681872
let mut _not_first = false;
18691873
$(
18701874
if _not_first {
1871-
let mut intermediate = _access.clone();
1875+
// we use an intermediate access because we only want to update the filters, not the access
1876+
let mut intermediate = FilteredAccess::default();
18721877
$name::update_component_access($name, &mut intermediate);
18731878
_new_access.append_or(&intermediate);
1874-
_new_access.extend_access(&intermediate);
18751879
} else {
18761880
$name::update_component_access($name, &mut _new_access);
18771881
_new_access.required = _access.required.clone();
18781882
_not_first = true;
18791883
}
18801884
)*
18811885

1882-
*_access = _new_access;
1886+
_access.filter_sets = _new_access.filter_sets;
18831887
}
18841888
#[allow(unused_variables)]
18851889
fn init_state(world: &mut World) -> Self::State {

crates/bevy_ecs/src/system/mod.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,51 @@ mod tests {
556556
run_system(&mut world, sys);
557557
}
558558

559+
#[test]
560+
fn any_of_working() {
561+
fn sys(_: Query<AnyOf<(&mut A, &B)>>) {}
562+
let mut world = World::default();
563+
run_system(&mut world, sys);
564+
}
565+
566+
#[test]
567+
#[should_panic = "&bevy_ecs::system::tests::A conflicts with a previous access in this query."]
568+
fn any_of_with_mut_and_ref() {
569+
fn sys(_: Query<AnyOf<(&mut A, &A)>>) {}
570+
let mut world = World::default();
571+
run_system(&mut world, sys);
572+
}
573+
574+
#[test]
575+
#[should_panic = "&mut bevy_ecs::system::tests::A conflicts with a previous access in this query."]
576+
fn any_of_with_ref_and_mut() {
577+
fn sys(_: Query<AnyOf<(&A, &mut A)>>) {}
578+
let mut world = World::default();
579+
run_system(&mut world, sys);
580+
}
581+
582+
#[test]
583+
#[should_panic = "&bevy_ecs::system::tests::A conflicts with a previous access in this query."]
584+
fn any_of_with_mut_and_option() {
585+
fn sys(_: Query<AnyOf<(&mut A, Option<&A>)>>) {}
586+
let mut world = World::default();
587+
run_system(&mut world, sys);
588+
}
589+
590+
#[test]
591+
fn any_of_with_entity_and_mut() {
592+
fn sys(_: Query<AnyOf<(Entity, &mut A)>>) {}
593+
let mut world = World::default();
594+
run_system(&mut world, sys);
595+
}
596+
597+
#[test]
598+
fn any_of_with_empty_and_mut() {
599+
fn sys(_: Query<AnyOf<((), &mut A)>>) {}
600+
let mut world = World::default();
601+
run_system(&mut world, sys);
602+
}
603+
559604
#[test]
560605
#[should_panic = "error[B0001]"]
561606
fn any_of_has_no_filter_with() {
@@ -564,6 +609,14 @@ mod tests {
564609
run_system(&mut world, sys);
565610
}
566611

612+
#[test]
613+
#[should_panic = "&mut bevy_ecs::system::tests::A conflicts with a previous access in this query."]
614+
fn any_of_with_conflicting() {
615+
fn sys(_: Query<AnyOf<(&mut A, &mut A)>>) {}
616+
let mut world = World::default();
617+
run_system(&mut world, sys);
618+
}
619+
567620
#[test]
568621
fn any_of_has_filter_with_when_both_have_it() {
569622
fn sys(_: Query<(AnyOf<(&A, &A)>, &mut B)>, _: Query<&mut B, Without<A>>) {}

0 commit comments

Comments
 (0)