-
Notifications
You must be signed in to change notification settings - Fork 67
Remove ChaChaXCore types #512
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
Conversation
chacha20/src/rng.rs
Outdated
| } | ||
| } | ||
| impl CryptoGenerator for $ChaChaXCore {} | ||
| impl CryptoGenerator for ChaChaCore<$rounds, Legacy> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the field will be private this impl can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point being that we don't need trait CryptoGenerator at all. The trait is (I believe) only used by ReseedingRng, so lets start with that. (Another PR will be needed to update to the new rand_core soon anyway.)
|
I think it better to merge this then make a new PR for rust-random/rand_core#68. |
|
Aside: it would be nice to expose the What is the preferred way to do this — a simple unparametrised method? Or provide a copy of |
|
I think we can add public inherent methods and then use them in implementation of |
|
Yeah, just add an inherent method with the same name/type signature as the trait. You can always disambiguate via UFCS |
|
Done. UFCS shouldn't be needed since inherent methods always have priority and the in/out types are the same... ... expect for |
|
I believe that makes the most sense, so done (last two commits). |
|
Note the failing CI. |
5db9f4a to
2999132
Compare
|
Why does #[cfg(any(feature = "cipher", feature = "rng"))]
state: [u32; STATE_WORDS],Is it supposed to compile to an empty struct if neither of these features are used? |
Corrects an oversight from #512.
In discussion of API bloat with @newpavlov, it became clear that we don't need these types.
The only known use is
rand::rngs::ThreadRng, yet there's no reason that can't useChaChaCore(already a public item).