Open
Description
RustFFT recently had a bug report: ejmahler/RustFFT#74
On the latest nightly, RustFFT no longer compiles due to the following error:
error[E0080]: evaluation of constant value failed
--> C:\Users\x\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\core\src\..\..\stdarch\crates\core_arch\src\macros.rs:8:17
|
8 | let _ = 1 / ((IMM >= MIN && IMM <= MAX) as usize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to divide `1_usize` by zero
A contributor was able to determine that the error was being caused by a _mm_blend_ps
intrinsic:
let blended = _mm_blend_ps(rows[0], rows[2], 0x33);
The fix was to change it to the following, removing unused bits from the imm4 value:
let blended = _mm_blend_ps(rows[0], rows[2], 0x03);
The reporter traced it back to the following code in this repo:
#[allow(unused_macros)]
macro_rules! static_assert_imm4 {
($imm:ident) => {
let _ = $crate::core_arch::macros::ValidateConstImm::<$imm, 0, { (1 << 4) - 1 }>::VALID;
};
}
I was able to trace this static assert back to this PR, although it appears to be part of a larger initiative to apply this static assert to as many intrinsics as possible.
A fix has been submitted to RustFFT, and I'm happy to make my library more correct, but I'm creating this issue to raise 2 items of feedback with this new static assert:
- The error isn't user-friendly. Instead of directly saying what I did wrong, I'm presented with a "secondary error" about dividing by zero. Additionally, if I decide to dig into the core::arch the code to investigate the error, there aren't any comments in the code surrounding this "divide by zero" line explaining the intent, so I can't even do the bare minimum of trying to intuit what I did wrong from reading the library code. Finally, the error is deep the guts of core::arch, and there's no hint of which of RustFFT's thousands of lines are triggering this error.
- If this change makes it into stable Rust, I'm worried that more than just RustFFT will break. I tested rustc 1.52 and rustc beta 1.53, and both accept the RustFFT code. so right now this is a nightly-only issue. Are there any plans to put this static assert change behind an edition flag, to avoid breaking working code?
Metadata
Metadata
Assignees
Labels
No labels