fix https://github.com/rust-random/rand/issues/661

[TheIronBorn]

fixes #661

[vks]

Should we add the test case from #661?

[dhardy]

@vks how? That test case re-implements the algorithm over a u8 RNG. Our code already has explicitly different behaviour for u16 and u32 types and testing all possible inputs for a u32 is way beyond the scope of CI testing.

[vks]

@dhardy We could make factor (range << range.leading_zeros()).wrapping_sub(1) into its own function (i.e. approximate_zone) and add the test case for the function. Not sure whether this is worth it, but it at least documents clearly why the wrapping_sub is necessary.

[TheIronBorn]

Something like StepRng might work except for the u8 as u32 optimization and knowing when to stop.

[dhardy]

knowing when to stop

Depends on $ty::MAX and the range, so I don't see any general option good enough to be a useful test.

I would just go with this PR, except maybe to say we subtract 1 because we are using inclusive bounds not exclusive upper bound.