forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Remove ExactSizeIterator from QueryCombinationIter (#5895)

Browse Source 
# Objective

- `QueryCombinationIter` can have sizes greater than `usize::MAX`.
- Fixes #5846 

## Solution

- Only the implementation of `ExactSizeIterator` has been removed. Instead of using `query_combination.len()`, you can use `query_combination.size_hint().0` to get the same value as before.

---

## Migration Guide

- Switch to using other methods of getting the length.
This commit is contained in:
Theo Ottah 2022-10-24 23:03:15 +00:00
parent ecb6f8fab3
commit 45e5eb1db3
4 changed files with 102 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
crates/bevy_ecs/src/query/iter.rs
View File

@ -454,21 +454,6 @@ where
}
}
impl<'w, 's, Q: ReadOnlyWorldQuery, F: ReadOnlyWorldQuery + ArchetypeFilter, const K: usize>
ExactSizeIterator for QueryCombinationIter<'w, 's, Q, F, K>
where
QueryFetch<'w, Q>: Clone,
QueryFetch<'w, F>: Clone,
{
/// Returns the exact length of the iterator.
///
/// **NOTE**: When the iterator length overflows `usize`, this will
/// return `usize::MAX`.
fn len(&self) -> usize {
self.size_hint().0
}
}
// This is correct as [`QueryCombinationIter`] always returns `None` once exhausted.
impl<'w, 's, Q: ReadOnlyWorldQuery, F: ReadOnlyWorldQuery, const K: usize> FusedIterator
for QueryCombinationIter<'w, 's, Q, F, K>

111
crates/bevy_ecs/src/query/mod.rs
View File

