Tracking issue for TryFrom/TryInto traits
alexcrichton opened this issue · 240 comments
Tracking issue for rust-lang/rfcs#1542
To do:
- Is existing documentation satisfactory?
- #56796 Change bounds on
TryFromblanket impl to useIntoinstead ofFrom - (Re)stabilization PR
Is there a way to generically print an error with the original value if the conversion fails without requiring Clone so a related method which panics on failure could have nice error messages?
A discussion of whether this should go in the prelude is pending for when this goes stable.
Apologies if this is covered somewhere else, but what would we like to see before marking this as stable? I'm pretty sure I've reimplemented this functionality a few times in various projects, so a common reusable trait would make me 😄
We can bring it up for discussion for the next cycle.
🔔 This issue is now entering a cycle-long final comment period for stabilization 🔔
As a point of stabilization, the libs team would also like to add these traits to the prelude as part of their stabilization. This will require a crater run being 100% clean at minimum, but we're relatively confident that the current state of resolve makes it backwards compatible to add traits to the prelude.
I have a few questions about the purpose of these traits.
- Which types in
stdwill these be implemented for? - Which types should get implementations like
impl TryFrom<T> for T? - Which types should get implementations like
impl TryFrom<U> for Tif they already haveimpl From<U> for T?
cc @sfackler, could you expand on the current set of impls and some of the rationale as well?
I think in general the intuition should be exactly the same for that of From/Into, except when the conversion can fail. Just like we gradually add implementations of From and Into to standard library types, I expect we'll do the same for TryFrom and TryInto. The most important guideline here is that the conversion should be the "canonically obvious" one - if there's more than one reasonable way to convert one type to another, TryFrom or From may not be the right things to use.
In general, I don't think it should be expected that all types should impl TryFrom<T> for T or manually lift an impl From<U> for T. In particular, it's often not clear which error type to pick. However, those kinds of implementations are very much ones that can and should be defined to make a particular API work. For example, we have both of these kinds of implementations for various combinations of primitive integer types for the reasons outlined in the RFC. As another example, one ad-hoc trait that TryFrom/TryInto will replace is postgres::IntoConnectParams, which has a reflexive implementation to convert a ConnectParams into itself.
In general, I don't think it should be expected that all types should impl
TryFrom<T> for Tor manually lift an implFrom<U> for T.
When a TryFrom conversion happens to be infallible the error type should be enum Void {}, right? (Or a similar enum with some other name.) Which by the way sounds to me like a good reason to have a general purpose Void type in std.
@SimonSapin that would break an IntoConnectParams style API as well as the integer conversion use case outlined in the RFC since the error types of the infallible and fallible conversions wouldn't match.
@sfackler Not if you use plain From for the error types(e.g. try!)! impl<T> From<!> for T can and should exist for that purpose.
Just like we gradually add implementations of
FromandIntoto standard library types, I expect we'll do the same forTryFromandTryInto.
That doesn't appear to have happened yet so I don't know if it's because nobody has got round to it or there are no applicable types in std. If there are applicable types in std it would be nice to see some implementations for them. For example are any of the following good implementations?
impl TryFrom<u32> for char
impl TryFrom<char> for u8
impl TryFrom<Vec<u8>> for String
impl TryFrom<&[u8]> for &str
In general, I don't think it should be expected that all types should
impl TryFrom<T> for Tor manually lift animpl From<U> for T.
The issue is that if you want to use TryInto<T> and T isn't in your crate then you have to just hope that impl TryFrom<T> for T has been implemented. That's an unfortunate limitation, one which doesn't exist for From/Into.
@SimonSapin that would break an
IntoConnectParamsstyle API as well as the integer conversion use case outlined in the RFC since the error types of the infallible and fallible conversions wouldn't match.
Does this mean the error type is somehow fixed based on the type you're converting to?
Why TryFrom::Err and TryInto::Err is not bounded by std::error::Error?
not bounded by std::error::Error?
That would prevent Err being (), which is a viable error type for infallible conversions.
If () is the error type, the function could return Err(()). This doesn't make the function infallible. It just fails to provide a useful error when it does fail. When writing code that uses conversions which may or may not be fallible, I think the best way is to bound by Into and write a specialization that uses TryInto.
Once RFC 1216 is implemented, ! will be the appropriate error type for infallible conversions. I think that would mean that an Error bound on TryFrom::Err/TryInto::Err would be OK.
I’d certainly like it if implementing From<T> yielded a TryFrom<T, Err = !> implementation (and ditto for Into, of course).
I think that would mean that an
Errorbound onTryFrom::Err/TryInto::Errwould be OK.
Yes, we'll definitely have impl Error for ! { ... } for precisely these sorts of cases.
I'm a bit worried about the type differences in the integer conversions use case, but it does seem like we should probably punt on stabilizing this until !-as-a-type lands to have a chance to experiment.
In my POV all associated types that represents errors should be bounded by Error. Sadly, we already left one type without it: FromStr::Err (The only other public types are TryInto and TryFrom, checked with ack 'type Err;' and ack 'type Error;'). Let not make this again.
Discussed recently, the libs team decided to punt on stabilizing these traits until the ! type is worked out.
I guess that now this is no longer blocked, right?
Removing nomination as @sfackler is going to investigate ! types and these traits.
Will this have a change to get stabilized in the foreseeable future?
Why TryFrom::Err and TryInto::Err is not bounded by std::error::Error?
std::err::Error doesn't exist in #![no_std] builds. Also, in many cases there is no benefit in implementing std::err::Error for an error type even when it is available. Thus, I would prefer to not have any such bound.
I have experimented with implementing this myself in my own library. I'd like to reiterate the concern expressed by @SimonSapin in rust-lang/rfcs#1542 (comment): try!(x.try_into()); is confusing because the word “try” is used two different ways in the same statement.
I understand that many people thing that such things should be written x.try_into()?;, however I am one of a substantial number of people (based on all the debates) that strongly prefer to not use the ? syntax because of...all the reasons mentioned in all the debates.
Personally I think we should still try to find some pattern that doesn't require the try_ prefix on the names.
I don't feel particularly strongly about the naming, but I can't personally think of anything that's better.
It's already become semi-standard to use try_ for flavours of functions that return a Result.
however I am one of a substantial number of people (based on all the debates) that strongly prefer to not use the
?syntax because of...all the reasons mentioned in all the debates.
That debate's over now though isn't it? I mean, I sympathize with that side of the debate: I don't know why people said they find try!() so annoying, ? is less visible, encourages lazy error-handling and it seems like a waste of syntax for something we already had a perfectly good macro for (unless it gets extended to become something much more general in the future).
But that's in the past now. ? is stable and it's not going away. So we all might as well all switch to it so that we're all using the same thing and we can stop worrying about name conflicts with try!.
I'd like to reiterate the concern expressed by @SimonSapin in rust-lang/rfcs#1542 (comment)
Since I’m cited by name, let me say that I don’t really have that concern anymore. At the time I made this comment the ? operator was a proposal whose future was uncertain, but now it’s here to stay.
Also, I think stabilizing sooner rather than latter is more important than another round of names bikeshedding. It’s been months since the RFC was accepted and this has been implemented #[unstable].
It's already become semi-standard to use try_ for flavours of functions that return a Result.
This is the thing I find most strange about this feature. Most functions I write returns a Result but I've never named any of those functions with a try_ prefix except when trying to experiment with this trait.
Also, I haven't found any practical advantage to writing this:
impl TryInto<X> for Y {
type Err = MyErrorType;
fn try_into(self) -> Result<X, Self::Err> { ... }
}Instead, I can always just write this, much less syntactic overhead:
fn into_x(self) -> Result<X, MyErrorType> { ... }I've never had to write generic code that was parameterized by TryInto or TryFrom despite having lots of conversions, so the latter form is sufficient for all my uses in types I define. I think having TryInto<...> or TryFrom<...> parameters seems like questionable form.
Also, I found that naming the associated error type Err instead of Error was error-prone as I always typed Error. I noticed that this mistake was made even during the drafting of the RFC itself: rust-lang/rfcs#1542 (comment). Also, code that uses Result already uses the name Err extensively since it is a Result constructor.
There was an alternate proposal that focused on integer types specifically and used terminology like “widen” and “narrow,” e.g. x = try!(x.narrow()); that I'd also implemented. I found that proposal was sufficient for my uses of the functionality proposed here in my actual usage as I only ended up doing such conversions on built-in integer types. It is also more ergonomic and clearer (IMO) for the use cases it is sufficient for.
Also, I haven't found any practical advantage to writing this...
I sort-of agree. This trait is for where a thing can be used to create another thing but sometimes that process can fail - but that sounds like just about every function. I mean, should we have these impls?:
impl TryInto<TcpStream> for SocketAddr {
type Err = io::Error;
fn try_into(self) -> Result<TcpStream, io::Error> {
TcpStream::connect(self)
}
}
impl<T> TryInto<MutexGuard<T>> for Mutex<T> {
type Err = TryLockError<MutexGuard<T>>;
fn try_into(self) -> Result<Mutex<T>, Self::Err> {
self.try_lock()
}
}
impl TryInto<process::Output> for process::Child {
type Err = io::Error;
fn try_into(self) -> Result<process::Output, io::Error> {
self.wait_with_output()
}
}
impl TryInto<String> for Vec<u8> {
type Err = FromUtf8Error;
fn try_into(self) -> Result<String, FromUtf8Error> {
String::from_utf8(self)
}
}
This trait seems almost too-generic. In all the above examples it would be much better to explicitly say what you're actually doing rather than call try_into.
I think having
TryInto<...>orTryFrom<...>parameters seems like questionable form.
Also agree. Why wouldn't you just do the conversion and handle the error before passing the value to the function?
std::err::Error doesn't exist in #![no_std] builds. Also, in many cases there is no benefit in implementing std::err::Error for an error type even when it is available. Thus, I would prefer to not have any such bound.
The one benefit to being bounded by std::error::Error is that it can be the return value of another error's cause(). I really don't know why there isn't a core::error::Error, but I haven't looked into that.
Also, I found that naming the associated error type Err instead of Error was error-prone as I always typed Error. I noticed that this mistake was made even during the drafting of the RFC itself: rust-lang/rfcs#1542 (comment). Also, code that uses Result already uses the name Err extensively since it is a Result constructor.
FromStr, which is stable, also uses Err for its associated type. Whether it's the best name or not, I think it's important to keep it consistent throughout the standard library.
Whether or not TryFrom and TryInto are too general, I would really like to see fallible conversions, at least between integer types, in the standard library. I have a crate for this, but I think the use cases go far enough to make it standard. Back when Rust was alpha or beta, I remember using FromPrimitive and ToPrimitive for this purpose, but those traits had even bigger problems.
Error can't be moved into core due to coherence issues.
Also due to coherence issues Box<Error> doesn't implement Error, another reason we don't bound the Err type.
There really isn't any need to bound it at the trait definition, anyway - you can always add that bound yourself later:
where T: TryInto<Foo>, T::Err: Error
I don't usually comment on these threads, but I've been waiting for this for a while and as expressed by several above, I'm not sure what the hold up is; I always want a from trait that can fail. I have code all over that is called try_from... This trait perfectly encapsulates that idea. Please let me use it on stable rust.
I also wrote a whole bunch of other things, but I've since deleted it as unfortunately, trait coherence prevents this trait from being as useful as it could be for me. E.g., I've essentially reimplemented a specialized version of this exact same trait for the primitive types for a highly generic parser of those types. Don't worry, I will rant about this some other time.
That being said, I believe str::parse will benefit greatly from this, as it also specializes a FromStr trait as the bound - which is exactly (TryFrom<str>) hand specialized.
So correct me if I'm wrong, but I believe stablilizing and using this for str::parse will:
- remove the boilerplate reimplementation, and hence remove a less familiar, and specialized trait
- add
TryFrom<str>in the signature, which properly is in the convert module and is more readily obvious what it does - will allow upstream clients to implement
TryFrom<str>for their data types and getstr.parse::<YourSweetDataType>()for free, along with othertry_fromthey feel like implementing, which makes for better logical code organization.
Lastly, abstractions aside, code re-use, bla bla, I think one of the understated benefits of traits like this is the semantic purchase they provide for beginners and veterans alike. Essentially they provide (or are beginning to provide, the more we stabilize) a uniform landscape with familiar, canonicalized behavior. Default, From, Clone are really great examples of this. They provide a memorable function landscape which users can reach to when performing certain operations, and whose behavior and semantics they already have a good grasp on (learn once, apply everywhere). E.g.:
- I want a default version; oh let me reach for
SomeType::default() - I want to convert this, I wonder if
SomeType::from(other)is implemented (you can just type it and see if it compiles, without reaching for documentation) - I want to clone this,
clone() - I want to try to get this from this, and since errors are an integral part of rust, it can fail in the signature, so let me
try_from- oh wait :P
All of these methods become commonplace and become apart of the Rust users toolkit, which imho reduces logical burden by allowing us to reach for familiar concepts (and thats just another name for a Trait!) whose documentation and semantic behavior we've already internalized.
Of course they don't always match up, in which case we specialize our datastructures, but traits like this reduce the API burden of users and coders alike by giving them access to concepts they've already studied, instead of reading documentation/finding your from_some_thing, etc. Without even having to read your documentation, by using std traits, users can navigate your api and datastructures in a logical, robust and efficient manner.
In some sense, it's formalizing a gentleperson's agreement amongst ourselves, and makes it easier and more familiar for us to perform certain familiar operations.
And that's all I have to say about that ;)
This was previously blocked on an investigation into the possibility of using ! as the error type for infallible integer conversions. As the feature is currently implemented, this fails in even the most simple cases: https://is.gd/Ws3K7V.
Are we still thinking about changing the method names, or should we put this feature into FCP?
@sfackler That playground link works for me if I change the return type on line 29 from Result<u32, ()> to Result<u32, !>: https://is.gd/A9pWbU It does fail to recognize that let Ok(x) = val; is an irrefutable pattern when val has Err type !, but that doesn't seem like a blocking issue.
@Ixrec a primary motivation for these traits was conversions to and from C integer typedefs. If I have a function
fn foo(x: i64) -> Result<c_long, TryFromIntError> {
x.try_into()
}This will compile on i686 targets but not on x86_64 targets.
Similarly, say I want to replace the IntoConnectParams type: https://docs.rs/postgres/0.13.4/postgres/params/trait.IntoConnectParams.html. How can I do that if there's a blanket impl<T> TryFrom<T> for T { type Error = ! }? I need the reflexive implementation for ConnectParams, but with a different concrete error type than !.
It does fail to recognize that
let Ok(x) = val;is an irrefutable pattern when val has Err type !
Note that there's a PR open for that.
If I have a function ...
This should work though
fn foo(x: i64) -> Result<c_long, TryFromIntError> {
let val = x.try_into()?;
Ok(val)
}
At the risk of being an annoying +1 comment, I just want to mention that after macros 1.1 arrives in Rust 1.15, try_from will be the last feature keeping Ruma on nightly Rust. Stable try_from is eagerly anticipated!
On a more substantial note...
This is the thing I find most strange about this feature. Most functions I write returns a Result but I've never named any of those functions with a try_ prefix except when trying to experiment with this trait.
That's a good observation, but I think the reason for the try_ prefix is not that it's necessary to identify the return type as a Result, but to distinguish it from the non-fallible equivalent.
Also, I found that naming the associated error type Err instead of Error was error-prone as I always typed Error. I noticed that this mistake was made even during the drafting of the RFC itself: rust-lang/rfcs#1542 (comment). Also, code that uses Result already uses the name Err extensively since it is a Result constructor.
I agree on this one. Most other error types I have come across in libraries are named "Error" and I like that so far "Err" has only ever meant Result::Err. It seems like stabilizing it as "Err" will result (no pun intended) in people constantly getting the name wrong.
@canndrew Of course it's possible to make work, but the entire point of that motivation for this feature is to make it easy to correctly handle these kinds of platform differences - we want to avoid the entire space of "compiles on x86 but not ARM".
I think I chose Err for consistency with FromStr but I would be very happy to switch to Error before stabilizing!
the last feature keeping Ruma on nightly Rust
Likewise, the alternate playground backend only needs nightly for access to TryFrom; the nightly serde stuff was too unstable for my liking, so I moved to the build script setup. With 1.15, I'll be moving back to #[derive]. I'm eagerly awaiting this feature becoming stable!
@sfackler Sorry I wasn't following. In the case of integer conversions it sounds like what we really need is to not typedef c_ulong to either u32 or u64 depending on platform but to somehow make it a newtype. That way a TryFrom<c_ulong> impl can't interfere with a TryFrom<u{32,64}> impl.
After all, we're always going to have "compiles one platform but not the other" problems if we define types differently on different platforms. It's a shame to have to sacrifice the otherwise-entirely-logical TryFrom<T> for U where U: From<T> impl just so that we can supporting what seems like questionable practice.
I'm not seriously suggesting we block this RFC until we get an integer newtype RFC written+merged+stabilized. But we should keep it in mind for the future.
Similarly, say I want to replace the IntoConnectParams type:
What's the problem here though? Why wouldn't you use one error type for TryFrom<ConnectParams> and another for TryFrom<&'a str>?
I would not advocate that we break literally all FFI code in the world. Having tried and failed to pick up similar integer newtype wrappers like Wrapping, there are massive ergonomic costs.
Is there a impl<T> From<!> for T in the standard library? I don't see it in the docs but that might just be a rustdoc bug. If it's not there, then there's no way to convert the TryFrom<ConnectParams> impl's ! Error into the one I actually need.
Having tried and failed to pick up similar integer newtype wrappers like Wrapping, there are massive ergonomic costs.
I was thinking something more like being able to define your own integer types a. la. C++:
trait IntLiteral: Integer {
const SUFFIX: &'static str;
const fn from_bytes(is_negative: bool, bytes: &[u8]) -> Option<Self>; // or whatever
}
impl IntLiteral for c_ulong {
const SUFFIX: &'static str = "c_ulong";
...
}
extern fn foo(x: c_ulong);
foo(123c_ulong); // use a c_ulong literal
foo(123); // infer the type of the integerWould that solve most the ergonomic issues? I don't actually like C++ user defined literals - or features in general that give people the power to change the language in confusing ways - but I guess it could end up being the lesser of two evils.
Is there a
impl<T> From<!> for Tin the standard library?
Not currently because it conflicts with the From<T> for T impl. My understanding is that impl specialization should eventually be able to handle this though.
That sounds like something we should circle back to in a couple of years.
What's the timeline on stabilization of specialization and !?
For specialization I don't know. For ! itself it's mostly a matter of whenever I can get my patches merged.
@canndrew I definitely agree that it shouldn't be implemented for everything. The docs say attempt to construct Self via a conversion, but what counts as a conversion? How about…changing the same thing from one representation to another, or adding or removing a wrapper? This covers your Vec<u8> -> String and Mutex<T> -> MutexGuard<T>, as well as things like u32 -> char and &str -> i64; while excluding SocketAddr -> TcpStream and process::Child -> process::Output.
I feel like impl From<T> for U should probably imply TryFrom<T, Err=!> for U. Without that, functions that take TryFrom<T>s can't also take From<T>s. (Using try_from()|try_into() for a concrete, infallible conversion would probably just be an anti-pattern. You would be able to do it, but…it would be silly, so just don't.)
I feel like impl From for U should probably imply TryFrom<T, Err=!> for U.
Agreed.
You would be able to do it, but…it would be silly, so just don't.
Sounds like a potential clippy lint.
@BlacklightShining I think it should be implemented for types where, given the output type, the "conversion" is obvious. As soon as multiple conversions are possible (utf8/16/32? serialising vs casting? etc...), it should be avoided.
IMHO:
Right now, we could easily provide a blanket implementation of TryFrom<&str> for everything that implements FromStr:
impl<'a, T: FromStr> TryFrom<&'a str> for T {
type Err = <T as FromStr>::Err;
fn try_from(s: &'a s) -> Result<T, Self::Err> {
T::from_str(s)
}
}I would like to see something like this added before try_from is stabilised, or at least some reference to it in the docs. Otherwise, it may be confusing to have implementations of TryFrom<&'a str> and FromStr that differ.
I agree that that would be a good inclusion, but it might conflict with the proposed Try -> TryFrom impl?
@sfackler potentially. It'd be perfectly fine if TryFrom, TryInto, and FromStr all referenced each other in the docs, but my main concern is that there's no existing standard that says whether str::parse and str::try_into return the same value.
I'd be fine with a soft request in the docs that people should implement them to have the same behaviour, although I can definitely see cases where someone might think that they can be different.
For example, let's say that someone creates a Password struct for a website. They might assume that "password".parse() would check the password for validity, then convert it into a hash, whereas Password::try_from("1234abcd") might assume that "1234abcd" is already a hash stored in the database and try to parse it directly into a hash which can be compared.
This makes sense, considering how the wording of parse implies that some level of parsing is done, whereas try_from is just a type conversion. However, in reality, we might want to clarify that both of these functions intend to perform the same thing.
Even though the language team has proposed to close the RFC for deprecating anonymous parameters, they all seem to agree that ideally we would stop creating new code that uses anonymous parameters. With that in mind, could we update the signature of try_from/try_into to give the parameters names? Or would it be more important to maintain symmetry with from/into?
Also, would it be useful to write a summary of major unanswered questions still remaining? I'm really hoping we can decide to stabilize this for the next release cycle. As I mentioned, it's the only remaining nightly feature I use a lot. :}
@jimmycuadra certainly yeah! Want to send a PR adding some parameter names?
With respect to the current state of things, I believe the only unanswered question is if there should be a
impl<T, U> TryFrom<U> for T
where T: From<U>
{
type Error = !;
fn try_from(u: U) -> Result<T, !> {
Ok(T::from(u))
}
}This adds some nice amount of symmetry, but makes things a bit more annoying to deal with in many cases since you no longer have a single error type for the things you want to convert.
type Error = !;
I suggest doing that it in a separate RFC that takes into account the result of whatever is decided about all the undecided stuff regarding !.
@sfackler I think that it'd be important to factor in the stuff I mentioned about FromStr too. Should we have a similar impl for FromStr implementors, or should they be allowed to be distinct, or should we just document that they should be the same but don't have to be?
I'm of the opinion that TryFrom<str> and FromStr should be functionally identical, and the documentation should make it clear that implementations of the two are intended to be identical. Implementing one should also give you the other, at least in terms of allowing you to use str::parse. If Rust had had TryFrom from the start, FromStr would never have been needed. For that reason, I would also document TryFrom<str> to be the preferred form for new code.
@jimmycuadra in that case, we should modify parse to use TryFrom and then put a blanket impl for FromStr -> TryFrom.
If we're going change str::parse to be implemented in terms of TryFrom, should we also change other implementations of FromStrfor concrete types similarly (i.e. all the implementors in this list: https://doc.rust-lang.org/stable/std/str/trait.FromStr.html)? Should the docs for FromStr be updated to suggest using TryFrom instead? Should the current concrete implementations of FromStr have their docs moved to the TryFrom version?
I think for backwards compatibility we can't change FromStr, right?
Removing FromStr in favor for TryFrom<&str> is definitely something to keep in mind for Rust 2.0.
Yep, once this feature stabilizes we'll file an issue and tag it with 2.0-breakage-wishlist.
One thing to also consider is adding a parse_into method to String which uses TryFrom<String>. I find myself implementing TryFrom for both often if a type internally stores a String but still requires validation.
If we're going to leave impl<T, U> TryFrom for T where T: From for a future RFC, is this feature then ready to stabilize now? I really don't want to miss another release cycle, so I'm hoping some folks on the Rust team have the bandwidth to discuss and make a decision.
I think that the problem is that it'd be difficult to stabilise that feature after it's been stabilised and people have provided impls for both.
I would expect that already having a T : From<U> would put U : TryFrom<T> in the "obvious API hole" category of change when the implementation is reasonable.
That implies there should at least be a T : TryFrom<T> with Error = !, but the version for any infallible From is clearly better than that.
IMO there isn't really a clear distinction between whether From should provide a TryFrom implementation or whether TryFrom should provide a From implementation.
Because on one hand, you could consider T::from(val) to be just T::try_from(val).unwrap(), and on the other, you could consider T::try_from(val) to be just Ok(T::from(val)). Which is better? I don't know.
you could consider
T::from(val)to be justT::try_from(val).unwrap()
I disagree with that one. From implementations are not expected to ever panic. Only the other way around makes sense.
@clarcharr Because From shouldn't panic, the options are From in terms of TryFrom<Error=!> or the other way around. But I'd hate to have the usual advice be "You should implement TryFrom with type Error = !" instead of "You should implement From".
Any way to get some movement on stabilizing this? We're running out of time before 1.18 goes into beta. @sfackler?
Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged teams:
No concerns currently listed.
Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
@sfackler: just to check in, are we good on various concerns around ! and blanket impls? I know we talked about this in the libs meeting, but it'd be helpful to get a summary here.
@aturon The last discussion about it was @sfackler wondering whether impl From<T> for U should provide impl TryFrom<T> for U where TryFrom::Error = !.
@briansmith suggested that a decision on that be a separate RFC once unresolved questions around the never type are worked out.
Isn't the main problem with stabilising this now is that such a change can't be made without breaking backwards compatibility? Or is the solution to just not move forward with that change?
I think the current set of impls is untenable. I could understand either of these positions:
TryFromis intended for fallible conversions only, so doesn't have things likeu8 -> u128orusize -> usize.TryFromis intended for all conversions, some of which are infallible and thus have an uninhabitedTryFrom::Errortype.
But right now things are in an odd hybrid state where the compiler will insert checking code for an i32 -> i32 conversion and yet you can't do a String -> String conversion.
What are the objections to ! as an Error type? The only thing I noticed in a quick skim was "but makes things a bit more annoying to deal with in many cases since you no longer have a single error type for the things you want to convert", but I'm not convinced I agree with that since you have to assume you got passed something custom with a custom error type in generic context no matter what.
Isn't the main problem with stabilising this now is that such a change can't be made without breaking backwards compatibility? Or is the solution to just not move forward with that change?
I'm of the opinion that it's overzealous to add a generic implementation of TryFrom when From is implemented. Although it's semantically true that if there is a From implementation, there is a TryFrom implementation that cannot produce an error, I don't see this provided implementation being practically useful at all, let alone a common enough need that it should be provided by default. If someone really needs that behavior for their type for some reason, it's just one simple implementation away.
If there's an example of a case when you'd ever use try_from instead of from for an infallible conversion, I could certainly change my mind.
What are the objections to ! as an Error type?
! is months away from stabilization. Pick one of stabilizing TryFrom in the near future or having the impl<T, U> TryFrom<U> for T where T: From<U>.
Have we considered using trait aliases (rust-lang/rfcs#1733) here? When that lands, we can alias From<T> to TryFrom<T, Error=!>, making the two traits one and the same.
@lfairy That would break user impls of From, alas.
@glaebhoerl Yeah, you're right 😥 The motivation section of that RFC mentions aliasing impls but the actual proposal disallows them.
(Even if it didn't, the methods have different names etc.)
That could go under a 2.0 wishlist but regardless it's not going to happen without breaking anything.
First of all, thank you @sfackler for a great conversation about this on IRC. After letting things sit in my head for a bit, here's where I ended up.
Pick one of stabilizing
TryFromin the near future or having theimpl<T, U> TryFrom<U> for T where T: From<U>.
I think the core question here is whether infallible conversions belong in the trait. I think that they do, for things like the infallible conversions in the RFC and for generic use (analogous to how there's the seemingly-useless T:From<T>). Given that, what I want most is to avoid a world where every type implementer is expected to impl TryFrom<MyType> for MyType, and every From impl should also result in a TryFrom impl. (Or get bugs filed later for not providing them.)
So could we have the blanket impl without stabilizing !? I think there's a way, since we already have !-like types in the library, such as std::string::ParseError. ("This enum is slightly awkward: it will never actually exist.")
A sketch of how this could work:
- A new type,
core::convert::Infallible, implemented exactly likestd::string::ParseError. (Maybe even change the latter to a type alias for the former.) impl<T> From<Infallible> for Tso that it's compatible in?with any error type (see thec_foostuff later)- Use
Infallibleas theErrortype in the blanket impl - Later, consider
type Infallible = !;as part of stabilizing the never type
I'll volunteer to make a PR doing that, if it would be helpful to concretize it.
As for c_foo: The above would continue to allow code like this:
fn foo(x: c_int) -> Result<i32, TryFromIntError> { Ok(x.try_into()?) }But it would make code like this a portability "footgun", because of the different error types
fn foo(x: c_int) -> Result<i32, TryFromIntError> { x.try_into() }Personally, I'm not concerned by that difference because so long as c_int is a type alias, there's a "full-auto" footgun:
fn foo(x: c_int) -> i32 { x }And in general, code expecting the associated type on a trait to be the same for different impls feels like a code smell to me. I read TryFrom as "generalize From"; if the goal were "better conversions between integer subsets"--which also feels useful--then the "always the same error type" is logical, but I'd expect something targeted in std::num instead, probably like num::cast::NumCast (or boost::numeric_cast).
(Aside: with #[repr(transparent)] in FCP merge, maybe the c_foo types can become newtypes, at which point these conversions could be more consistent. The From&TryFrom impls could codify the C "char <= short <= int <= long" rules, as well as the standard-required minimum sizes for them as things like c_int:From<i16> or c_long:TryFrom<i64>. Then the conversion above would be i32:TryFrom<c_int> on all platforms, with always the same Error type, and the problem goes away.)
Regarding "This enum is slightly awkward: it will never actually exist."
Is there a reason why the error type couldn't just be unit? Why bother with the unique ParseError struct if the conversation can never error?
@sunjay () is the type system representation of "this can happen, but there's nothing interesting to tell you when it does". Uninhabited types (like ! and std::string::ParseError) are the opposite, the way the type system says "this situation cannot happen ever, so you don't need to deal with it."
If there's an example of a case when you'd ever use try_from instead of from for an infallible conversion, I could certainly change my mind.
I think the core question here is whether infallible conversions belong in the trait.
Here's my use case: I have a config file format where values can be bool, numeric, or string, and a macro for writing literal config values where the keys can be either enum variants or string. For example:
let cfg = config![
BoolOpt::SomeCfgKey => true,
"SomeOtherCfgKey" => 77,
];
Long story short, the macro ends up expanding to a list of ($k, $v).into() calls. I'd like to have a check in the conversion for string keys to make sure they name a valid config option, i.e. implementing TryFrom<(String, ???)> and changing the macro to use ($k, $v).try_into(). It would make it harder to do all this if there wasn't a single method name for the macro to use for all the conversions.
🔔 This is now entering its final comment period, as per the review above. 🔔
I actually really like the idea of:
impl<U: TryFrom<T, Error=!>> From<T> for U {
fn from(val: T) -> U {
val.unwrap()
}
}
Because anyone who wants TryFrom<Error=!> can implement it, but people can still implement From if they like. Perhaps we could ultimately deprecate From but we don't have to.
@scottmcm's plan for using an empty enum sounds great to me.
Not if you use plain
Fromfor the error types(e.g.try!)!impl<T> From<!> for Tcan and should exist for that purpose.
How would this work in practice? Say I'm trying to write a function like this:
fn myfn<P: TryInto<MyType>>(p: P) -> Result<(), MyError>Except this of course doesn't work, I need to specify Error= on TryInto. But what type should I write there? MyError seems obvious but then I can't use MyType with the blanket TryFrom impl.
Are you suggesting the following?
fn myfn<E: Into<MyError>, P: TryInto<MyType, Error=E>>(p: P) -> Result<(), MyError>That seems quite verbose.
There is a more general question of how this is supposed to work if I want multiple “infallible” conversions for the same type but with different error types.
Maybe the TryFrom definition should be changed to the following:
pub trait TryFrom<T, E>: Sized {
type Error: Into<E>;
fn try_from(t: T) -> Result<Self, E>;
}You can constrain the error to be convertible into MyError without having to give it an explicit name, like
fn myfn<P: TryInto<MyType>>(p: P) -> Result<(), MyError> where MyError: From<P::Error>it's still a little verbose, but really does state the constraints for calling the function nicely (playground)
EDIT: And trying with a variant like P::Error: Into<MyError> doesn't actually work with ? since there is no blanket implementation of From for Into. Changing TryFrom as you've shown I would expect to run into the same issue.
The final comment period is now complete.
@jethrogb heh you quoted me from a year ago so I had to think for a bit. @Nemo157 is absolutely right and that seems reasonable.
In the specific case of ! we ought to have a blanket impl, but I recall it overlaps another. It's an annoying overlap as both implementations agree on implementation---undefined behavior / dead code.
A comment on this from another issue: #41904 (comment)
Anyone from the libs team have thoughts on scottmcm's idea? It sounds like a great approach to me, and I'd like to continue pushing this feature forward after missing another release cycle.
The libs team talked about this again a couple of weeks ago (sorry for the delay in writing this up)! We came to the conclusion that the source of the troubles over this feature were primarily that the FFI case doesn't really fit with any other use case of these traits - it's unique in that you're calling it on concrete types through aliases that vary based on target.
So, the basic plan of action is to add the impl<T, U> TryFrom<T> for U where U: From<T> and remove the explicit impls for integer conversions that are infallible. To handle the FFI use case we'll make a separate API.
Using a type alias to avoid blocking on ! is an interesting one. My only concern there is if ! is more "special" than normal uninhabited types that would cause breakage when we swapped the alias from an uninhabited enum to !.