Skip to content

Bump rand, mt19937; fix associated build failures#7800

Closed
joshuamegnauth54 wants to merge 1 commit into
RustPython:mainfrom
joshuamegnauth54:bump-random
Closed

Bump rand, mt19937; fix associated build failures#7800
joshuamegnauth54 wants to merge 1 commit into
RustPython:mainfrom
joshuamegnauth54:bump-random

Conversation

@joshuamegnauth54
Copy link
Copy Markdown
Contributor

@joshuamegnauth54 joshuamegnauth54 commented May 8, 2026

Supercedes: #7716

Bumping rand and mt19937 is not trivial so Dependabot's PR causes build failures.

Update: Draft until I figure out why the WebAssembly build is failing. The rest of it is working, though. I reverted the getrandom bump. Apparently, the new version is conflicting with version 0.3 which is pulled in by other dependencies.

Summary by CodeRabbit

  • Chores

    • Updated random-related dependencies across the workspace to newer, compatible versions.
    • Enabled the JS/random backend and explicitly enabled std+wasm features for WebAssembly targets.
  • Bug Fixes / Behavior Change

    • When no seed is provided, the default RNG now seeds from the platform thread RNG instead of attempting direct OS entropy.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: ffa66430-9157-4346-9448-51e77d524ebc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Workspace RNG deps upgraded (getrandom→0.4, mt19937→3.3, rand→0.10). Stdlib switched from workspace rand_core to rand. random.rs now seeds MT19937 from the thread RNG. WASM target getrandom features added and a test import updated.

Changes

Random Number Generation Dependency Upgrade

Layer / File(s) Summary
Workspace Dependency Upgrades
Cargo.toml
getrandom bumped 0.3→0.4; mt19937 set to 3.3; rand upgraded 0.9→0.10; explicit rand_core entry removed.
Stdlib crate dependency alignment
crates/stdlib/Cargo.toml
Dependency changed from rand_core = { workspace = true } to rand = { workspace = true }; adds a wasm-target getrandom entry.
Random Module Implementation
crates/stdlib/src/random.rs
Imports switched to rand::{Rng, SeedableRng}; PyRandom::seed(None) now uses thread RNG via MT19937::from_rng instead of OS-based try_from_os_rng().
WASM Feature
crates/vm/Cargo.toml, crates/common/Cargo.toml
wasmbind feature now includes getrandom/wasm_js; target-specific deps added for cfg(all(target_arch = "wasm32", target_os = "unknown")) enabling getrandom with ["std","wasm_js"].
Tests
crates/literal/src/float.rs
test_to_hex imports rand::RngExt instead of rand::Rng.

Sequence Diagram(s)

sequenceDiagram
  participant PyRandom
  participant ThreadRNG as thread::rng()
  participant MT as MT19937
  PyRandom->>ThreadRNG: request RNG
  ThreadRNG-->>PyRandom: provide RNG
  PyRandom->>MT: MT19937::from_rng(&mut rng)
  MT-->>PyRandom: seeded MT19937
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • RustPython/RustPython#7747: Modifies stdlib random-related dependencies and wasm/getrandom settings; similar dependency alignment.

Suggested reviewers

  • ShaharNaveh

Poem

🐰 I hopped through Cargo with a hum and twitch,
nudged rand and getrandom up a switch,
seeded MT from the thread's bright song,
set wasm flags and fixed a test along,
tiny paws, big changes — all stitched.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: bumping rand and mt19937 versions and fixing the associated build failures caused by these version updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
Cargo.toml (1)

261-261: 💤 Low value

sys_rng feature is redundant with rand 0.10 defaults.

thread_rng is a default feature of rand 0.10 and already implies sys_rng. Listing sys_rng explicitly does not break anything, but it is unnecessary noise.

🔧 Simplified dependency
-rand = { version = "0.10", features = ["sys_rng"] }
+rand = { version = "0.10" }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Cargo.toml` at line 261, The rand dependency declaration includes an explicit
features entry for "sys_rng" which is redundant with rand 0.10 defaults; update
the Cargo.toml rand entry (the dependency named "rand") by removing the features
= ["sys_rng"] clause so it relies on the default features (e.g., use rand =
"0.10" or remove the features array), keeping the crate name "rand" intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/stdlib/src/random.rs`:
- Line 18: Import the correct trait so SeedableRng::from_rng can use an RngCore
source: replace the current import use rand::{Rng, SeedableRng} with use
rand::{RngCore, SeedableRng} (so MT19937::from_rng / SeedableRng::from_rng has
the required RngCore bound); no changes needed for MT19937::next_u32 since that
is a concrete method.

---

Nitpick comments:
In `@Cargo.toml`:
- Line 261: The rand dependency declaration includes an explicit features entry
for "sys_rng" which is redundant with rand 0.10 defaults; update the Cargo.toml
rand entry (the dependency named "rand") by removing the features = ["sys_rng"]
clause so it relies on the default features (e.g., use rand = "0.10" or remove
the features array), keeping the crate name "rand" intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: e2351995-7f8a-459f-b8a7-0eb1680752b2

📥 Commits

Reviewing files that changed from the base of the PR and between e10a27b and 88a2032.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Cargo.toml
  • crates/stdlib/Cargo.toml
  • crates/stdlib/src/random.rs

use mt19937::MT19937;
use num_traits::{Signed, Zero};
use rand_core::{RngCore, SeedableRng};
use rand::{Rng, SeedableRng};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check what rand/RngCore/Rng methods are called in random.rs
rg -n "\.next_u32\|\.next_u64\|\.fill_bytes\|\.gen\b\|\.random\b\|rand::Rng\|rand::RngCore\|rand_core" crates/stdlib/src/random.rs

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Check if file exists and get the imports section
head -30 crates/stdlib/src/random.rs

Repository: RustPython/RustPython

Length of output: 851


🏁 Script executed:

# Search more broadly for any 'use' statements and RNG variable usage
rg -A 5 -B 5 "use rand" crates/stdlib/src/random.rs

Repository: RustPython/RustPython

Length of output: 387


🏁 Script executed:

# Look for where the RNG object is actually being used
rg -n "rng\|Rng\|RngCore" crates/stdlib/src/random.rs

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Read the full file to understand RNG usage
cat -n crates/stdlib/src/random.rs | head -100

Repository: RustPython/RustPython

Length of output: 4288


🏁 Script executed:

# Search for method calls on 'rng' variable without escaping
rg -n "\brng\." crates/stdlib/src/random.rs

Repository: RustPython/RustPython

Length of output: 530


🏁 Script executed:

# Check what MT19937 implements - search documentation or trait bounds
rg -n "MT19937" crates/stdlib/src/random.rs | head -20

Repository: RustPython/RustPython

Length of output: 249


🏁 Script executed:

# Check if there are any compiler errors or clippy warnings in the repo
# Try to find cargo check or build output
cd crates/stdlib && cargo check 2>&1 | grep -A 5 -B 5 "random\|Rng\|error\|warning" | head -50

Repository: RustPython/RustPython

Length of output: 151


🏁 Script executed:

# Search for MT19937::from_rng usage and documentation in mt19937 crate
rg -A 2 "from_rng" crates/stdlib/src/random.rs

Repository: RustPython/RustPython

Length of output: 160


🏁 Script executed:

# Check what trait MT19937 implements by searching for impl statements
rg "impl.*MT19937\|impl.*for.*MT19937" --type rust

Repository: RustPython/RustPython

Length of output: 47


🌐 Web query:

mt19937 Rust crate from_rng method parameter trait

💡 Result:

