Skip to content
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

Add blanket TryFrom impl when From is implemented. #44174

Merged
merged 13 commits into from
Sep 30, 2017
Merged

Add blanket TryFrom impl when From is implemented. #44174

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
80e3f89
Add blanket TryFrom impl when From is implemented.
jimmycuadra Aug 30, 2017
b0edfce
Implement TryFrom explicitly for infallible numeric conversions.
jimmycuadra Aug 30, 2017
414ee9a
Remove test case that assumes FromStr provides TryFrom<&'a str>.
jimmycuadra Aug 30, 2017
36c0ff8
Reword docs for Infallible to make them easier to understand.
jimmycuadra Sep 1, 2017
93a56cd
impl From<Infallible> for TryFromIntError.
jimmycuadra Sep 1, 2017
5f6de3d
Derive additional traits for Infallible.
jimmycuadra Sep 19, 2017
9562981
impl std::error::Error for convert::Infallible.
jimmycuadra Sep 22, 2017
79f2439
Impl fmt::Display for convert::Infallible.
jimmycuadra Sep 22, 2017
4d2a8c5
Simplify implementation of Display and Error for convert::Infallible.
jimmycuadra Sep 24, 2017
ba74a86
Add back mistakenly removed numeric conversions.
jimmycuadra Sep 24, 2017
1a29e82
Remove conflicting TryFrom impls on 32-bit targets.
jimmycuadra Sep 29, 2017
966cf33
Simplify implementation of From<Infallible> for TryFromIntError.
jimmycuadra Sep 29, 2017
27d95d3
Fix more TryFrom impls for integers.
jimmycuadra Sep 29, 2017
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
41 changes: 29 additions & 12 deletions src/libcore/convert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,24 @@

#![stable(feature = "rust1", since = "1.0.0")]

use str::FromStr;
use fmt;

/// An uninhabited type used as the error type for implementations of fallible
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that adding "uninhabited" here might confuse new users. The latter paragraph explains better so I think we can just omit it?

/// conversion traits in cases where they cannot actually fail.
///
/// Because `Infallible` has no constructors (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 {}
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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 add Error to the name?

Also, I'd rather this implement Error too.

Copy link
Member

Choose a reason for hiding this comment

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

This comment reminded me of something: Can you please add an impl From<Infallible> for TryFromIntError? That way code like this will compile even if the try_into is actually infallible:

fn foo(x: c_int) -> Result<i32, TryFromIntError> { Ok(x.try_into()?) }

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still too not sold on the name Infalliable; perhaps NeverError or something similar would be better?

Copy link
Contributor

Choose a reason for hiding this comment

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

We also should have more impls for this, like Eq, PartialEq, Ord, PartialOrd, Hash, and Error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed on the extra derived impls. I will add those. NeverError probably goes against the style guidelines around "stutter" in names. I'd be open to just Never, though. After all, that's the way you say ! (as a type).

Copy link
Contributor

Choose a reason for hiding this comment

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

After thinking about it, maybe CannotError? I think it should probably end in Error by convention.


#[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.
Expand Down Expand Up @@ -417,6 +434,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> {
Copy link
Member

Choose a reason for hiding this comment

The 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 U:Into<T>, to pick up more 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.

Sounds fine to me. I'd be happy to change it to that if everyone is in agreement to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather not simply because it'd be confusing for from to not work but try_from to work in those cases.

type Error = Infallible;

fn try_from(value: U) -> Result<Self, Self::Error> {
Ok(T::from(value))
}
}

////////////////////////////////////////////////////////////////////////////////
// CONCRETE IMPLS
////////////////////////////////////////////////////////////////////////////////
Expand All @@ -442,14 +470,3 @@ impl AsRef<str> for str {
self
}
}

// FromStr implies TryFrom<&str>
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this broke something in Travis:

[00:53:44] error[E0277]: the trait bound `char: core::convert::From<&str>` is not satisfied
[00:53:44]   --> /checkout/src/libcore/../libcore/tests/char.rs:35:16
[00:53:44]    |
[00:53:44] 35 |     assert_eq!(char::try_from("a").unwrap(), 'a');
[00:53:44]    |                ^^^^^^^^^^^^^^ the trait `core::convert::From<&str>` is not implemented for `char`
[00:53:44]    |
[00:53:44]    = help: the following implementations were found:
[00:53:44]              <char as core::convert::From<u8>>
[00:53:44]    = note: required because of the requirements on the impl of `core::convert::TryFrom<&str>` for `char`

#[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)
}
}
2 changes: 2 additions & 0 deletions src/libcore/iter/range.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ macro_rules! step_impl_unsigned {
}

#[inline]
#[allow(unreachable_patterns)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just remove the Err branch?

Copy link
Member

Choose a reason for hiding this comment

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

Because it's only unreachable for certain $ts (like u128), not all of them (like u8).

fn add_usize(&self, n: usize) -> Option<Self> {
match <$t>::try_from(n) {
Ok(n_as_t) => self.checked_add(n_as_t),
Expand Down Expand Up @@ -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) => {
Expand Down
36 changes: 9 additions & 27 deletions src/libcore/num/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2507,12 +2507,10 @@ impl fmt::Display for TryFromIntError {
macro_rules! try_from_unbounded {
($source:ty, $($target:ty),*) => {$(
#[unstable(feature = "try_from", issue = "33417")]
impl TryFrom<$source> for $target {
type Error = TryFromIntError;

impl From<$source> for $target {
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 this needs to stay TryFrom, at least for now, since this would be insta-stable and I don't think libs wants it until the portability lint RFC is available (if ever).

#[inline]
fn try_from(u: $source) -> Result<$target, TryFromIntError> {
Ok(u as $target)
fn from(value: $source) -> $target {
value as $target
}
}
)*}
Expand Down Expand Up @@ -2584,31 +2582,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);
Copy link
Member

Choose a reason for hiding this comment

The 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);
Expand All @@ -2627,10 +2611,8 @@ 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 {
Expand All @@ -2647,14 +2629,14 @@ 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);
}

Expand All @@ -2673,14 +2655,14 @@ 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);
Copy link
Member

Choose a reason for hiding this comment

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

This impl needs to remain, it's the i8 to isize impl below that needs to be removed. The same two impls need to be removed for the 16-bit case as well. Basically ba74a86 needs to be reverted.

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);
}

Expand All @@ -2699,14 +2681,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);
}

Expand Down
7 changes: 2 additions & 5 deletions src/libcore/str/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use self::pattern::Pattern;
use self::pattern::{Searcher, ReverseSearcher, DoubleEndedSearcher};

use char;
use convert::TryFrom;
use fmt;
use iter::{Map, Cloned, FusedIterator};
use slice::{self, SliceIndex};
Expand Down Expand Up @@ -2142,7 +2141,7 @@ pub trait StrExt {
#[stable(feature = "core", since = "1.6.0")]
fn is_empty(&self) -> bool;
#[stable(feature = "core", since = "1.6.0")]
fn parse<'a, T: TryFrom<&'a str>>(&'a self) -> Result<T, T::Error>;
fn parse<T: FromStr>(&self) -> Result<T, T::Err>;
}

// truncate `&str` to length at most equal to `max`
Expand Down Expand Up @@ -2462,9 +2461,7 @@ impl StrExt for str {
fn is_empty(&self) -> bool { self.len() == 0 }

#[inline]
fn parse<'a, T>(&'a self) -> Result<T, T::Error> where T: TryFrom<&'a str> {
T::try_from(self)
}
fn parse<T: FromStr>(&self) -> Result<T, T::Err> { FromStr::from_str(self) }
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down