Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SignedShrinker bounded for <$ty>::MIN #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

neithernut
Copy link

Originally BurntSushi#296

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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant