From 684378eaace9b621d218a3831f1914e73be94b8c Mon Sep 17 00:00:00 2001 From: Julian Ganz Date: Mon, 9 Aug 2021 21:55:35 +0200 Subject: [PATCH 1/3] impl: make SignedShrinker bounded for <$ty>::MIN Previously, the `SignedShrinker` was guaranteed to yield a result if initialized with `<$ty>::MIN` (e.g. `i32::MIN` for shrinking `i32` and `f32`). This would result in endless shrinking if a test would happen to fail for these values. Shrinking signed values is done by halfing a second value `i` and subtracting it from the original `x`. Thus `i` will be equal to `0` at some point. We choose to halt the iteration in this state, i.e. not yield any new values. Yielding `x - 0` would be pointless anyway, since that obviously equals the original value which was the initial witness for the test failure. --- src/arbitrary.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/arbitrary.rs b/src/arbitrary.rs index 92f893b..cf457f2 100644 --- a/src/arbitrary.rs +++ b/src/arbitrary.rs @@ -824,7 +824,9 @@ macro_rules! signed_shrinker { impl Iterator for SignedShrinker { type Item = $ty; fn next(&mut self) -> Option<$ty> { - if self.x == <$ty>::MIN + if self.i == 0 { + None + } else if self.x == <$ty>::MIN || (self.x - self.i).abs() < self.x.abs() { let result = Some(self.x - self.i); From a00228cf0a930484d5be057e99882caccd7a8049 Mon Sep 17 00:00:00 2001 From: Julian Ganz Date: Wed, 11 Aug 2021 20:37:07 +0200 Subject: [PATCH 2/3] impl: simplify control flow in SignedShrinker The condition `(self.x - self.i).abs() < self.x.abs()` is true if `self.i` is not `0` and has the same sign than `self.x`. The latter is always the case due to the initialization, the former was recently introduced to bound the iteration in the case of `self.x == <$ty>::MIN`. Thus, both of these conditions became redundant. --- src/arbitrary.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/arbitrary.rs b/src/arbitrary.rs index cf457f2..4c32f2f 100644 --- a/src/arbitrary.rs +++ b/src/arbitrary.rs @@ -824,11 +824,7 @@ macro_rules! signed_shrinker { impl Iterator for SignedShrinker { type Item = $ty; fn next(&mut self) -> Option<$ty> { - if self.i == 0 { - None - } else if self.x == <$ty>::MIN - || (self.x - self.i).abs() < self.x.abs() - { + if self.i != 0 { let result = Some(self.x - self.i); self.i = self.i / 2; result From 0c279d6f15d262b471d94ac70ad4e4d1ac5b6fad Mon Sep 17 00:00:00 2001 From: Julian Ganz Date: Thu, 26 Aug 2021 17:52:47 +0200 Subject: [PATCH 3/3] impl: make sure a float's shrinker does not yield the original value For floating point values, we use a shrinker for signed integers and convert the output to the float's type. The underlying shrinker is bounded and never includes the original value in the output, i.e. it behaves correctly. However, when converting the value to the target type we may loose some information and thus yield the original value, again. If the test happens to fail for such a value, we may end up endlessly repeating the test with a sequence of values including the original one over and over again. This change avoids the issue by applying an additional filter. --- src/arbitrary.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/arbitrary.rs b/src/arbitrary.rs index 4c32f2f..9d972fc 100644 --- a/src/arbitrary.rs +++ b/src/arbitrary.rs @@ -894,7 +894,8 @@ macro_rules! float_arbitrary { fn shrink(&self) -> Box> { signed_shrinker!($shrinkable); let it = shrinker::SignedShrinker::new(*self as $shrinkable); - Box::new(it.map(|x| x as $t)) + let old = *self; + Box::new(it.map(|x| x as $t).filter(move |x| *x != old)) } } )*};