Generics #319
Generics #319
Conversation
dhardy
added
X-enhancement
B-API
|
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. |
| @@ -805,7 +805,7 @@ pub trait NewRng: SeedableRng { | |||
| /// | |||
| /// fn foo() -> Result<(), Error> { | |||
| /// // 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 |
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.