Fix _mm_extract_ps example. #1261
Conversation
|
r? @Amanieu (rust-highfive has picked a reviewer for you, use r? to override) |
crates/core_arch/src/x86/sse41.rs
Outdated
| /// # unsafe fn worker() { | ||
| /// let mut float_store = vec![1.0, 1.0, 2.0, 3.0]; | ||
| /// unsafe { | ||
| /// let simd_floats = _mm_set_ps(2.5, 5.0, 7.5, 10.0); | ||
| /// let x: i32 = _mm_extract_ps::<2>(simd_floats); | ||
| /// float_store.push(f32::from_bits(x as u32)); | ||
| /// } | ||
| /// assert_eq!(float_store, vec![1.0, 1.0, 2.0, 3.0, 5.0]); | ||
| /// # } | ||
| /// # unsafe { worker() } |
There was a problem hiding this comment.
This appears "unused" from inside the source, but not to the user reading the docs, due to the unsafe "worker" function (that is required to use target_feature(enable)) it is nested inside, so if a user copies this code from the rendered docs with this change, they will need to readd the unsafe { }. That is to say: this makes the documentation less correct. This is an instance of unused_unsafe only because fn worker is not annotated #[warn(unsafe_op_in_unsafe_fn)], which should also eliminate the warning, and without altering the correctness of the documentation.
There was a problem hiding this comment.
Yea, I understand why it was done. I was just changing it to match the style of all the other examples. I certainly could allow the warning, but then it would be inconsistent.
My impression is that anyone using low-level SIMD instructions knows that they will need to be unsafe. Also, if they are pasting the example into an unsafe function, they will end up with the same warning if they include an unsafe block.
EDIT: Though I am fine with allowing the warning if that's what is wanted.
ehuss
force-pushed
the
fix-unsafe-mm_extract_ps
branch
from
7e2ba27
to
4681b9f
Compare
|
CI is blocked on rust-lang/rust#91381. |
|
Pardon me bumming in like this, but should this PR even be needed in face of https://rust-lang.github.io/rfcs/2585-unsafe-block-in-unsafe-fn.html ? Wouldn't the better fix be adding |
The example fails when run with rustdoc since this crate denies warnings. The test has an unneeded
unsafeblock which triggersunused_unsafe.This normally doesn't show up in CI since it requires testing with a
-C target-cputhat has sse4.1.