Update comments

Author: vladbat00

crates/bevy_ecs/src/query/access.rs

@@ -403,6 +403,11 @@ impl<T: SparseSetIndex> Default for AccessFilters<T> {
 
 impl<T: SparseSetIndex> AccessFilters<T> {
     fn is_ruled_out_by(&self, other: &Self) -> bool {
+        // Although not technically complete, we don't consider the case when `AccessFilters`'s
+        // `without` bitset contradicts its own `with` bitset (e.g. `(With<A>, Without<A>)`).
+        // Such query would be considered compatible with any other query, but as it's almost
+        // always an error, we ignore this case instead of treating such query as compatible
+        // with others.
         !self.with.is_disjoint(&other.without) || !self.without.is_disjoint(&other.with)
     }
 }

crates/bevy_ecs/src/query/fetch.rs

@@ -1509,34 +1509,21 @@ macro_rules! impl_anytuple_fetch {
             fn update_component_access(state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {
                 let ($($name,)*) = state;
 
-                // We do not unconditionally add `$name`'s `with`/`without` accesses to `_access`
-                // as this would be unsound. For example the following two queries should conflict:
-                // - Query<(AnyOf<(&A, ())>, &mut B)>
-                // - Query<&mut B, Without<A>>
-                //
-                // If we were to unconditionally add `$name`'s `with`/`without` accesses then `AnyOf<(&A, ())>`
-                // would have a `With<A>` access which is incorrect as this `WorldQuery` will match entities that
-                // do not have the `A` component. This is the same logic as the `Or<...>: WorldQuery` impl.
-                //
-                // The correct thing to do here is to only add a `with`/`without` access to `_access` if all
-                // `$name` params have that `with`/`without` access. More jargony put- we add the intersection
-                // of all `with`/`without` accesses of the `$name` params to `_access`.
-                let mut _intersected_access = _access.clone();
+                let mut _new_access = access.clone();
                 let mut _not_first = false;
                 $(
                     if _not_first {
                         let mut intermediate = _access.clone();
                         $name::update_component_access($name, &mut intermediate);
-                        _intersected_access.append_or(&intermediate);
-                        _intersected_access.extend_access(&intermediate);
+                        _new_access.append_or(&intermediate);
+                        _new_access.extend_access(&intermediate);
                     } else {
-
-                        $name::update_component_access($name, &mut _intersected_access);
+                        $name::update_component_access($name, &mut _new_access);
                         _not_first = true;
                     }
                 )*
 
-                *_access = _intersected_access;
+                *access = _new_access;
             }
 
             fn update_archetype_component_access(state: &Self::State, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {

crates/bevy_ecs/src/query/filter.rs

@@ -348,33 +348,21 @@ macro_rules! impl_query_filter_tuple {
             fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
                 let ($($filter,)*) = state;
 
-                // We do not unconditionally add `$filter`'s `with`/`without` accesses to `access`
-                // as this would be unsound. For example the following two queries should conflict:
-                // - Query<&mut B, Or<(With<A>, ())>>
-                // - Query<&mut B, Without<A>>
-                //
-                // If we were to unconditionally add `$name`'s `with`/`without` accesses then `Or<(With<A>, ())>`
-                // would have a `With<A>` access which is incorrect as this `WorldQuery` will match entities that
-                // do not have the `A` component. This is the same logic as the `AnyOf<...>: WorldQuery` impl.
-                //
-                // The correct thing to do here is to only add a `with`/`without` access to `_access` if all
-                // `$filter` params have that `with`/`without` access. More jargony put- we add the intersection
-                // of all `with`/`without` accesses of the `$filter` params to `access`.
-                let mut _intersected_access = access.clone();
+                let mut _new_access = access.clone();
                 let mut _not_first = false;
                 $(
                     if _not_first {
                         let mut intermediate = access.clone();
                         $filter::update_component_access($filter, &mut intermediate);
-                        _intersected_access.append_or(&intermediate);
-                        _intersected_access.extend_access(&intermediate);
+                        _new_access.append_or(&intermediate);
+                        _new_access.extend_access(&intermediate);
                     } else {
-                        $filter::update_component_access($filter, &mut _intersected_access);
+                        $filter::update_component_access($filter, &mut _new_access);
                         _not_first = true;
                     }
                 )*
 
-                *access = _intersected_access;
+                *access = _new_access;
             }
 
             fn update_archetype_component_access(state: &Self::State, archetype: &Archetype, access: &mut Access<ArchetypeComponentId>) {