Skip to content

improve the TryFrom implementations #43248

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 25, 2017
Merged

improve the TryFrom implementations #43248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
215 changes: 177 additions & 38 deletions src/libcore/num/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2504,76 +2504,209 @@ impl fmt::Display for TryFromIntError {
}
}

macro_rules! same_sign_try_from_int_impl {
($storage:ty, $target:ty, $($source:ty),*) => {$(
// no possible bounds violation
macro_rules! try_from_unbounded {
($source:ty, $($target:ty),*) => {$(
#[unstable(feature = "try_from", issue = "33417")]
impl TryFrom<$source> for $target {
type Error = TryFromIntError;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this can never fail, shouldn't this be an empty type like enum NeverIntError {}?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh-- but I see there are cfg'd impls of this for usize based on the current target's pointer width, so those would have to be kept as TryFromIntError in order to ensure target compatibility.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the infallible ones are moving to a blanket impl, per #33417 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep-- I was suggesting enum NeverIntError {} so that this wouldn't be blocked on never_type. Once never_type and the blanket impl land, NeverIntError can be changed to an alias for !.


#[inline]
fn try_from(u: $source) -> Result<$target, TryFromIntError> {
let min = <$target as FromStrRadixHelper>::min_value() as $storage;
let max = <$target as FromStrRadixHelper>::max_value() as $storage;
if u as $storage < min || u as $storage > max {
Err(TryFromIntError(()))
} else {
Ok(u as $target)
}
}
)*}
}

// only negative bounds
macro_rules! try_from_lower_bounded {
($source:ty, $($target:ty),*) => {$(
#[unstable(feature = "try_from", issue = "33417")]
impl TryFrom<$source> for $target {
type Error = TryFromIntError;

#[inline]
fn try_from(u: $source) -> Result<$target, TryFromIntError> {
if u >= 0 {
Ok(u as $target)
} else {
Err(TryFromIntError(()))
}
}
}
)*}
}

same_sign_try_from_int_impl!(u128, u8, u8, u16, u32, u64, u128, usize);
same_sign_try_from_int_impl!(i128, i8, i8, i16, i32, i64, i128, isize);
same_sign_try_from_int_impl!(u128, u16, u8, u16, u32, u64, u128, usize);
same_sign_try_from_int_impl!(i128, i16, i8, i16, i32, i64, i128, isize);
same_sign_try_from_int_impl!(u128, u32, u8, u16, u32, u64, u128, usize);
same_sign_try_from_int_impl!(i128, i32, i8, i16, i32, i64, i128, isize);
same_sign_try_from_int_impl!(u128, u64, u8, u16, u32, u64, u128, usize);
same_sign_try_from_int_impl!(i128, i64, i8, i16, i32, i64, i128, isize);
same_sign_try_from_int_impl!(u128, u128, u8, u16, u32, u64, u128, usize);
same_sign_try_from_int_impl!(i128, i128, i8, i16, i32, i64, i128, isize);
same_sign_try_from_int_impl!(u128, usize, u8, u16, u32, u64, u128, usize);
same_sign_try_from_int_impl!(i128, isize, i8, i16, i32, i64, i128, isize);

macro_rules! cross_sign_from_int_impl {
($unsigned:ty, $($signed:ty),*) => {$(
// unsigned to signed (only positive bound)
macro_rules! try_from_upper_bounded {
($source:ty, $($target:ty),*) => {$(
#[unstable(feature = "try_from", issue = "33417")]
impl TryFrom<$unsigned> for $signed {
impl TryFrom<$source> for $target {
type Error = TryFromIntError;

fn try_from(u: $unsigned) -> Result<$signed, TryFromIntError> {
let max = <$signed as FromStrRadixHelper>::max_value() as u128;
if u as u128 > max {
#[inline]
fn try_from(u: $source) -> Result<$target, TryFromIntError> {
if u > (<$target>::max_value() as $source) {
Err(TryFromIntError(()))
} else {
Ok(u as $signed)
Ok(u as $target)
}
}
}
)*}
}

// all other cases
macro_rules! try_from_both_bounded {
($source:ty, $($target:ty),*) => {$(
#[unstable(feature = "try_from", issue = "33417")]
impl TryFrom<$signed> for $unsigned {
impl TryFrom<$source> for $target {
type Error = TryFromIntError;

fn try_from(u: $signed) -> Result<$unsigned, TryFromIntError> {
let max = <$unsigned as FromStrRadixHelper>::max_value() as u128;
if u < 0 || u as u128 > max {
#[inline]
fn try_from(u: $source) -> Result<$target, TryFromIntError> {
let min = <$target>::min_value() as $source;
let max = <$target>::max_value() as $source;
if u < min || u > max {
Err(TryFromIntError(()))
} else {
Ok(u as $unsigned)
Ok(u as $target)
}
}
}
)*}
}

cross_sign_from_int_impl!(u8, i8, i16, i32, i64, i128, isize);
cross_sign_from_int_impl!(u16, i8, i16, i32, i64, i128, isize);
cross_sign_from_int_impl!(u32, i8, i16, i32, i64, i128, isize);
cross_sign_from_int_impl!(u64, i8, i16, i32, i64, i128, isize);
cross_sign_from_int_impl!(u128, i8, i16, i32, i64, i128, isize);
cross_sign_from_int_impl!(usize, i8, i16, i32, i64, i128, isize);
macro_rules! rev {
($mac:ident, $source:ty, $($target:ty),*) => {$(
$mac!($target, $source);
)*}
}

/// intra-sign conversions
try_from_unbounded!(u8, u8, u16, u32, u64, u128);
try_from_unbounded!(u16, u16, u32, u64, u128);
try_from_unbounded!(u32, u32, u64, u128);
try_from_unbounded!(u64, u64, u128);
try_from_unbounded!(u128, u128);
try_from_upper_bounded!(u16, u8);
try_from_upper_bounded!(u32, u16, u8);
try_from_upper_bounded!(u64, u32, u16, u8);
try_from_upper_bounded!(u128, u64, u32, u16, u8);

try_from_unbounded!(i8, i8, i16, i32, i64, i128);
try_from_unbounded!(i16, i16, i32, i64, i128);
try_from_unbounded!(i32, i32, i64, i128);
try_from_unbounded!(i64, i64, i128);
try_from_unbounded!(i128, i128);
try_from_both_bounded!(i16, i8);
try_from_both_bounded!(i32, i16, i8);
try_from_both_bounded!(i64, i32, i16, i8);
try_from_both_bounded!(i128, i64, i32, i16, i8);

// unsigned-to-signed
try_from_unbounded!(u8, i16, i32, i64, i128);
try_from_unbounded!(u16, i32, i64, i128);
try_from_unbounded!(u32, i64, i128);
try_from_unbounded!(u64, i128);
try_from_upper_bounded!(u8, i8);
try_from_upper_bounded!(u16, i8, i16);
try_from_upper_bounded!(u32, i8, i16, i32);
try_from_upper_bounded!(u64, i8, i16, i32, i64);
try_from_upper_bounded!(u128, i8, i16, i32, i64, i128);

// signed-to-unsigned
try_from_lower_bounded!(i8, u8, u16, u32, u64, u128);
try_from_lower_bounded!(i16, u16, u32, u64, u128);
try_from_lower_bounded!(i32, u32, u64, u128);
try_from_lower_bounded!(i64, u64, u128);
try_from_lower_bounded!(i128, u128);
try_from_both_bounded!(i16, u8);
try_from_both_bounded!(i32, u16, u8);
try_from_both_bounded!(i64, u32, u16, u8);
try_from_both_bounded!(i128, u64, u32, u16, u8);

#[unstable(feature = "try_from", issue = "33417")]
pub use self::ptr_try_from_impls::*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for? You can't (and don't need to) reexport impls.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? Because the pub makes them visible where the mods are not. I deem them implementation detail, so I chose to keep them private.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impls are always visible, even if they're in private modules.


#[cfg(target_pointer_width = "16")]
mod ptr_try_from_impls {
use super::TryFromIntError;
use convert::TryFrom;

try_from_upper_bounded!(usize, u8);
try_from_unbounded!(usize, usize, u16, u32, u64, u128);
try_from_upper_bounded!(usize, i8, i16, isize);
try_from_unbounded!(usize, i32, i64, i128);

try_from_both_bounded!(isize, u8);
try_from_lower_bounded!(isize, usize, u16, u32, u64, u128);
try_from_both_bounded!(isize, i8);
try_from_unbounded!(isize, i16, i32, i64, i128);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it's missing isize to isize. It might be clearer to put the following outside these target specific modules:

try_from_unbounded!(usize, usize);
try_from_upper_bounded!(usize, isize);
try_from_lower_bounded!(isize, usize);
try_from_unbounded!(isize, isize);


rev!(try_from_unbounded, usize, u8, u16);
rev!(try_from_upper_bounded, usize, u32, u64, u128);
rev!(try_from_lower_bounded, usize, i8, i16);
rev!(try_from_both_bounded, usize, i32, i64, i128);

rev!(try_from_unbounded, isize, u8);
rev!(try_from_upper_bounded, isize, u16, u32, u64, u128);
rev!(try_from_unbounded, isize, i8, i16);
rev!(try_from_both_bounded, isize, i32, i64, i128);
}

#[cfg(target_pointer_width = "32")]
mod ptr_try_from_impls {
use super::TryFromIntError;
use convert::TryFrom;

try_from_upper_bounded!(usize, u8, u16);
try_from_unbounded!(usize, usize, u32, u64, u128);
try_from_upper_bounded!(usize, i8, i16, i32, isize);
try_from_unbounded!(usize, i64, i128);

try_from_both_bounded!(isize, u8, u16);
try_from_lower_bounded!(isize, usize, u32, u64, u128);
try_from_both_bounded!(isize, i8, i16);
try_from_unbounded!(isize, i32, i64, i128);

rev!(try_from_unbounded, usize, u8, u16, u32);
rev!(try_from_upper_bounded, usize, u64, u128);
rev!(try_from_lower_bounded, usize, i8, i16, i32);
rev!(try_from_both_bounded, usize, i64, i128);

rev!(try_from_unbounded, isize, u8, u16);
rev!(try_from_upper_bounded, isize, u32, u64, u128);
rev!(try_from_unbounded, isize, i8, i16, i32);
rev!(try_from_both_bounded, isize, i64, i128);
}

#[cfg(target_pointer_width = "64")]
mod ptr_try_from_impls {
use super::TryFromIntError;
use convert::TryFrom;

try_from_upper_bounded!(usize, u8, u16, u32);
try_from_unbounded!(usize, usize, u64, u128);
try_from_upper_bounded!(usize, i8, i16, i32, i64, isize);
try_from_unbounded!(usize, i128);

try_from_both_bounded!(isize, u8, u16, u32);
try_from_lower_bounded!(isize, usize, u64, u128);
try_from_both_bounded!(isize, i8, i16, i32);
try_from_unbounded!(isize, i64, i128);

rev!(try_from_unbounded, usize, u8, u16, u32, u64);
rev!(try_from_upper_bounded, usize, u128);
rev!(try_from_lower_bounded, usize, i8, i16, i32, i64);
rev!(try_from_both_bounded, usize, i128);

rev!(try_from_unbounded, isize, u8, u16, u32);
rev!(try_from_upper_bounded, isize, u64, u128);
rev!(try_from_unbounded, isize, i8, i16, i32, i64);
rev!(try_from_both_bounded, isize, i128);
}

#[doc(hidden)]
trait FromStrRadixHelper: PartialOrd + Copy {
Expand All @@ -2587,15 +2720,21 @@ trait FromStrRadixHelper: PartialOrd + Copy {

macro_rules! doit {
($($t:ty)*) => ($(impl FromStrRadixHelper for $t {
#[inline]
fn min_value() -> Self { Self::min_value() }
#[inline]
fn max_value() -> Self { Self::max_value() }
#[inline]
fn from_u32(u: u32) -> Self { u as Self }
#[inline]
fn checked_mul(&self, other: u32) -> Option<Self> {
Self::checked_mul(*self, other as Self)
}
#[inline]
fn checked_sub(&self, other: u32) -> Option<Self> {
Self::checked_sub(*self, other as Self)
}
#[inline]
fn checked_add(&self, other: u32) -> Option<Self> {
Self::checked_add(*self, other as Self)
}
Expand Down
Loading