Skip to content

Conversation

@dhardy
Copy link
Member

@dhardy dhardy commented Oct 20, 2025

  • Added a CHANGELOG.md entry

Summary

Move fn read_adapter out of trait TryRngCore, renaming the adapter to RngReader.

Remove &mut indirection inside RngReader. We can still use &mut R where R: RngCore but not &mut OsRng since the latter does not implement TryRngCore. (Taking non-RngCore try-RNGs by value like RngReader(OsRng) does work.)

Also add unstable feature specialization. Since this is documented as unstable and only usable on nightly rustc we don't need to worry about mis-use too much, except maybe via --all-features.

Alternatives

We could drop the specialization feature. Removed.

We could keep the &mut indirection; I don't think there's a strong incentive though.

We could put RngReader in rand or another crate. Moved to rand.

@dhardy dhardy requested a review from newpavlov October 20, 2025 11:01
@dhardy
Copy link
Member Author

dhardy commented Oct 20, 2025

This is currently only available in rand through rand::rand_core::RngReader. What should be the experience there?

@newpavlov
Copy link
Member

I think we could move the method to the rand::Rng trait and RngReader to rand.


# Enable better Debug impl for RngReader
# This is an unstable feature requiring nightly rustc!
specialization = []
Copy link
Member

@newpavlov newpavlov Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to just remove the method, the wrapper, and the std feature.

@dhardy
Copy link
Member Author

dhardy commented Oct 20, 2025

Methods fix the receiver type (i.e. self or &mut self) and hide this from the call site (does rng.read_adapter() take rng by value?). Since this is relevant here I don't much like using a method to construct RngReader.

@newpavlov
Copy link
Member

My main point is that we probably should move RngReader to rand. I do not particularly care about the method.

@dhardy dhardy mentioned this pull request Oct 20, 2025
6 tasks
@dhardy dhardy force-pushed the push-mzsvuzxrqsyy branch from 5ff6ed9 to 8c5840f Compare October 21, 2025 09:39
Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also remove the std feature since it's only used to enable getrandom/std and we plan to remove getrandom either way.

Also, do not forget to update changelogs.

@dhardy
Copy link
Member Author

dhardy commented Oct 21, 2025

I would also remove the std feature

It's still used by OsError, so it can wait.

@dhardy dhardy merged commit d0eef34 into master Oct 21, 2025
15 checks passed
@dhardy dhardy deleted the push-mzsvuzxrqsyy branch October 21, 2025 12:48
@dhardy dhardy mentioned this pull request Oct 22, 2025
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants