-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
narrowing_rem, narrowing_and #72762
Comments
Why not the following? let _x: u8 = (152_u64 % 51_u64) as u8; |
Because it contains a naked "as" in user code. |
There is nothing wrong with |
It's just a matter of how you define "unsafe". If you use a narrow definition of memory safety, then you're right. But Rust designers have understood that's a insufficient definition. A cast from floating point to integral value was undefined behaviour. We have patched this problem, leaving a escape hatch (to_int_unchecked). The narrowing cast between two integral values using "as" is well defined, it's not undefined behaviour, but if you write lot of numerical code in Rust you slowly understand that current "as" casts are unsafe and they lead to problems in C code. In Rust there's try_from to avoid this risk (but there isn't a function like try_from that becomes an "as" in release mode only). So a wide coding experience in C/Rust and other languages shows you that reducing the number of naked "as" casts in your user code is a good idea to avoid bugs. That's why I have suggested this narrowing rem that's a hopefully sufficiently common case that's safe (also handled by the D language type system). |
FWIW I agree that |
However @Elinvynia I am a bit surprised to see the "typesystem" label here. This is just a libs addition not affecting the type system, isn't it? |
From my understanding of this request, I thought that it having to do with "type conversions" would be a good reason to include that label, what would be more appropriate here? |
But TryFrom in some performance-sensitive places is heavy. So in my code I use a short to!{} macro that performs the TryFrom in debug mode and uses "as" in release mode. And another little macro for FP=>Int casts, that uses "as" in debug mode and to_int_unchecked in release mode. |
Yeah I can see how that's useful. For Miri, arithmetic is not our limiting factor in terms of performance. In fact I'd prefer our additions etc. to be overflow-checked in release mode as well.
oO okay that's UB when anything goes wrong (i.e., this is an unsafe-to-use macro), I would not accept that in my own codebases without benchmarks for each individual invocation of the macro. ;) But of course, everyone makes their own decisions for these trade-offs. Btw, IIRC people were looking for feedback on cases where the |
Yes, I use this second fto!{} macro only in special cases, and after benchmarking. |
In the meantime I've found about a dozen usage points for this narrowing_rem in my codebase. |
The code could be simplified a bit using Self: fn narrowing_rem(&self, den: u8) -> u8 { (*self % Self::from(den)) as u8 } But now I'm trying to use a (possibly overengineered) more complex version: #![feature(core_intrinsics, const_fn, const_panic)]
use core::intrinsics::assume;
use std::num::{NonZeroU8, NonZeroU16};
trait HasNonZero {
type NonZero;
fn from_nonzero(v: Self::NonZero) -> Self;
}
impl HasNonZero for u8 {
type NonZero = NonZeroU8;
fn from_nonzero(v: Self::NonZero) -> Self { v.get() }
}
impl HasNonZero for u16 {
type NonZero = NonZeroU16;
fn from_nonzero(v: Self::NonZero) -> Self { v.get() }
}
//...
//- - - - - - - - - - - - - - - - - - - -
// To be used at compile-time, it raises a panic. TODO until Option::unwrap becomes const.
const fn nonzero_u8(x: u8) -> NonZeroU8 {
match x {
0 => panic!("nonzero: x == 0"),
_ => unsafe { NonZeroU8::new_unchecked(x) },
}
}
const fn nonzero_u16(x: u16) -> NonZeroU16 {
match x {
0 => panic!("nonzero: x == 0"),
_ => unsafe { NonZeroU16::new_unchecked(x) },
}
}
//...
//- - - - - - - - - - - - - - - - - - - -
trait NarrowRem<Out> where Out: HasNonZero {
fn narrowing_rem(&self, den: Out::NonZero) -> Out;
}
impl NarrowRem<u8> for u32 {
#[inline(always)]
fn narrowing_rem(&self, den: <u8 as HasNonZero>::NonZero) -> u8 {
let divisor = Self::from(u8::from_nonzero(den));
unsafe { assume(divisor != 0); } // TODO, will be unnecessary.
(*self % divisor) as u8
}
}
impl NarrowRem<u8> for usize {
#[inline(always)]
fn narrowing_rem(&self, den: <u8 as HasNonZero>::NonZero) -> u8 {
let divisor = Self::from(u8::from_nonzero(den));
unsafe { assume(divisor != 0); } // TODO, will be unnecessary.
(*self % divisor) as u8
}
}
//... |
nonzero_uX are now useless, we can define NonZeroUX::new().unwrap() in constants. |
Now it can be implemented without unsafe (beside the hard #![feature(const_trait_impl)]
#![allow(incomplete_features)]
use std::num::{NonZeroU8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroUsize, NonZeroU128};
trait HasNonZero {
type NonZero;
fn from_nonzero(v: Self::NonZero) -> Self;
}
macro_rules! has_nonzero_helper {
($T:ty, $NZT:ty) => {
impl const HasNonZero for $T {
type NonZero = $NZT;
fn from_nonzero(v: Self::NonZero) -> Self { v.get() }
}
}
}
has_nonzero_helper!(u8, NonZeroU8);
has_nonzero_helper!(u16, NonZeroU16);
has_nonzero_helper!(u32, NonZeroU32);
has_nonzero_helper!(u64, NonZeroU64);
has_nonzero_helper!(u128, NonZeroU128);
has_nonzero_helper!(usize, NonZeroUsize);
// Performs a rem followed by a safe narrowing cast:
// (T1 % (T2 as T1)) as T2 where sizeof<T2> < sizeof<T1>
trait NarrowRem<Out> where Out: HasNonZero {
fn narrowing_rem(&self, den: Out::NonZero) -> Out;
}
macro_rules! narrow_rem_helper {
($TSelf:ty, $TRem:ty) => {
impl NarrowRem<$TRem> for $TSelf {
#[inline(always)]
fn narrowing_rem(&self, den: <$TRem as HasNonZero>::NonZero) -> $TRem {
(*self % <Self as HasNonZero>::NonZero::from(den)) as _
}
}
}
}
narrow_rem_helper!(u16, u8);
narrow_rem_helper!(u32, u8);
narrow_rem_helper!(u64, u8);
narrow_rem_helper!(u128, u8);
narrow_rem_helper!(usize, u8);
narrow_rem_helper!(u32, u16);
narrow_rem_helper!(u64, u16);
narrow_rem_helper!(u128, u16);
narrow_rem_helper!(usize, u16);
narrow_rem_helper!(u64, u32);
narrow_rem_helper!(u128, u32);
narrow_rem_helper!(u128, u64);
fn main() {} |
A similar |
Chiming in here, what would it take to simply add support for this to the built in |
To remove some unsafe "as" casts and keep the code safe (lossless) and nice, sometimes I'd like to use a rem+cast. So what do you think about adding this to the stdlib?
An example usage:
D language performs this lossless cast operation automatically and transparently (while it doesn't perform lossy casts silently):
The text was updated successfully, but these errors were encountered: