Skip to content

Commit 988ecca

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 988ecca

4 files changed

Lines changed: 60 additions & 12 deletions

File tree

‎library/core/src/num/uint_macros.rs‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -515,8 +515,13 @@ macro_rules! uint_impl {
515515
///
516516
/// # Panics
517517
///
518-
/// This function will panic if `n` is greater than or equal to the number of
519-
/// bits in `self`.
518+
/// ## Overflow behavior
519+
///
520+
/// If overflow checks are enabled (default in debug mode), this function will panic if `n`
521+
/// is greater than or equal to the number of bits in `self`. If overflow checks are
522+
/// disabled (default in release mode), there is no panic; instead, the value is shifted
523+
/// by `n % Self::BITS`.
524+
// FIXME(wrapping_funnel_shifts): link to `wrapping_funnel_shl` when stable.
520525
///
521526
/// # Examples
522527
///
@@ -543,21 +548,31 @@ macro_rules! uint_impl {
543548
///
544549
/// ```should_panic
545550
/// #![feature(funnel_shifts)]
551+
/// # #![feature(cfg_overflow_checks)]
552+
/// # #[cfg(overflow_checks)] {
546553
///
547554
#[doc = concat!("let a = ", stringify!($SelfT), "::MAX;")]
548555
/// // Okay
549556
#[doc = concat!("let _ = a.rotate_left(", stringify!($SelfT), "::BITS);")]
550-
/// // Panics
557+
/// // Panics (only when overflow checks are enabled)
551558
#[doc = concat!("let _ = a.funnel_shl(a, ", stringify!($SelfT), "::BITS);")]
559+
/// # }
560+
/// # #[cfg(not(overflow_checks))] panic!("fulfill should_panic");
552561
/// ```
553562
#[rustc_const_unstable(feature = "funnel_shifts", issue = "145686")]
554563
#[unstable(feature = "funnel_shifts", issue = "145686")]
555564
#[must_use = "this returns the result of the operation, without modifying the original"]
556565
#[inline(always)]
566+
#[rustc_inherit_overflow_checks]
557567
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) }
568+
if intrinsics::overflow_checks() {
569+
assert!(n < Self::BITS, "attempt to funnel shift left with overflow");
570+
}
571+
// SAFETY: `n` is wrapped to within range
572+
unsafe {
573+
let n = n & (Self::BITS - 1);
574+
self.unchecked_funnel_shl(right, n)
575+
}
561576
}
562577

563578
/// Performs a right funnel shift.
@@ -571,8 +586,13 @@ macro_rules! uint_impl {
571586
///
572587
/// # Panics
573588
///
574-
/// This function will panic if `n` is greater than or equal to the number of
575-
/// bits in `self`.
589+
/// ## Overflow behavior
590+
///
591+
/// If overflow checks are enabled (default in debug mode), this function will panic if `n`
592+
/// is greater than or equal to the number of bits in `self`. If overflow checks are
593+
/// disabled (default in release mode), there is no panic; instead, the value is shifted
594+
/// by `n % Self::BITS`.
595+
// FIXME(wrapping_funnel_shifts): link to `wrapping_funnel_shr` when stable.
576596
///
577597
/// # Examples
578598
///
@@ -599,21 +619,31 @@ macro_rules! uint_impl {
599619
///
600620
/// ```should_panic
601621
/// #![feature(funnel_shifts)]
622+
/// # #![feature(cfg_overflow_checks)]
623+
/// # #[cfg(overflow_checks)] {
602624
///
603625
#[doc = concat!("let a = ", stringify!($SelfT), "::MAX;")]
604626
/// // Okay
605627
#[doc = concat!("let _ = a.rotate_right(", stringify!($SelfT), "::BITS);")]
606-
/// // Panics
628+
/// // Panics (only when overflow checks are enabled)
607629
#[doc = concat!("let _ = a.funnel_shr(a, ", stringify!($SelfT), "::BITS);")]
630+
/// # }
631+
/// # #[cfg(not(overflow_checks))] panic!("fulfill should_panic");
608632
/// ```
609633
#[rustc_const_unstable(feature = "funnel_shifts", issue = "145686")]
610634
#[unstable(feature = "funnel_shifts", issue = "145686")]
611635
#[must_use = "this returns the result of the operation, without modifying the original"]
612636
#[inline(always)]
637+
#[rustc_inherit_overflow_checks]
613638
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) }
639+
if intrinsics::overflow_checks() {
640+
assert!(n < Self::BITS, "attempt to funnel shift right with overflow");
641+
}
642+
// SAFETY: `n` is wrapped to within range
643+
unsafe {
644+
let n = n & (Self::BITS - 1);
645+
self.unchecked_funnel_shr(right, n)
646+
}
617647
}
618648

619649
/// 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)