Let RngCore: TryRngCore<Error = Infallible>
#45
Conversation
|
Updated. Some things need further discussion (see above). |
dhardy
changed the title
RngCore: TryRngCoreRngCore: TryRngCore<Error = Infallible>
This fixes changes made in #2195 This is due to the refactor made in rand_core in rust-random/rand_core#45 which dropped the "trait dependency" between CryptoRng and RngCore
|
We noticed something a little weird here:
So: pub trait CryptoRng: TryCryptoRng<Error = Infallible> {}impl<R: TryRngCore<Error = Infallible>> RngCore for R { ...I would expect that means any type which impls |
|
I guess we should revert my suggestion above and add a test for this. |
|
I don't get it... I tried to make a contrived version in the playground and it... works? |
|
Aha, the I think adding Updated contrived example: |
Adds `?Sized` to the bounds of the blanket impls, so unsized types can also be used by way of the blanket impl. See also: #45
|
Opened a PR to add |
| /// Wrap RNG with the [`UnwrapMut`] wrapper. | ||
| fn unwrap_mut(&mut self) -> UnwrapMut<'_, Self> { | ||
| UnwrapMut(self) | ||
| } |
There was a problem hiding this comment.
Dropping UnwrapMut causes me trouble in dhkem. UnwrapErr requires R to be Sized which may not be possible if the Rng comes from a trait.
There was a problem hiding this comment.
I only see failures "failed to load source for dependency rand_core". Can you point me to a job failing because of UnwrapErr?
Note that UnwrapErr itself does not require the Sized bound and its only field is public, so you should be able to create it using UnwrapErr(rng).
There was a problem hiding this comment.
Hm, we already have test which test the ?Sized case, see: https://github.com/rust-random/rand_core/blob/master/src/lib.rs#L620-L623
There was a problem hiding this comment.
@baloo (&mut rng).unwrap_err() should be equivalent to the old rng.unwrap_mut().
(We could perhaps add unwrap_mut(&mut self) -> UnwrapErr<&mut Self>, but it shouldn't be necessary.)
There was a problem hiding this comment.
Maybe we should just remove the unwrap_err method in favor of wrapping in UnwrapErr?
There was a problem hiding this comment.
Yeah, UnwrapErr(rng) worked, I was convinced I tried it out yesterday night, but I guess not. Thanks a lot!
CHANGELOG.mdentrySummary
Revise the core traits according to @newpavlov's suggestion and similar to #41 but keeping the infallible methods.
No trait renames are done in this PR. Also not sure if we need any.
Details
TryRngCoreis implemented for anyR: DerefMutwhereTarget: TryRngCore.RngCore: TryRngCore<Error = Infallible>with automatic implementation overTryRngCoreUnwrapMutis removedCryptoRng = RngCore + TryCryptoRngor as close as possible without sum traits; it is kept since I believe some people needdyn CryptoRngutils::{next_u64_via_u32, next_word_via_fill}now use closures instead ofR: RngCoreQuestions
Should
utilsfns support potentially-fallible uses (try_next_u64_via_u32takesFnMut() -> Result<u32; E>for genericE)? I think unnecessary.