Fix overflows applying selection iterators to empty trees by choosing…

… a more tame value for AABB::new_empty (#162)

- [x] I agree to follow the project's [code of
conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [x] I added an entry to `rstar/CHANGELOG.md` if knowledge of this
change could be valuable to users.
---

Closes #161

rstar/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Changed
 - Switched to unstable sort for envelopes and node reinsertion ([PR](https://github.com/georust/rstar/pull/160))
+- Use a more tame value for `AABB::new_empty` to avoid overflow panics applying selections on empty trees ([PR](https://github.com/georust/rstar/pull/162))
 
 # 0.12.0
 

rstar/src/aabb.rs

@@ -1,6 +1,6 @@
 use crate::point::{max_inline, Point, PointExt};
 use crate::{Envelope, RTreeObject};
-use num_traits::{Bounded, One, Zero};
+use num_traits::{One, Zero};
 
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
@@ -220,11 +220,11 @@ where
 }
 
 fn new_empty<P: Point>() -> AABB<P> {
-    let max = P::Scalar::max_value();
-    let min = P::Scalar::min_value();
+    let one = P::Scalar::one();
+    let zero = P::Scalar::zero();
     AABB {
-        lower: P::from_value(max),
-        upper: P::from_value(min),
+        lower: P::from_value(one),
+        upper: P::from_value(zero),
     }
 }
 

rstar/src/algorithm/bulk_load/bulk_load_sequential.rs

@@ -4,6 +4,7 @@ use crate::object::RTreeObject;
 use crate::params::RTreeParams;
 use crate::point::Point;
 
+#[cfg(not(test))]
 use alloc::{vec, vec::Vec};
 
 #[allow(unused_imports)] // Import is required when building without std

rstar/src/algorithm/bulk_load/cluster_group_iterator.rs

@@ -1,5 +1,6 @@
 use crate::{Envelope, Point, RTreeObject, RTreeParams};
 
+#[cfg(not(test))]
 use alloc::vec::Vec;
 
 #[allow(unused_imports)] // Import is required when building without std

rstar/src/algorithm/intersection_iterator.rs

@@ -4,6 +4,7 @@ use crate::RTreeNode;
 use crate::RTreeNode::*;
 use crate::RTreeObject;
 
+#[cfg(not(test))]
 use alloc::vec::Vec;
 use core::mem::take;
 

rstar/src/algorithm/iterators.rs

@@ -304,4 +304,11 @@ mod test {
             assert!(located.contains(point));
         }
     }
+
+    #[test]
+    fn test_locate_within_distance_on_empty_tree() {
+        let tree: RTree<[i64; 3]> = RTree::new();
+
+        tree.locate_within_distance([0, 0, 0], 10);
+    }
 }

rstar/src/algorithm/nearest_neighbor.rs

@@ -2,7 +2,9 @@ use crate::node::{ParentNode, RTreeNode};
 use crate::point::{min_inline, Point};
 use crate::{Envelope, PointDistance, RTreeObject};
 
-use alloc::{collections::BinaryHeap, vec, vec::Vec};
+use alloc::collections::BinaryHeap;
+#[cfg(not(test))]
+use alloc::{vec, vec::Vec};
 use core::mem::replace;
 use heapless::binary_heap as static_heap;
 use num_traits::Bounded;

rstar/src/algorithm/removal.rs

@@ -6,6 +6,7 @@ use crate::object::RTreeObject;
 use crate::params::RTreeParams;
 use crate::{Envelope, RTree};
 
+#[cfg(not(test))]
 use alloc::{vec, vec::Vec};
 
 #[allow(unused_imports)] // Import is required when building without std
@@ -252,7 +253,7 @@ mod test {
     use crate::point::PointExt;
     use crate::primitives::Line;
     use crate::test_utilities::{create_random_points, create_random_rectangles, SEED_1, SEED_2};
-    use crate::{RTree, AABB};
+    use crate::AABB;
 
     use super::*;
 

rstar/src/algorithm/rstar.rs

@@ -5,6 +5,7 @@ use crate::params::{InsertionStrategy, RTreeParams};
 use crate::point::{Point, PointExt};
 use crate::rtree::RTree;
 
+#[cfg(not(test))]
 use alloc::vec::Vec;
 use num_traits::{Bounded, Zero};
 

rstar/src/node.rs

@@ -2,6 +2,7 @@ use crate::envelope::Envelope;
 use crate::object::RTreeObject;
 use crate::params::RTreeParams;
 
+#[cfg(not(test))]
 use alloc::vec::Vec;
 
 #[cfg(feature = "serde")]

rstar/src/point.rs

@@ -14,16 +14,20 @@ use num_traits::{Bounded, Num, Signed, Zero};
 /// # Example
 /// ```
 /// # extern crate num_traits;
-/// use num_traits::{Bounded, Num, Signed};
+/// use num_traits::{Num, One, Signed, Zero};
 ///
 /// #[derive(Clone, Copy, PartialEq, PartialOrd, Debug)]
 /// struct MyFancyNumberType(f32);
 ///
-/// impl Bounded for MyFancyNumberType {
+/// impl Zero for MyFancyNumberType {
 ///   // ... details hidden ...
-/// # fn min_value() -> Self { MyFancyNumberType(Bounded::min_value()) }
-/// #
-/// # fn max_value() -> Self { MyFancyNumberType(Bounded::max_value()) }
+/// # fn zero() -> Self { MyFancyNumberType(Zero::zero()) }
+/// # fn is_zero(&self) -> bool { unimplemented!() }
+/// }
+///
+/// impl One for MyFancyNumberType {
+///   // ... details hidden ...
+/// # fn one() -> Self { MyFancyNumberType(One::one()) }
 /// }
 ///
 /// impl Signed for MyFancyNumberType {
@@ -54,13 +58,9 @@ use num_traits::{Bounded, Num, Signed, Zero};
 /// rtree.insert([MyFancyNumberType(0.0), MyFancyNumberType(0.0)]);
 /// # }
 ///
-/// # impl num_traits::Zero for MyFancyNumberType {
-/// #   fn zero() -> Self { unimplemented!() }
-/// #   fn is_zero(&self) -> bool { unimplemented!() }
-/// # }
-/// #
-/// # impl num_traits::One for MyFancyNumberType {
-/// #   fn one() -> Self { unimplemented!() }
+/// # impl num_traits::Bounded for MyFancyNumberType {
+/// #   fn min_value() -> Self { unimplemented!() }
+/// #   fn max_value() -> Self { unimplemented!() }
 /// # }
 /// #
 /// # impl core::ops::Mul for MyFancyNumberType {
@@ -202,13 +202,7 @@ pub trait PointExt: Point {
         other: &Self,
         mut f: impl FnMut(Self::Scalar, Self::Scalar) -> bool,
     ) -> bool {
-        // TODO: Maybe do this by proper iteration
-        for i in 0..Self::DIMENSIONS {
-            if !f(self.nth(i), other.nth(i)) {
-                return false;
-            }
-        }
-        true
+        (0..Self::DIMENSIONS).all(|i| f(self.nth(i), other.nth(i)))
     }
 
     /// Returns the dot product of `self` and `rhs`.
@@ -225,12 +219,7 @@ pub trait PointExt: Point {
     ///
     /// After applying the closure to every component of the Point, fold() returns the accumulator.
     fn fold<T>(&self, start_value: T, mut f: impl FnMut(T, Self::Scalar) -> T) -> T {
-        let mut accumulated = start_value;
-        // TODO: Maybe do this by proper iteration
-        for i in 0..Self::DIMENSIONS {
-            accumulated = f(accumulated, self.nth(i));
-        }
-        accumulated
+        (0..Self::DIMENSIONS).fold(start_value, |accumulated, i| f(accumulated, self.nth(i)))
     }
 
     /// Returns a Point with every component set to `value`.

rstar/src/rtree.rs

@@ -1,18 +1,17 @@
 use crate::algorithm::bulk_load;
-use crate::algorithm::intersection_iterator::IntersectionIterator;
 use crate::algorithm::iterators::*;
 use crate::algorithm::nearest_neighbor;
 use crate::algorithm::nearest_neighbor::NearestNeighborDistance2Iterator;
 use crate::algorithm::nearest_neighbor::NearestNeighborIterator;
 use crate::algorithm::removal;
-use crate::algorithm::removal::DrainIterator;
 use crate::algorithm::selection_functions::*;
 use crate::envelope::Envelope;
 use crate::node::ParentNode;
 use crate::object::{PointDistance, RTreeObject};
 use crate::params::{verify_parameters, DefaultParams, InsertionStrategy, RTreeParams};
 use crate::Point;
 
+#[cfg(not(test))]
 use alloc::vec::Vec;
 
 #[cfg(feature = "serde")]