Conversation
|
I am in favor of bumping MSRV and releasing v0.4 (see #722) regardless of whether (I will wait for CI to be fixed before approving) |
| matrix: | ||
| os: [ubuntu-24.04, windows-2022] | ||
| toolchain: [nightly, beta, stable, "1.63"] | ||
| toolchain: [nightly, beta, stable, "1.85"] |
There was a problem hiding this comment.
Ideally, we should extract MSRV from the rust-version field instead of manually providing it.
|
Releasing v0.4 works for me. Okay, I'll try to get this fixed. |
|
So yesterday CI showed this error: Today after adding This is using nightly rustc. So why does it not think unsafe is necessary here? |
|
Note: |
| #[target_feature(enable = "rdrand")] | ||
| #[cfg_attr(target_os = "uefi", allow(unused_unsafe))] // HACK: Rust lint gives false positive on uefi | ||
| unsafe fn rdrand() -> Option<Word> { | ||
| for _ in 0..RETRY_LIMIT { | ||
| let mut val = 0; | ||
| if rdrand_step(&mut val) == 1 { | ||
| if unsafe { rdrand_step(&mut val) } == 1 { | ||
| return Some(val); | ||
| } | ||
| } |
There was a problem hiding this comment.
Got a better idea?
At least allow(unused_unsafe) is fairly harmless.
There was a problem hiding this comment.
It's not quite a false positive... On Nightly the RDRAND intrinsics are marked as safe (e.g. see here). I am not sure why it's still marked as unsafe on the stable toolchain, since most intrinsics were changed to safe in 1.87 (after 1.86 relaxed unsafety requirements on functions marked with #[target_feature(enable="..")]).
For now, I would just mark the whole function with allow(unused_unsafe) without gating it on the target with a comment stating that in new Rust versions RDRAND intrinsics are safe.
newpavlov
left a comment
There was a problem hiding this comment.
we may be able to drop the std feature after this
We also use std for the From<Error> for io::Error impl and in the fmt::Debug/Display impls.
I don't have a strong opinion on whether we should keep them or remove the std feature completely.
| ### Added | ||
| - `RawOsError` type alias [#739] | ||
|
|
||
| ### Changes |
There was a problem hiding this comment.
| ### Changes | |
| ### Changed |
The MSRV bump is required if we want to make
getrandomdepend onrand_core(see rust-random/rand#1675). Unless we use a split MSRV depending on whetherrand_coreis a dependency.It is however a significant bump, so we might prefer not to.