rust-random / rand

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

Inherent rng methods #1492

Closed dhardy closed 1 month ago

dhardy commented 2 months ago

Summary

Make Rng methods inherent methods on RNGs, and more usability tweaks.

Motivation and details

This follows arguments made in #989 and #1488. Specifically,

I did consider publicly exporting impl_rng_methods_as_inherent, but realised a problem: rand_chacha cannot use this since rand depends on rand_chacha (and the macro cannot be moved to rand_core); some other RNG crates could use this but should not depend on rand.

Incomplete / questions

Do we want to rename gen_range to just range? What about gen_bool, gen_ratio?

Do we want to remove rand::random? If not, do we want to add random_iter, gen_range etc. as free functions? (rand::random is hardly used inside rand itself. This GitHub search has 33k code hits but it's hard to tell how many are genuine matches.)

@oconnor663 gave motivation to rename thread_rng to just rng:

you [the user] have to understand what thread_rng means and that it's the right thing to use

We could do this. If this were a clean design then probably we should: it's an implementation detail that we use a thread-local generator and not, say, a mutex over a single (static) generator. But it's also rather widely used: approx 150 mentions in this repo and 176k code hits via GitHub search.

dhardy commented 2 months ago

I was intending to leave these changes until after rand v0.9, but now think we should get these merged first: upgrading to 0.9 already brings many small breaking changes for users; rolling these into a single (larger) changeset should result in less work for those upgrading.

Some more thoughts:

dhardy commented 2 months ago

This PR no longer removes the rand::random function (left to a future PR). Other changes of this PR I'd like to see merged, unless there's any issue I haven't spotted here. Ultimately I hope we can replace these inherent impls with native Rust support for inherent trait methods.

dhardy commented 1 month ago

I want to agree, especially since these inherent impls will not work for any RNG outside of rand. The objective was to ease usage (esp. for people new to Rust), but the result making this less consistent across RNGs does not help.