ring icon indicating copy to clipboard operation
ring copied to clipboard

Replace dynamic assertions with static assertions

Open briansmith opened this issue 3 years ago • 3 comments

When reviewing PR #1368, @complexspaces noted:

     const LG_BASE: usize = 2; // Shifts vs. squaring trade-off.
    debug_assert_eq!(LG_BASE.count_ones(), 1); // Must be 2**n for n >= 0.

Since ring has a higher MSRV, this could be replaced with a compile-time check akin to how static-assertions allows values to be checked at compile time. This would entirely remove this runtime overhead for a known-good value.

briansmith avatar Aug 30 '21 18:08 briansmith

    const LG_BASE: usize = 2; // Shifts vs. squaring trade-off.
    debug_assert_eq!(LG_BASE.count_ones(), 1); // Must be 2**n for n >= 0.

In this case, we can also construct LG_BASE to make it more obvious that it is a power of two:

// Shift vs. squaring tradeoff.
const LG_LG_BASE: u32 = 1; 
const LG_BASE: usize = 1 << LG_LG_BASE;

briansmith avatar Aug 30 '21 18:08 briansmith

Basically the tactic to employ here is to look for debug_assert and replace as many as possible with the equivalent const _SOME_DESCRIPTIVE_NAME: () = assert!(...) and/or refactoring the code to make the assertion completely unneeded.

briansmith avatar Jan 13 '24 01:01 briansmith

PR #1892 is an example of this.

briansmith avatar Jan 13 '24 01:01 briansmith