-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add blanket TryFrom impl when From is implemented. #44174
Changes from 5 commits
80e3f89
b0edfce
414ee9a
36c0ff8
93a56cd
5f6de3d
9562981
79f2439
4d2a8c5
ba74a86
1a29e82
966cf33
27d95d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,23 @@ | |
|
||
#![stable(feature = "rust1", since = "1.0.0")] | ||
|
||
use str::FromStr; | ||
use fmt; | ||
|
||
/// A type used as the error type for implementations of fallible conversion | ||
/// traits in cases where conversions cannot actually fail. | ||
/// | ||
/// Because `Infallible` has no variants, a value of this type can never exist. | ||
/// It is used only to satisfy trait signatures that expect an error type, and | ||
/// signals to both the compiler and the user that the error case is impossible. | ||
#[unstable(feature = "try_from", issue = "33417")] | ||
pub enum Infallible {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment reminded me of something: Can you please add an fn foo(x: c_int) -> Result<i32, TryFromIntError> { Ok(x.try_into()?) } There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still too not sold on the name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also should have more impls for this, like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed on the extra derived impls. I will add those. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After thinking about it, maybe |
||
|
||
#[unstable(feature = "try_from", issue = "33417")] | ||
impl fmt::Debug for Infallible { | ||
fn fmt(&self, _: &mut fmt::Formatter) -> fmt::Result { | ||
match *self {} | ||
} | ||
} | ||
|
||
/// A cheap reference-to-reference conversion. Used to convert a value to a | ||
/// reference value within generic code. | ||
|
@@ -417,6 +433,17 @@ impl<T, U> TryInto<U> for T where U: TryFrom<T> | |
} | ||
} | ||
|
||
// Infallible conversions are semantically equivalent to fallible conversions | ||
// with an uninhabited error type. | ||
#[unstable(feature = "try_from", issue = "33417")] | ||
impl<T, U> TryFrom<U> for T where T: From<U> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea if it's better or worse, but could this be on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds fine to me. I'd be happy to change it to that if everyone is in agreement to do it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather not simply because it'd be confusing for |
||
type Error = Infallible; | ||
|
||
fn try_from(value: U) -> Result<Self, Self::Error> { | ||
Ok(T::from(value)) | ||
} | ||
} | ||
|
||
//////////////////////////////////////////////////////////////////////////////// | ||
// CONCRETE IMPLS | ||
//////////////////////////////////////////////////////////////////////////////// | ||
|
@@ -442,14 +469,3 @@ impl AsRef<str> for str { | |
self | ||
} | ||
} | ||
|
||
// FromStr implies TryFrom<&str> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this broke something in Travis:
|
||
#[unstable(feature = "try_from", issue = "33417")] | ||
impl<'a, T> TryFrom<&'a str> for T where T: FromStr | ||
{ | ||
type Error = <T as FromStr>::Err; | ||
|
||
fn try_from(s: &'a str) -> Result<T, Self::Error> { | ||
FromStr::from_str(s) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,6 +89,7 @@ macro_rules! step_impl_unsigned { | |
} | ||
|
||
#[inline] | ||
#[allow(unreachable_patterns)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it's only unreachable for certain |
||
fn add_usize(&self, n: usize) -> Option<Self> { | ||
match <$t>::try_from(n) { | ||
Ok(n_as_t) => self.checked_add(n_as_t), | ||
|
@@ -120,6 +121,7 @@ macro_rules! step_impl_signed { | |
} | ||
|
||
#[inline] | ||
#[allow(unreachable_patterns)] | ||
fn add_usize(&self, n: usize) -> Option<Self> { | ||
match <$unsigned>::try_from(n) { | ||
Ok(n_as_unsigned) => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ | |
|
||
#![stable(feature = "rust1", since = "1.0.0")] | ||
|
||
use convert::TryFrom; | ||
use convert::{Infallible, TryFrom}; | ||
use fmt; | ||
use intrinsics; | ||
use str::FromStr; | ||
|
@@ -2503,16 +2503,23 @@ impl fmt::Display for TryFromIntError { | |
} | ||
} | ||
|
||
#[unstable(feature = "try_from", issue = "33417")] | ||
impl From<Infallible> for TryFromIntError { | ||
fn from(_: Infallible) -> TryFromIntError { | ||
TryFromIntError(()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might also be a bit more clearly expressed as |
||
} | ||
} | ||
|
||
// 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; | ||
type Error = Infallible; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this might need to remain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this macro only being called when the target's pointer width makes the conversion infallible? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this macro used anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's used in the target-specific There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we want these impls changing based on your target. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most calls to this macro were removed in this PR. The ones that are still there are the conversions from specifically sized integer types to |
||
|
||
#[inline] | ||
fn try_from(u: $source) -> Result<$target, TryFromIntError> { | ||
Ok(u as $target) | ||
fn try_from(value: $source) -> Result<Self, Self::Error> { | ||
Ok(value as $target) | ||
} | ||
} | ||
)*} | ||
|
@@ -2584,31 +2591,17 @@ macro_rules! rev { | |
} | ||
|
||
/// 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice to see all these go away 🎉 |
||
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); | ||
|
@@ -2627,15 +2620,13 @@ try_from_both_bounded!(i64, u32, u16, u8); | |
try_from_both_bounded!(i128, u64, u32, u16, u8); | ||
|
||
// usize/isize | ||
try_from_unbounded!(usize, usize); | ||
try_from_upper_bounded!(usize, isize); | ||
try_from_lower_bounded!(isize, usize); | ||
try_from_unbounded!(isize, isize); | ||
|
||
#[cfg(target_pointer_width = "16")] | ||
mod ptr_try_from_impls { | ||
use super::TryFromIntError; | ||
use convert::TryFrom; | ||
use convert::{Infallible, TryFrom}; | ||
|
||
try_from_upper_bounded!(usize, u8); | ||
try_from_unbounded!(usize, u16, u32, u64, u128); | ||
|
@@ -2647,21 +2638,21 @@ mod ptr_try_from_impls { | |
try_from_both_bounded!(isize, i8); | ||
try_from_unbounded!(isize, i16, i32, i64, i128); | ||
|
||
rev!(try_from_unbounded, usize, u8, u16); | ||
rev!(try_from_unbounded, usize, 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_unbounded, isize, 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; | ||
use convert::{Infallible, TryFrom}; | ||
|
||
try_from_upper_bounded!(usize, u8, u16); | ||
try_from_unbounded!(usize, u32, u64, u128); | ||
|
@@ -2673,21 +2664,21 @@ mod ptr_try_from_impls { | |
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_unbounded, usize, 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This impl needs to remain, it's the |
||
rev!(try_from_upper_bounded, isize, u32, u64, u128); | ||
rev!(try_from_unbounded, isize, i8, i16, i32); | ||
rev!(try_from_unbounded, isize, 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; | ||
use convert::{Infallible, TryFrom}; | ||
|
||
try_from_upper_bounded!(usize, u8, u16, u32); | ||
try_from_unbounded!(usize, u64, u128); | ||
|
@@ -2699,14 +2690,14 @@ mod ptr_try_from_impls { | |
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_unbounded, usize, 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_unbounded, isize, i16, i32, i64); | ||
rev!(try_from_both_bounded, isize, i128); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,6 @@ fn test_convert() { | |
#[test] | ||
fn test_from_str() { | ||
assert_eq!(char::from_str("a").unwrap(), 'a'); | ||
assert_eq!(char::try_from("a").unwrap(), 'a'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this test removed? Shouldn't it succeed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was because of the |
||
assert_eq!(char::from_str("\0").unwrap(), '\0'); | ||
assert_eq!(char::from_str("\u{D7FF}").unwrap(), '\u{d7FF}'); | ||
assert!(char::from_str("").is_err()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this could become
!
at some point. Is that the kind of thing that goes in comments, or only in tracking issues?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it, but assuming TryFrom stabilizes first and this ends up in the stable docs, I wouldn't want to explain it in terms of an unstable feature that could change or theoretically go away completely. When the never type lands, it'll simply the explanation here, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too fond of this; maybe we could rename it
NeverError
or something similar to addError
to the name?Also, I'd rather this implement
Error
too.