In the rand_core-based Rust ecosystem, the MT19937 generator’s “from_rng” style constructor is exposed via the rand_core::SeedableRng trait as: fn from_rng<R: RngCore>(rng: R) -> Result<Self, Error> So the parameter is a type parameter R constrained by rand_core::RngCore (an RNG producing values without error), passed by value (rng: R) and returns Result<Self, Error>. This is the generic trait signature in rand_core’s SeedableRng: [1]. Common related (infallible) variant in rand_mt: rand_mt’s Mt implements a method with signature: fn from_rng(rng: &mut R) -> Self where R: Rng +?Sized Here the parameter is &mut R, where R: rand::Rng (the infallible high-level RNG trait), and the method returns Self directly. [2] For the “mt19937” crate specifically (coolreader18/hiroshima-univ translation), its source shows implementations of rand_core::TryRng and rand_core::SeedableRng for MT19937 (but the exact from_rng signature there follows the rand_core::SeedableRng trait; the trait provides from_rng generically). [3][4] Therefore, when you ask “mt19937 Rust crate from_rng method parameter trait”, the trait bound for from_rng is R: RngCore in rand_core::SeedableRng. [1] If instead you are referring to rand_mt’s Mt::from_rng convenience API, the parameter uses &mut R where R: rand::Rng +?Sized. [2]

Citations:


🏁 Script executed:

# Check if RngCore is imported elsewhere or re-exported
rg "RngCore\|rand_core" crates/stdlib/src/random.rs

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Check if there's a re-export of RngCore in the rand crate imports
rg "use rand" crates/stdlib/src/random.rs -A 1 -B 1

Repository: RustPython/RustPython

Length of output: 185


🏁 Script executed:

# Try to check Cargo.toml to see rand version and features
cat crates/stdlib/Cargo.toml | grep -A 5 -B 5 "rand"

Repository: RustPython/RustPython

Length of output: 325


🏁 Script executed:

# Search for any build output or error logs
find . -name "*.log" -type f 2>/dev/null | head -5

Repository: RustPython/RustPython

Length of output: 47


Replace Rng with RngCore in the import statement.

The MT19937::from_rng() call at line 62 uses the SeedableRng::from_rng() trait method, which requires a generic parameter bound by RngCore, not Rng. The current import (use rand::{Rng, SeedableRng}) is missing RngCore and cannot satisfy the trait bound for from_rng(). While next_u32() at line 82 is a concrete method on MT19937 itself (not requiring a trait import), the trait-based from_rng() method cannot be properly used without RngCore in scope.

Proposed fix
-    use rand::{Rng, SeedableRng};
+    use rand_core::RngCore;
+    use rand::SeedableRng;

or equivalently:

-    use rand::{Rng, SeedableRng};
+    use rand::{SeedableRng};
+    use rand_core::RngCore;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/random.rs` at line 18, Import the correct trait so
SeedableRng::from_rng can use an RngCore source: replace the current import use
rand::{Rng, SeedableRng} with use rand::{RngCore, SeedableRng} (so
MT19937::from_rng / SeedableRng::from_rng has the required RngCore bound); no
changes needed for MT19937::next_u32 since that is a concrete method.

@joshuamegnauth54 joshuamegnauth54 force-pushed the bump-random branch 5 times, most recently from d25758a to a60c45b Compare May 8, 2026 01:56
@joshuamegnauth54 joshuamegnauth54 marked this pull request as draft May 8, 2026 02:21
@joshuamegnauth54 joshuamegnauth54 force-pushed the bump-random branch 2 times, most recently from e363af0 to ef88086 Compare May 8, 2026 17:04
@joshuamegnauth54 joshuamegnauth54 changed the title Bump getrandom, rand, mt19937 Bump rand, mt19937; fix associated build failures May 8, 2026
@joshuamegnauth54
Copy link
Copy Markdown
Contributor Author

tkaitchuck/aHash#274

Seems relevant to the getrandom + WebAssembly issues.

@joshuamegnauth54 joshuamegnauth54 force-pushed the bump-random branch 3 times, most recently from 149112d to 723e4ae Compare May 8, 2026 18:33
Supercedes: RustPython#7716

Bumping rand and mt19937 is not trivial, so Dependabot's PR causes build
failures.

`getrandom` cannot be bumped yet. RustPython's crate graph has several
old versions of `getrandom`. Apparently, this is causing a conflict with
the WebAssembly build.
@joshuamegnauth54
Copy link
Copy Markdown
Contributor Author

Uh, I give up on this for now. 😓

@joshuamegnauth54 joshuamegnauth54 deleted the bump-random branch May 9, 2026 17:02
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.

1 participant