Skip to content

Commit fc370f6

Browse files
committed
core: Make funnel shifts panic only if overflow checks are enabled
Change from the existing behavior of panicking unconditionally to panicking only if debug assertions are enabled, wrapping otherwise. This is more consistent with other operations, and gives slightly better codegen [1]. Suggested in the stabilization PR, RUST-161015. [1]: https://rust.godbolt.org/z/Yz81zhbno
1 parent c83b0a4 commit fc370f6

4 files changed

Lines changed: 58 additions & 8 deletions

File tree

library/core/src/num/uint_macros.rs

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,12 @@ macro_rules! uint_impl {
518518
/// This function will panic if `n` is greater than or equal to the number of
519519
/// bits in `self`.
520520
///
521+
/// ## Overflow behavior
522+
///
523+
/// The above panic happens only if overflow checks are enabled (default in debug mode).
524+
/// With overflow checks disabled (default in release mode), there is no panic; instead,
525+
/// the value is shifted by `n % Self::BITS`.
526+
///
521527
/// # Examples
522528
///
523529
/// ```
@@ -543,21 +549,31 @@ macro_rules! uint_impl {
543549
///
544550
/// ```should_panic
545551
/// #![feature(funnel_shifts)]
552+
/// # #![feature(cfg_overflow_checks)]
553+
/// # #[cfg(overflow_checks)] {
546554
///
547555
#[doc = concat!("let a = ", stringify!($SelfT), "::MAX;")]
548556
/// // Okay
549557
#[doc = concat!("let _ = a.rotate_left(", stringify!($SelfT), "::BITS);")]
550-
/// // Panics
558+
/// // Panics (only when overflow checks are enabled)
551559
#[doc = concat!("let _ = a.funnel_shl(a, ", stringify!($SelfT), "::BITS);")]
560+
/// # }
561+
/// # #[cfg(not(overflow_checks))] panic!("fulfill should_panic");
552562
/// ```
553563
#[rustc_const_unstable(feature = "funnel_shifts", issue = "145686")]
554564
#[unstable(feature = "funnel_shifts", issue = "145686")]
555565
#[must_use = "this returns the result of the operation, without modifying the original"]
556566
#[inline(always)]
567+
#[rustc_inherit_overflow_checks]
557568
pub const fn funnel_shl(self, right: Self, n: u32) -> Self {
558-
assert!(n < Self::BITS, "attempt to funnel shift left with overflow");
559-
// SAFETY: just checked that `shift` is in-range
560-
unsafe { self.unchecked_funnel_shl(right, n) }
569+
if intrinsics::overflow_checks() {
570+
assert!(n < Self::BITS, "attempt to funnel shift left with overflow");
571+
}
572+
// SAFETY: `n` is wrapped to within range
573+
unsafe {
574+
let n = n & (Self::BITS - 1);
575+
self.unchecked_funnel_shl(right, n)
576+
}
561577
}
562578

563579
/// Performs a right funnel shift.
@@ -574,6 +590,12 @@ macro_rules! uint_impl {
574590
/// This function will panic if `n` is greater than or equal to the number of
575591
/// bits in `self`.
576592
///
593+
/// ## Overflow behavior
594+
///
595+
/// The above panic happens only if overflow checks are enabled (default in debug mode).
596+
/// With overflow checks disabled (default in release mode), there is no panic; instead,
597+
/// the value is shifted by `n % Self::BITS`.
598+
///
577599
/// # Examples
578600
///
579601
/// ```
@@ -599,21 +621,31 @@ macro_rules! uint_impl {
599621
///
600622
/// ```should_panic
601623
/// #![feature(funnel_shifts)]
624+
/// # #![feature(cfg_overflow_checks)]
625+
/// # #[cfg(overflow_checks)] {
602626
///
603627
#[doc = concat!("let a = ", stringify!($SelfT), "::MAX;")]
604628
/// // Okay
605629
#[doc = concat!("let _ = a.rotate_right(", stringify!($SelfT), "::BITS);")]
606-
/// // Panics
630+
/// // Panics (only when overflow checks are enabled)
607631
#[doc = concat!("let _ = a.funnel_shr(a, ", stringify!($SelfT), "::BITS);")]
632+
/// # }
633+
/// # #[cfg(not(overflow_checks))] panic!("fulfill should_panic");
608634
/// ```
609635
#[rustc_const_unstable(feature = "funnel_shifts", issue = "145686")]
610636
#[unstable(feature = "funnel_shifts", issue = "145686")]
611637
#[must_use = "this returns the result of the operation, without modifying the original"]
612638
#[inline(always)]
639+
#[rustc_inherit_overflow_checks]
613640
pub const fn funnel_shr(self, right: Self, n: u32) -> Self {
614-
assert!(n < Self::BITS, "attempt to funnel shift right with overflow");
615-
// SAFETY: just checked that `shift` is in-range
616-
unsafe { self.unchecked_funnel_shr(right, n) }
641+
if intrinsics::overflow_checks() {
642+
assert!(n < Self::BITS, "attempt to funnel shift right with overflow");
643+
}
644+
// SAFETY: `n` is wrapped to within range
645+
unsafe {
646+
let n = n & (Self::BITS - 1);
647+
self.unchecked_funnel_shr(right, n)
648+
}
617649
}
618650

619651
/// Unchecked funnel shift left.

library/coretests/tests/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#![feature(borrowed_buf_init)]
1212
#![feature(bstr)]
1313
#![feature(casefold)]
14+
#![feature(cfg_overflow_checks)]
1415
#![feature(cfg_target_has_reliable_f16_f128)]
1516
#![feature(char_internals)]
1617
#![feature(clone_to_uninit)]

library/coretests/tests/num/uint_macros.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,17 +216,31 @@ macro_rules! uint_module {
216216
}
217217

218218
#[test]
219+
#[cfg(overflow_checks)]
219220
#[should_panic = "attempt to funnel shift left with overflow"]
220221
fn test_funnel_shl_overflow() {
221222
let _ = <$T>::funnel_shl(A, B, $T::BITS);
222223
}
223224

224225
#[test]
226+
#[cfg(overflow_checks)]
225227
#[should_panic = "attempt to funnel shift right with overflow"]
226228
fn test_funnel_shr_overflow() {
227229
let _ = <$T>::funnel_shr(A, B, $T::BITS);
228230
}
229231

232+
#[test]
233+
#[cfg(not(overflow_checks))]
234+
fn test_funnel_shl_overflow() {
235+
let _ = <$T>::funnel_shl(A, B, A);
236+
}
237+
238+
#[test]
239+
#[cfg(not(overflow_checks))]
240+
fn test_funnel_shr_overflow() {
241+
let _ = <$T>::funnel_shr(A, B, B);
242+
}
243+
230244
#[test]
231245
fn test_funnel_shifts_runtime() {
232246
for i in 0..$T::BITS - 1 {

tests/ui/std/overflow-check-ops.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//@[WRAP] compile-flags: -C overflow-checks=false
88

99
#![feature(cfg_overflow_checks)]
10+
#![feature(funnel_shifts)]
1011

1112
use std::hint::black_box as bb;
1213
use std::{assert_matches, fmt, panic};
@@ -28,6 +29,8 @@ fn main() {
2829
check(|| bb(u32::MAX) * bb(2), u32::MAX << 1, "mul");
2930
check(|| bb(1u32) << bb(32), 1, "shl");
3031
check(|| bb(u32::MAX) >> bb(32), u32::MAX, "shr");
32+
check(|| bb(1234u32).funnel_shl(4567, bb(32)), 1234, "funnel_shl");
33+
check(|| bb(1234u32).funnel_shr(4567, bb(32)), 4567, "funnel_shr");
3134
check(|| bb(u32::MAX).pow(bb(2)), 1, "pow");
3235
check(|| bb(u32::MAX).next_power_of_two(), 0, "next_power_of_two");
3336

0 commit comments

Comments
 (0)