dhardy / rand

http://doc.rust-lang.org/rand
Other
2 stars 2 forks source link

Re-export `rand-core` members in `rand`? #15

Closed dhardy closed 6 years ago

dhardy commented 7 years ago

Since rand-core is intended to be a separate crate, but with rand depending on it, should some parts of be re-exported in rand?

I argue that the following should be re-exported in rand so that "normal users" do not have to depend on rand-core:

The rand_core::impls module is aimed at implementations of Rng so does not need to be re-exported (in my opinion).

~There is one other item: the mock module. Probably it is useful to re-export this from rand.~

dhardy commented 7 years ago

It is also worth mentioning the story for crates implementing Rng / CryptoRng: the intention is that such crates can depend on rand-core but don't need to use rand.

pitdicker commented 7 years ago

Not re-exporting rand_core::impls would almost force crates implementing Rng to depend on rand-core instead of rand, guiding them to the right dependency. 👍

Could you explain why the mock module should be in rand_core?

dhardy commented 7 years ago

I put mock in there to allow tests on the Rng trait. It doesn't really need to there of course.

dhardy commented 7 years ago

Ok, I moved mock from rand_core to rand, so that's not a problem any more. I think the rest is resolved?

pitdicker commented 7 years ago

The re-exports seem fine to me.

I still like to look over rand_core::impls. I now have a branch with 45% wins for fill_bytes with Isaac, and suspect a much smaller win for something like Xorshift should be possible to. And it needs testing. But that is not relevant for this issue...

burdges commented 6 years ago

I know this messes up the crate separation, but did you consider

pub trait Rng : Sample { ... }
pub trait Sample { ... }
impl<R: Rng> Sample for R { ... }

Would that eliminate needing use rand::Sample? I know that a minor thing, and probably not even a good thing for reading/auditing, but it's vaguely relevant to backwards compatibility. It might also create a bunch of double references in trait objects, which maybe slows down &mut R : Rng slightly.

dhardy commented 6 years ago

I don't believe it would eliminate the need to use rand::Sample, no.

Good point about this being a breaking change; I don't see an obvious way around it though (also the crate separation is quite important IMO).

burdges commented 6 years ago

Right, if that existed it'd require an extra pub like pub trait Rng : pub Sample or something.

dhardy commented 6 years ago

Like class extension in classic OO languages? Yes, but I don't think anything like that exists in Rust.

nixpulvis commented 6 years ago

I think it would be a really good exercise to write down the complete desired interfaces for both rand and rand_core in once place. Might make discussions like this a bit easier (at least for me).

Edit: Public interface. I don't care about internal details for the purposes of this comment.

dhardy commented 6 years ago

Check out the documentation: https://dhardy.github.io/rand/

dhardy commented 6 years ago

Important to this topic are: which other parts of rand should be moved to other crates?

There are more subdivisions possible, although these appear less useful:

Finally, rand::random is the most derived feature: it depends on Sample and distribution code for conversion, and on thread_rng which uses StdRng and ReseedingRng with entropy from the external RNGs — about the only things it doesn't depend on are the sequences module, weak_rng, and the Xorshift and ChaCha generators.

Conclusion

Re-exports

rand is dependent on rand-core, the external RNGs and some PRNGs. To save users depending on multiple crates, we should import anything from these crates which (1) is likely to be useful (includes most stuff, except perhaps rand-core::impls) and (2) is unlikely to be replaced (this may rule out PRNGs as noted in #58).

dhardy commented 6 years ago

I think this topic is either decided or will be reviewed later, so closing the issue.