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

When using gen_range the panics appear to not be documented correctly resulting in potential overflow #1337

Closed
michael-jaquier opened this issue Sep 12, 2023 · 6 comments

Comments

@michael-jaquier
Copy link

Hello,

I recently tried to run this code in one of our tests

let mut rng = thread_rng();
let value: f32 = rng.gen_range(f32::MIN, f32::MAX)

The result was an overflow error which makes sense in context of this ( https://docs.rs/rand/latest/src/rand/distributions/uniform.rs.html#884 )

BigNumber - ( - BigNumber ) == VeryBigNumber

So overflow here is not surprising but in the list of panics here -> https://docs.rs/rand/latest/rand/trait.Rng.html#method.gen_range

We see only considerations for "if the range is empty" leading me to initially believe using min and max ranges for a type as valid but I believe they are not?

@dhardy
Copy link
Member

dhardy commented Sep 13, 2023

using min and max ranges for a type as valid

This statement does not apply equally to integer and floating-point types.

See #1090. Also, I'm not sure if this applies equally to 0.8.5 (which is quite old) and the latest in master.

@michael-jaquier
Copy link
Author

michael-jaquier commented Sep 13, 2023

Hi thanks for the reply. i read the other issue and it does seem nearly identical to this issue -- but im unsure if there was a positive conclusion to it?


using min and max ranges for a type as valid

This statement does not apply equally to integer and floating-point types.

See #1090. Also, I'm not sure if this applies equally to 0.8.5 (which is quite old) and the latest in master.

Im not sure to which statement you are referring to; Or is it the absence of a statement regarding bounds checking? That said it would not only be the case of min & max in this event but max and any negative value or min and any positive number would cause overflow in this case.

Would not a # Panic descrbing the limitations on overflow be called for in this event? e.g

# Panics
If the values max - min result in a value greater than the type can store ( e.g., `gen_range(-1.0, f32::MAX)`) the function will panic 

While ensuring that those bounds are respected by the compiler is probably tricky documenting the caveat seems appropriate unless of course im missing something obvious about the documentation where this limitation is spelled out. It wasn't obvious to three of our engineers which made me believe the docs could benefit from being more explicit -- again unless its documented elsewhere

@dhardy
Copy link
Member

dhardy commented Sep 13, 2023

Yes, we probably do need better docs regarding this.

Regarding panic on overflow, we should consider this.

@vks
Copy link
Collaborator

vks commented Oct 30, 2023

I just checked that this panics on master, but the docs of gen_range could mention this.

@vks
Copy link
Collaborator

vks commented Oct 30, 2023

Fixed in #1351.

@dhardy
Copy link
Member

dhardy commented Oct 31, 2023

Thanks @vks

@dhardy dhardy closed this as completed Oct 31, 2023
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

No branches or pull requests

3 participants