Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rand: re-export UnwrapMut & UnwrapErr #1594

Closed
wants to merge 1 commit into from

Conversation

baloo
Copy link
Contributor

@baloo baloo commented Feb 18, 2025

  • Added a CHANGELOG.md entry

Summary

Motivation

Consumers may not directly depend on rand_core and those unwrap wrappers may be convenient.

Details

@vks
Copy link
Collaborator

vks commented Feb 21, 2025

Do you have a use case where this is convenient?

@newpavlov
Copy link
Member

newpavlov commented Feb 21, 2025

I think that use of these wrappers should be rare enough in practice to warrant a minor pain of importing them as rand::rand_core::{UnwrapMut, UnwapErr}'.

@baloo baloo force-pushed the baloo/rand/export-unwrap branch from 6a89119 to 3b25329 Compare February 21, 2025 17:28
@baloo
Copy link
Contributor Author

baloo commented Feb 21, 2025

I'm not using it, that was just following up on this comment:
#1593 (review)

@dhardy
Copy link
Member

dhardy commented Feb 21, 2025

I think that use of these wrappers should be rare enough in practice to warrant a minor pain of importing them as rand::rand_core::{UnwrapMut, UnwapErr}'.

We don't have (pub) rand::rand_core, because most parts which aren't implementation helpers are already re-export from rand, however on examination we're also missing:

  • OsError (though this is available as <OsRng as TryRngCore>::Error)
  • RngReadAdapter

All of these are things which might be rarely used. I think the best thing may be to just export the three adapters from rand and the error type from rand::rngs.

@newpavlov
Copy link
Member

We don't have (pub) rand::rand_core, because most parts which aren't implementation helpers are already re-export from rand

Huh... I missed that. I think it's better to simply re-export rand_core. There are virtually no drawbacks to doing that (the only thing I could think of is an additional entry in docs, but here you propose to create several such entries), while not having access to rand_core from rand creates headaches like this.

@baloo baloo mentioned this pull request Feb 22, 2025
1 task
@baloo
Copy link
Contributor Author

baloo commented Feb 22, 2025

I agree with the idea rand should re-export rand_core. Here is a PR for that: #1604 up for you to choose.

@dhardy dhardy closed this Feb 22, 2025
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.

4 participants