@ -57,6 +57,102 @@ mod tests {
#[test]
fn query_filtered_exactsizeiterator_len() {
fn assert_all_sizes_iterator_equal(
iterator: impl ExactSizeIterator,
expected_size: usize,
query_type: &'static str,
) {
let len = iterator.len();
let size_hint_0 = iterator.size_hint().0;
let size_hint_1 = iterator.size_hint().1;
// `count` tests that not only it is the expected value, but also
// the value is accurate to what the query returns.
let count = iterator.count();
// This will show up when one of the asserts in this function fails
println!(
r#"query declared sizes:
for query: {query_type}
expected: {expected_size}
len: {len}
size_hint().0: {size_hint_0}
size_hint().1: {size_hint_1:?}
count(): {count}"#
);
assert_eq!(len, expected_size);
assert_eq!(size_hint_0, expected_size);
assert_eq!(size_hint_1, Some(expected_size));
assert_eq!(count, expected_size);
}
fn assert_all_sizes_equal<Q, F>(world: &mut World, expected_size: usize)
where
Q: ReadOnlyWorldQuery,
F: ReadOnlyWorldQuery,
F::ReadOnly: ArchetypeFilter,
for<'w> QueryFetch<'w, Q::ReadOnly>: Clone,
for<'w> QueryFetch<'w, F::ReadOnly>: Clone,
{
let mut query = world.query_filtered::<Q, F>();
let iter = query.iter(world);
let query_type = type_name::<QueryState<Q, F>>();
assert_all_sizes_iterator_equal(iter, expected_size, query_type);
}
let mut world = World::new();
world.spawn((A(1), B(1)));
world.spawn((A(2),));
world.spawn((A(3),));
assert_all_sizes_equal::<&A, With<B>>(&mut world, 1);
assert_all_sizes_equal::<&A, Without<B>>(&mut world, 2);
let mut world = World::new();
world.spawn((A(1), B(1), C(1)));
world.spawn((A(2), B(2)));
world.spawn((A(3), B(3)));
world.spawn((A(4), C(4)));
world.spawn((A(5), C(5)));
world.spawn((A(6), C(6)));
world.spawn((A(7),));
world.spawn((A(8),));
world.spawn((A(9),));
world.spawn((A(10),));
// With/Without for B and C
assert_all_sizes_equal::<&A, With<B>>(&mut world, 3);
assert_all_sizes_equal::<&A, With<C>>(&mut world, 4);
assert_all_sizes_equal::<&A, Without<B>>(&mut world, 7);
assert_all_sizes_equal::<&A, Without<C>>(&mut world, 6);
// With/Without (And) combinations
assert_all_sizes_equal::<&A, (With<B>, With<C>)>(&mut world, 1);
assert_all_sizes_equal::<&A, (With<B>, Without<C>)>(&mut world, 2);
assert_all_sizes_equal::<&A, (Without<B>, With<C>)>(&mut world, 3);
assert_all_sizes_equal::<&A, (Without<B>, Without<C>)>(&mut world, 4);
// With/Without Or<()> combinations
assert_all_sizes_equal::<&A, Or<(With<B>, With<C>)>>(&mut world, 6);
assert_all_sizes_equal::<&A, Or<(With<B>, Without<C>)>>(&mut world, 7);
assert_all_sizes_equal::<&A, Or<(Without<B>, With<C>)>>(&mut world, 8);
assert_all_sizes_equal::<&A, Or<(Without<B>, Without<C>)>>(&mut world, 9);
assert_all_sizes_equal::<&A, (Or<(With<B>,)>, Or<(With<C>,)>)>(&mut world, 1);
assert_all_sizes_equal::<&A, Or<(Or<(With<B>, With<C>)>, With<D>)>>(&mut world, 6);
for i in 11..14 {
world.spawn((A(i), D(i)));
}
assert_all_sizes_equal::<&A, Or<(Or<(With<B>, With<C>)>, With<D>)>>(&mut world, 9);
assert_all_sizes_equal::<&A, Or<(Or<(With<B>, With<C>)>, Without<D>)>>(&mut world, 10);
// a fair amount of entities
for i in 14..20 {
world.spawn((C(i), D(i)));
}
assert_all_sizes_equal::<Entity, (With<C>, With<D>)>(&mut world, 6);
}
#[test]
fn query_filtered_combination_size() {
fn choose(n: usize, k: usize) -> usize {
if n == 0 || k == 0 || n < k {
return 0;
@ -100,27 +196,24 @@ mod tests {
assert_combination::<Q, F, 128>(world, choose(expected, 128));
}
fn assert_all_sizes_iterator_equal(
iterator: impl ExactSizeIterator,
iterator: impl Iterator,
expected_size: usize,
query_type: &'static str,
) {
let size_hint_0 = iterator.size_hint().0;
let size_hint_1 = iterator.size_hint().1;
let len = iterator.len();
// `count` tests that not only it is the expected value, but also
// the value is accurate to what the query returns.
let count = iterator.count();
// This will show up when one of the asserts in this function fails
println!(
r#"query declared sizes:
for query: {query_type}
expected: {expected_size}
len(): {len}
size_hint().0: {size_hint_0}
size_hint().1: {size_hint_1:?}
count(): {count}"#
for query: {query_type}
expected: {expected_size}
size_hint().0: {size_hint_0}
size_hint().1: {size_hint_1:?}
count(): {count}"#
);
assert_eq!(len, expected_size);
assert_eq!(size_hint_0, expected_size);
assert_eq!(size_hint_1, Some(expected_size));
assert_eq!(count, expected_size);

20
crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.rs
View File

@ -1,20 +0,0 @@
use bevy_ecs::prelude::*;
#[derive(Component)]
struct Foo;
#[derive(Component)]
struct Bar;
fn on_changed(query: Query<&Foo, Or<(Changed<Foo>, With<Bar>)>>) {
// this should fail to compile
is_exact_size_iterator(query.iter_combinations::<2>());
}
fn on_added(query: Query<&Foo, (Added<Foo>, Without<Bar>)>) {
// this should fail to compile
is_exact_size_iterator(query.iter_combinations::<2>());
}
fn is_exact_size_iterator<T: ExactSizeIterator>(_iter: T) {}
fn main() {}

51
crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.stderr
View File

@ -1,51 +0,0 @@
error[E0277]: the trait bound `bevy_ecs::query::Changed<Foo>: ArchetypeFilter` is not satisfied
--> tests/ui/query_combin_exact_sized_iterator_safety.rs:10:28
|
10 | is_exact_size_iterator(query.iter_combinations::<2>());
| ---------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `ArchetypeFilter` is not implemented for `bevy_ecs::query::Changed<Foo>`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `ArchetypeFilter`:
()
(F0, F1)
(F0, F1, F2)
(F0, F1, F2, F3)
(F0, F1, F2, F3, F4)
(F0, F1, F2, F3, F4, F5)
(F0, F1, F2, F3, F4, F5, F6)
(F0, F1, F2, F3, F4, F5, F6, F7)
and $N others
= note: required because of the requirements on the impl of `ArchetypeFilter` for `bevy_ecs::query::Or<(bevy_ecs::query::Changed<Foo>, bevy_ecs::query::With<Bar>)>`
= note: required because of the requirements on the impl of `ExactSizeIterator` for `QueryCombinationIter<'_, '_, &Foo, bevy_ecs::query::Or<(bevy_ecs::query::Changed<Foo>, bevy_ecs::query::With<Bar>)>, 2>`
note: required by a bound in `is_exact_size_iterator`
--> tests/ui/query_combin_exact_sized_iterator_safety.rs:18:30
|
18 | fn is_exact_size_iterator<T: ExactSizeIterator>(_iter: T) {}
| ^^^^^^^^^^^^^^^^^ required by this bound in `is_exact_size_iterator`
error[E0277]: the trait bound `bevy_ecs::query::Added<Foo>: ArchetypeFilter` is not satisfied
--> tests/ui/query_combin_exact_sized_iterator_safety.rs:15:28
|
15 | is_exact_size_iterator(query.iter_combinations::<2>());
| ---------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `ArchetypeFilter` is not implemented for `bevy_ecs::query::Added<Foo>`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `ArchetypeFilter`:
()
(F0, F1)
(F0, F1, F2)
(F0, F1, F2, F3)
(F0, F1, F2, F3, F4)
(F0, F1, F2, F3, F4, F5)
(F0, F1, F2, F3, F4, F5, F6)
(F0, F1, F2, F3, F4, F5, F6, F7)
and $N others
= note: required because of the requirements on the impl of `ArchetypeFilter` for `(bevy_ecs::query::Added<Foo>, bevy_ecs::query::Without<Bar>)`
= note: required because of the requirements on the impl of `ExactSizeIterator` for `QueryCombinationIter<'_, '_, &Foo, (bevy_ecs::query::Added<Foo>, bevy_ecs::query::Without<Bar>), 2>`
note: required by a bound in `is_exact_size_iterator`
--> tests/ui/query_combin_exact_sized_iterator_safety.rs:18:30
|
18 | fn is_exact_size_iterator<T: ExactSizeIterator>(_iter: T) {}
| ^^^^^^^^^^^^^^^^^ required by this bound in `is_exact_size_iterator`