rust-random / rand

A Rust library for random number generation.
https://crates.io/crates/rand
Other
1.67k stars 432 forks source link

Annotate functions with #[track_caller] #1437

Closed Tahinli closed 6 months ago

Tahinli commented 7 months ago

Summary

Hi, I suggest that code should error at the line that is written by developer not in the crate itself when rand fails.

Details

What I mean is:

use rand::Rng;
fn main() {
    println!("Hello, world!");
    let x = rand::thread_rng().gen_range(1.0..1.0);
    println!("{}", x);
}

4th line will fail as we all know, because there is no random number can be produced between them. But problem is when this line fails we get error from inside of the crate not the line itself.

thread 'main' panicked at /home/tahinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rand-0.8.5/src/rng.rs:134:9:
cannot sample empty range
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This makes code hard to debug. You may say can't you see there is a problem, yes I can, but while using variables inside loops or something like that, it's really hard. Especially if there are more rand line then one. It's hard to find which line fails. Only way to find where it's fails is debugging as far as I know.

Motivation

I think for friendliness about Rust ecosystem especially for compile errors. This errors should raise on the precise line.

Alternatives

as an alternative =

rand::thread_rng().gen_range()

may return option so developer can handle errors.

josephlr commented 7 months ago

The main issue here is that we don't have gen_range annotated with #[track_caller]. If we did, the panic error message would properly display the relevant file/line number of the error. In general, for functions which have an initial assert! or # Panics doc section, we should probably just annotate those functions with #[track_caller]. They are usually short, generic functions, so will (almost always) be inlined anyway, so there's not a perf penalty.

I'll update the description to reflect this.

In the meantime, you can debug such panics by doing what the error message suggests: passing RUST_BACKTRACE=1 or RUST_BACKTRACE=full. It's not perfect, mostly because the more detailed debug information is removed in release mode.

as an alternative =

rand::thread_rng().gen_range()

may return option so developer can handle errors.

We probably won't do this as it would be very disruptive to all users of rand and a backwards incompatible change.

vks commented 7 months ago

If you prefer to get a Result, please consider using Uniform::try_from with Rand >= 0.9.