Change FromStr for String to use Infallible directly #67925
Conversation
|
r? @shepmaster (rust_highfive has picked a reviewer for you, use r? to override) |
587a893 to
02b3c1c
Compare
src/liballoc/string.rs
Outdated
| #[stable(feature = "str_parse_error", since = "1.5.0")] | ||
| pub type ParseError = core::convert::Infallible; | ||
|
|
||
| #[stable(feature = "rust1", since = "1.0.0")] | ||
| impl FromStr for String { | ||
| type Err = core::convert::Infallible; | ||
| #[inline] | ||
| fn from_str(s: &str) -> Result<String, ParseError> { | ||
| fn from_str(s: &str) -> Result<String, core::convert::Infallible> { |
There was a problem hiding this comment.
Does this make ParseError unused?
There was a problem hiding this comment.
Yes. I think the obvious thing for downstream code that needs to specify ParseError for some reason to do is replace it with Infallible. Of course, that's a backwards compatibility issue: your code won't compile with older versions of Rust.
Though I'd expect it to be a fairly rare one, as I doubt much downstream code needs to actually mention ParseError directly.
There was a problem hiding this comment.
This might as well be -> Result<String, Self::Err> to prevent any future churn.
src/liballoc/string.rs
Outdated
| /// defined, but, given that a [`String`] can always be made into a new | ||
| /// [`String`] without error, this type will never actually be returned. As | ||
| /// such, it is only here to satisfy said signature, and is useless otherwise. | ||
| /// This alias exists for backwards compatibility, and will be eventually deprecated. |
There was a problem hiding this comment.
Why will it be deprecated? Says who?
There was a problem hiding this comment.
If the never type works on all editions I don't see why people would ever use Infalliable long-term. Deprecation doesn't prevent its usage, just discourage it.
|
ping from triage: |
02b3c1c to
6935883
Compare
|
@JohnCSimon @shepmaster I changed it to return |
|
The code change itself looks fine, but I'm not cool enough to vet the changes about deprecation. |
|
I think this is fine as is. The only non-doc changes are definitely non-breaking. And mentioning |
|
Ping from triage: @SimonSapin - can you please review this? |
Fixes the confusing documentation on `ParseError` by making it irrelevant.
6935883 to
883e69d
Compare
|
@bors r+ |
|
📌 Commit 883e69d has been approved by |
|
☀️ Test successful - checks-azure |
Fixes the confusing documentation on
ParseErrorby making it irrelevant.It might be fine to mark it as depreciated right now too - I can't imagine much code uses
ParseErrordirectly.