From 33b74618b673a3c9b9072ecf7f880a98435b7f82 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 23 Sep 2019 20:40:07 -0400 Subject: [PATCH 1/3] Debug-panic in clamp_min/max if min/max is NAN This also improves the docs for `clamp`, `clamp_min`, and `clamp_max`. --- src/lib.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 1bc9e7b..7146002 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -364,6 +364,8 @@ float_trait_impl!(Num for f32 f64); /// If input is less than min then this returns min. /// If input is greater than max then this returns max. /// Otherwise this returns input. +/// +/// **Panics** in debug mode if `!(min <= max)`. #[inline] pub fn clamp(input: T, min: T, max: T) -> T { debug_assert!(min <= max, "min must be less than or equal to max"); @@ -381,8 +383,11 @@ pub fn clamp(input: T, min: T, max: T) -> T { /// If input is less than min then this returns min. /// Otherwise this returns input. /// `clamp_min(std::f32::NAN, 1.0)` preserves `NAN` different from `f32::min(std::f32::NAN, 1.0)`. +/// +/// **Panics** in debug mode if `!(min == min)`. (This occurs if `min` is `NAN`.) #[inline] pub fn clamp_min(input: T, min: T) -> T { + debug_assert!(min == min, "min must not be NAN"); if input < min { min } else { @@ -395,8 +400,11 @@ pub fn clamp_min(input: T, min: T) -> T { /// If input is greater than max then this returns max. /// Otherwise this returns input. /// `clamp_max(std::f32::NAN, 1.0)` preserves `NAN` different from `f32::max(std::f32::NAN, 1.0)`. +/// +/// **Panics** in debug mode if `!(max == max)`. (This occurs if `max` is `NAN`.) #[inline] pub fn clamp_max(input: T, max: T) -> T { + debug_assert!(max == max, "max must not be NAN"); if input > max { max } else { @@ -428,6 +436,28 @@ fn clamp_test() { assert!(clamp_max(::core::f32::NAN, 1.0).is_nan()); } +#[test] +fn clamp_nan_bound() { + /// When debug assertions are enabled, checks that the expression panics. + macro_rules! assert_debug_panics { + ($body:expr) => { + if cfg!(debug_assertions) { + if let Ok(v) = ::std::panic::catch_unwind(|| $body) { + panic!( + "assertion failed: should_panic; non-panicking result: {:?}", + v + ); + } + } + }; + } + assert_debug_panics!(clamp(0., ::core::f32::NAN, 1.)); + assert_debug_panics!(clamp(0., -1., ::core::f32::NAN)); + assert_debug_panics!(clamp(0., ::core::f32::NAN, ::core::f32::NAN)); + assert_debug_panics!(clamp_min(0., ::core::f32::NAN)); + assert_debug_panics!(clamp_max(0., ::core::f32::NAN)); +} + #[test] fn from_str_radix_unwrap() { // The Result error must impl Debug to allow unwrap() From d02f1667652fd26b64514d02d79a10892c96c805 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 23 Sep 2019 22:12:35 -0400 Subject: [PATCH 2/3] Restrict panic testing to when std is enabled --- src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7146002..28855f4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -438,10 +438,12 @@ fn clamp_test() { #[test] fn clamp_nan_bound() { - /// When debug assertions are enabled, checks that the expression panics. + /// When debug assertions and the `std` feature are enabled, checks that + /// the expression panics. macro_rules! assert_debug_panics { ($body:expr) => { - if cfg!(debug_assertions) { + #[cfg(all(debug_assertions, feature = "std"))] + { if let Ok(v) = ::std::panic::catch_unwind(|| $body) { panic!( "assertion failed: should_panic; non-panicking result: {:?}", From 987ed8fd3891823ef8d441b54b6dd3643cd8330a Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 23 Sep 2019 22:21:33 -0400 Subject: [PATCH 3/3] Split clamp panicking test into separate tests --- src/lib.rs | 53 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 28855f4..73fcdb3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -437,27 +437,38 @@ fn clamp_test() { } #[test] -fn clamp_nan_bound() { - /// When debug assertions and the `std` feature are enabled, checks that - /// the expression panics. - macro_rules! assert_debug_panics { - ($body:expr) => { - #[cfg(all(debug_assertions, feature = "std"))] - { - if let Ok(v) = ::std::panic::catch_unwind(|| $body) { - panic!( - "assertion failed: should_panic; non-panicking result: {:?}", - v - ); - } - } - }; - } - assert_debug_panics!(clamp(0., ::core::f32::NAN, 1.)); - assert_debug_panics!(clamp(0., -1., ::core::f32::NAN)); - assert_debug_panics!(clamp(0., ::core::f32::NAN, ::core::f32::NAN)); - assert_debug_panics!(clamp_min(0., ::core::f32::NAN)); - assert_debug_panics!(clamp_max(0., ::core::f32::NAN)); +#[should_panic] +#[cfg(debug_assertions)] +fn clamp_nan_min() { + clamp(0., ::core::f32::NAN, 1.); +} + +#[test] +#[should_panic] +#[cfg(debug_assertions)] +fn clamp_nan_max() { + clamp(0., -1., ::core::f32::NAN); +} + +#[test] +#[should_panic] +#[cfg(debug_assertions)] +fn clamp_nan_min_max() { + clamp(0., ::core::f32::NAN, ::core::f32::NAN); +} + +#[test] +#[should_panic] +#[cfg(debug_assertions)] +fn clamp_min_nan_min() { + clamp_min(0., ::core::f32::NAN); +} + +#[test] +#[should_panic] +#[cfg(debug_assertions)] +fn clamp_max_nan_max() { + clamp_max(0., ::core::f32::NAN); } #[test]