Skip to content

Make CryptoRngCore trait imply CryptoRng as well#1230

Merged
dhardy merged 1 commit into
rust-random:masterfrom
cbeck88:crypto-rng-core-requires-crypto-rng
May 19, 2022
Merged

Make CryptoRngCore trait imply CryptoRng as well#1230
dhardy merged 1 commit into
rust-random:masterfrom
cbeck88:crypto-rng-core-requires-crypto-rng

Conversation

@cbeck88

@cbeck88 cbeck88 commented May 14, 2022

Copy link
Copy Markdown

Hi,

I recently had a case where I needed to make an API that takes &mut dyn (CryptoRng + RngCore). I didn't want to make my trait generic because it would greatly complicate some FFI situations.

Since rust doesn't currently allow dyn (CryptoRng + RngCore), I needed to make an extension trait that implies both of these, and, naturally, is implied for any object with both of these traits.

It turns out that rand crate already does almost the same thing with CryptoRngCore (which is actually what I named my extension trait).

However the implementation in rand crate is slightly different: CryptoRngCore implies RngCore but not CryptoRng.

I can't see right now that there's a compelling use-case for it the way it's implemented right now: any place where I would write &mut dyn CryptoRngCore my code would also work if my function takes &mut dyn RngCore, since CryptoRngCore provides no additional functionality other than a function that converts self to &mut dyn RngCore. (But I could have started with that...)

OTOH if CryptoRngCore implies both RngCore and CryptoRng then it's quite useful, because it works around the inability to make dyn (X + Y) in rust right now. Then I would just use the upstream version and drop my version.

Let me know what you think. Thank you!

@dhardy

dhardy commented May 16, 2022

Copy link
Copy Markdown
Member

See also #1187. Looks good to me but I'll give others a chance to take a look.

@cbeck88

cbeck88 commented May 16, 2022

Copy link
Copy Markdown
Author

(I guess this is also described in the alternatives section of this RFC: #1143)

@dhardy dhardy merged commit 3543f4b into rust-random:master May 19, 2022
@cbeck88 cbeck88 deleted the crypto-rng-core-requires-crypto-rng branch May 19, 2022 18:08
@dhardy dhardy mentioned this pull request Mar 25, 2024
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