From 72e001b6c8927c1369fb6880ed8b47282736b521 Mon Sep 17 00:00:00 2001 From: Theo Ottah Date: Mon, 24 Oct 2022 23:03:15 +0000 Subject: [PATCH] Remove ExactSizeIterator from QueryCombinationIter (#5895) # 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. --- crates/bevy_ecs/src/query/iter.rs | 15 --- crates/bevy_ecs/src/query/mod.rs | 111 ++++++++++++++++-- ...uery_combin_exact_sized_iterator_safety.rs | 20 ---- ..._combin_exact_sized_iterator_safety.stderr | 51 -------- 4 files changed, 102 insertions(+), 95 deletions(-) delete mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.rs delete mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.stderr diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 34d1763aecc63..374c0f7d6f1ba 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -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> diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index df0f9281e4afd..4adfbd2e369ee 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -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(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::(); + let iter = query.iter(world); + let query_type = type_name::>(); + 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>(&mut world, 1); + assert_all_sizes_equal::<&A, Without>(&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>(&mut world, 3); + assert_all_sizes_equal::<&A, With>(&mut world, 4); + assert_all_sizes_equal::<&A, Without>(&mut world, 7); + assert_all_sizes_equal::<&A, Without>(&mut world, 6); + + // With/Without (And) combinations + assert_all_sizes_equal::<&A, (With, With)>(&mut world, 1); + assert_all_sizes_equal::<&A, (With, Without)>(&mut world, 2); + assert_all_sizes_equal::<&A, (Without, With)>(&mut world, 3); + assert_all_sizes_equal::<&A, (Without, Without)>(&mut world, 4); + + // With/Without Or<()> combinations + assert_all_sizes_equal::<&A, Or<(With, With)>>(&mut world, 6); + assert_all_sizes_equal::<&A, Or<(With, Without)>>(&mut world, 7); + assert_all_sizes_equal::<&A, Or<(Without, With)>>(&mut world, 8); + assert_all_sizes_equal::<&A, Or<(Without, Without)>>(&mut world, 9); + assert_all_sizes_equal::<&A, (Or<(With,)>, Or<(With,)>)>(&mut world, 1); + assert_all_sizes_equal::<&A, Or<(Or<(With, With)>, With)>>(&mut world, 6); + + for i in 11..14 { + world.spawn((A(i), D(i))); + } + + assert_all_sizes_equal::<&A, Or<(Or<(With, With)>, With)>>(&mut world, 9); + assert_all_sizes_equal::<&A, Or<(Or<(With, With)>, Without)>>(&mut world, 10); + + // a fair amount of entities + for i in 14..20 { + world.spawn((C(i), D(i))); + } + assert_all_sizes_equal::, With)>(&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::(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); diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.rs b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.rs deleted file mode 100644 index 6a9f1775c48a5..0000000000000 --- a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.rs +++ /dev/null @@ -1,20 +0,0 @@ -use bevy_ecs::prelude::*; - -#[derive(Component)] -struct Foo; -#[derive(Component)] -struct Bar; - -fn on_changed(query: Query<&Foo, Or<(Changed, With)>>) { - // this should fail to compile - is_exact_size_iterator(query.iter_combinations::<2>()); -} - -fn on_added(query: Query<&Foo, (Added, Without)>) { - // this should fail to compile - is_exact_size_iterator(query.iter_combinations::<2>()); -} - -fn is_exact_size_iterator(_iter: T) {} - -fn main() {} diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.stderr b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.stderr deleted file mode 100644 index 4799c4d6164c5..0000000000000 --- a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_combin_exact_sized_iterator_safety.stderr +++ /dev/null @@ -1,51 +0,0 @@ -error[E0277]: the trait bound `bevy_ecs::query::Changed: 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` - | | - | 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, bevy_ecs::query::With)>` - = note: required because of the requirements on the impl of `ExactSizeIterator` for `QueryCombinationIter<'_, '_, &Foo, bevy_ecs::query::Or<(bevy_ecs::query::Changed, bevy_ecs::query::With)>, 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(_iter: T) {} - | ^^^^^^^^^^^^^^^^^ required by this bound in `is_exact_size_iterator` - -error[E0277]: the trait bound `bevy_ecs::query::Added: 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` - | | - | 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, bevy_ecs::query::Without)` - = note: required because of the requirements on the impl of `ExactSizeIterator` for `QueryCombinationIter<'_, '_, &Foo, (bevy_ecs::query::Added, bevy_ecs::query::Without), 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(_iter: T) {} - | ^^^^^^^^^^^^^^^^^ required by this bound in `is_exact_size_iterator`