Generics #319
Generics #319
Conversation
dhardy
added
X-enhancement
B-API
pitdicker
left a comment
pitdicker
left a comment
There was a problem hiding this comment.
All looks good to me, and cleaner 👍. Also very readable documentation.
|
Do you want to wait for someone to review? Or is this ready to be merged? |
|
I think it's good, but with significant changes like this I prefer to wait a couple of days. |
| /// // This uses StdRng, but is valid for any R: SeedableRng | ||
| /// let mut rng = StdRng::from_rng(&mut EntropyRng::new())?; | ||
| /// let mut rng = StdRng::from_rng(EntropyRng::new())?; | ||
| /// |
There was a problem hiding this comment.
I think the examples and documentation should do &mut much more frequently than not, as inexperienced rust developers may find the error message from a missing &mut unintuitive. I don't see any problem with doing this one without the &mut of course.
There was a problem hiding this comment.
Using a different convention for from_rng than the other RNG consumers bugs me a bit, but seems the best option. It sounds like the RFC I opened about reborrowing is being postponed, so we're not going to get a nice uniform solution any time soon; I guess then this is the best we can do.
There was a problem hiding this comment.
I haven't checked the error message myself, but really that sounds like the only problematic part. If it's really confusion then we could suggest a rewording as a rustc bug, which get addressed quickly.
This is the motivation for the previous commit.
|
Merge collision 😒 I rebased; merge after CI checks are complete |
3 participants
Implement #287 (as it currently stands). Without changes to Rust itself I think this is our best option. I'm unsure about making some further changes in the future; I guess it depends on the state of Rust when we want to make 1.0.