Skip to content

Conversation

@allan2
Copy link
Contributor

@allan2 allan2 commented Nov 4, 2025

This bumps rand_core to 0.10.0-rc-2 (2025-10-31).

clippy and fmt are clean.

Comment on lines 110 to 113
use rand_core::{
le::{self},
RngCore,
};
Copy link
Member

Choose a reason for hiding this comment

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

This can be reformatted (delete ::{self}).

Comment on lines 100 to 114
#[inline]
fn from_rng(rng: &mut impl RngCore) -> Self {
fn from_rng<R>(rng: &mut R) -> Self
where
R: RngCore + ?Sized,
{
Hc128Rng(BlockRng::<Hc128Core>::from_rng(rng))
}

#[inline]
fn try_from_rng<R: TryRngCore>(rng: &mut R) -> Result<Self, R::Error> {
fn try_from_rng<R>(rng: &mut R) -> Result<Self, R::Error>
where
R: TryRngCore + ?Sized,
{
BlockRng::<Hc128Core>::try_from_rng(rng).map(Hc128Rng)
}
Copy link
Member

Choose a reason for hiding this comment

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

The {try_}from_rng implementations can be deleted.

allan2 added a commit to allan2/rngs that referenced this pull request Nov 5, 2025
allan2 added a commit to allan2/rngs that referenced this pull request Nov 5, 2025
@allan2
Copy link
Contributor Author

allan2 commented Nov 5, 2025

PR updated!

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

You can also delete the from_rng one-line implementations.

@dhardy
Copy link
Member

dhardy commented Nov 5, 2025

Also looks like the Isaac RNGs do need those try_from_rng / from_rng methods.

allan2 added a commit to allan2/rngs that referenced this pull request Nov 5, 2025
@allan2
Copy link
Contributor Author

allan2 commented Nov 5, 2025

Also looks like the Isaac RNGs do need those try_from_rng / from_rng methods.

PR updated. Thanks for the patience.

I'm not sure if from_rng/try_from_rng in impl SeedableRng for IsaacRng is really needed or if it was just to satisfy the failing test.

rand_isaac/src/isaac.rs

#[test]
fn test_isaac_construction() {
    let seed = [
        1, 0, 0, 0, 23, 0, 0, 0, 200, 1, 0, 0, 210, 30, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
        0, 0, 0, 0, 0,
    ];
    let mut rng1 = IsaacRng::from_seed(seed);
    assert_eq!(rng1.next_u32(), 2869442790);

    // do we need this?
    let mut rng2 = IsaacRng::from_rng(&mut rng1);
    assert_eq!(rng2.next_u32(), 3094074039);
}

@dhardy
Copy link
Member

dhardy commented Nov 5, 2025

See the impl of SeedableRng for IsaacCore: that has specific impls of {try_}from_rng. The ones you restored wrap those (otherwise from_seed would be used).

@dhardy
Copy link
Member

dhardy commented Nov 5, 2025

The benchmarks are harder to update since from_os_rng and OsRng have been removed, so this is blocked by rust-random/rand#1675.

@tarcieri
Copy link

tarcieri commented Nov 7, 2025

Perhaps the benchmarks could initialize from a static seed rather than from_os_rng in order to unblock this?

@tarcieri
Copy link

tarcieri commented Nov 7, 2025

It also looks like the MSRV needs to be bumped from 1.63 to 1.85

@dhardy
Copy link
Member

dhardy commented Nov 8, 2025

I updated the MSRV in #73.

@tarcieri
Copy link

tarcieri commented Nov 8, 2025

@allan2 can you rebase/merge to pick up the MSRV change?

@allan2
Copy link
Contributor Author

allan2 commented Nov 10, 2025

@allan2 can you rebase/merge to pick up the MSRV change?

Done

Comment on lines 19 to 21
<<<<<<< HEAD
<<<<<<< HEAD
<<<<<<< HEAD

Choose a reason for hiding this comment

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

Looks like some merge conflicts

@tarcieri
Copy link

Looks like it's just the benchmarks that need updated now. Can the os_rng dependency be removed and replaced with a static seed?

(this PR is blocking updating one of my dependencies, so I'm very excited to see it land)

@dhardy
Copy link
Member

dhardy commented Nov 10, 2025

Yes, it's just benchmarks. I can fix those in a new PR.

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

Thanks @allan2

@dhardy dhardy marked this pull request as ready for review November 10, 2025 14:53
@dhardy dhardy merged commit a2ca99b into rust-random:master Nov 10, 2025
11 of 12 checks passed
@dhardy dhardy changed the title Use rand 0.10 Use rand_core 0.10.0-rc-2 Nov 10, 2025
@tarcieri
Copy link

@dhardy would you mind cutting some prereleases of these? I specifically need rand_xorshift

@dhardy
Copy link
Member

dhardy commented Nov 10, 2025

@tarcieri lets get #75 merged first.

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