-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Rename TryFrom's associated type and implement str::parse using TryFrom. #40281
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
This seems reasonable to me, though I'm not sure if that blanket impl will conflict with any other's we may want to add (e.g. |
The renaming also sounds fine to me, yeah, but I agree that I'm less sure of the coherence impact of the blanket impl here. It definitely seems desirable but I'd want to make sure it doesn't otherwise forbid useful (regardless due to a change in signature we'll likely want to do a crater run just to make sure) |
We discussed this PR in the libs team triage, and are happy to go in this directions. Thanks @jimmycuadra! @bors: r+ |
📌 Commit 7fe2e7e has been approved by |
🔒 Merge conflict |
Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See #33417 (comment). TryFrom<&str> and FromStr are equivalent, so have the latter provide the former to ensure that. Using TryFrom in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See #33417 (comment) and #33417 (comment). Refs #33417.
@bors: r=aturon |
📌 Commit 2561dcd has been approved by |
…uron Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See rust-lang#33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See rust-lang#33417 (comment) and rust-lang#33417 (comment). Refs rust-lang#33417.
…uron Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See rust-lang#33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See rust-lang#33417 (comment) and rust-lang#33417 (comment). Refs rust-lang#33417.
…uron Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See rust-lang#33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See rust-lang#33417 (comment) and rust-lang#33417 (comment). Refs rust-lang#33417.
…uron Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See rust-lang#33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See rust-lang#33417 (comment) and rust-lang#33417 (comment). Refs rust-lang#33417.
…uron Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See rust-lang#33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See rust-lang#33417 (comment) and rust-lang#33417 (comment). Refs rust-lang#33417.
…uron Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See rust-lang#33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See rust-lang#33417 (comment) and rust-lang#33417 (comment). Refs rust-lang#33417.
⌛ Testing commit 2561dcd with merge 432843b... |
💔 Test failed - status-travis |
sccache broken pipe:
@bors retry |
@alexcrichton - why is |
⌛ Testing commit 2561dcd with merge 37e4c14... |
Mac builders not starting. Someone call travis. @bors retry |
…uron Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See rust-lang#33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See rust-lang#33417 (comment) and rust-lang#33417 (comment). Refs rust-lang#33417.
Rename TryFrom's associated type and implement str::parse using TryFrom. Per discussion on the tracking issue, naming `TryFrom`'s associated type `Error` is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate from `Result::Err`, the most common "Err". See #33417 (comment). `TryFrom<&str>` and `FromStr` are equivalent, so have the latter provide the former to ensure that. Using `TryFrom` in the implementation of `str::parse` means types that implement either trait can use it. When we're ready to stabilize `TryFrom`, we should update `FromStr` to suggest implementing `TryFrom<&str>` instead for new code. See #33417 (comment) and #33417 (comment). Refs #33417.
☀️ Test successful - status-appveyor, status-travis |
This is due to rust-lang/rust#40281.
Add blanket TryFrom impl when From is implemented. Adds `impl<T, U> TryFrom<T> for U where U: From<T>`. Removes `impl<'a, T> TryFrom<&'a str> for T where T: FromStr` (originally added in #40281) due to overlapping impls caused by the new blanket impl. This removal is to be discussed further on the tracking issue for TryFrom. Refs #33417. /cc @sfackler, @scottmcm (thank you for the help!), and @aturon
Per discussion on the tracking issue, naming
TryFrom
's associated typeError
is generally more consistent with similar traits in the Rust ecosystem, and what people seem to assume it should be called. It also helps disambiguate fromResult::Err
, the most common "Err".See #33417 (comment).
TryFrom<&str>
andFromStr
are equivalent, so have the latter provide the former to ensure that. UsingTryFrom
in the implementation ofstr::parse
means types that implement either trait can use it. When we're ready to stabilizeTryFrom
, we should updateFromStr
tosuggest implementing
TryFrom<&str>
instead for new code.See #33417 (comment)
and #33417 (comment).
Refs #33417.