Add partialeq implementation for TryFromIntError type #53476
Conversation
|
r? @shepmaster (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
There was a problem hiding this comment.
My thoughts...
-
I am a big fan of comparable error types for the reasons outlined in the issue.
-
This is unstable, so doesn't seem like a big risk
-
We are already deriving
Copywhich feels like more of a forwards-compatibility hazard thanPartialEq. -
But it's still a forwards-compatibility hazard, so I don't feel qualified approving.
| fn main() { | ||
| let x: u32 = 125; | ||
| let y: Result<u8, TryFromIntError> = u8::try_from(x); | ||
| let _ = y == Ok(125); |
There was a problem hiding this comment.
Can't this just be y == Ok(125)?
|
r? @KodrAus |
src/libcore/num/mod.rs
Outdated
| @@ -4248,7 +4248,7 @@ from_str_radix_int_impl! { isize i8 i16 i32 i64 i128 usize u8 u16 u32 u64 u128 } | |||
|
|
|||
| /// The error type returned when a checked integral type conversion fails. | |||
| #[unstable(feature = "try_from", issue = "33417")] | |||
| #[derive(Debug, Copy, Clone)] | |||
| #[derive(Debug, Copy, Clone, PartialEq)] | |||
There was a problem hiding this comment.
Is there any reason not to go full Eq here since we should satisfy the trait's requirements?
There was a problem hiding this comment.
None indeed. Adding it.
GuillaumeGomez
force-pushed
the
try-from-int-error-partial-eq
branch
from
75cf431 to
da4febd
Compare
|
Updated. |
|
So as far as I know this error type is only attempting to communicate a conversion from one integral type to another where the value is outside of its possible range so I think from a forwards-compatibility perspective we're ok. Thanks, this looks good to me @GuillaumeGomez! @bors r+ rollup |
|
📌 Commit da4febd has been approved by |
bors
added
S-waiting-on-bors
…artial-eq, r=KodrAus Add partialeq implementation for TryFromIntError type Fixes rust-lang#53458.
…artial-eq, r=KodrAus Add partialeq implementation for TryFromIntError type Fixes rust-lang#53458.
…artial-eq, r=KodrAus Add partialeq implementation for TryFromIntError type Fixes rust-lang#53458.
Rollup of 20 pull requests Successful merges: - #51760 (Add another PartialEq example) - #53113 (Add example for Cow) - #53129 (remove `let x = baz` which was obscuring the real error) - #53389 (document effect of join on memory ordering) - #53472 (Use FxHash{Map,Set} instead of the default Hash{Map,Set} everywhere in rustc.) - #53476 (Add partialeq implementation for TryFromIntError type) - #53513 (Force-inline `shallow_resolve` at its hottest call site.) - #53655 (set applicability) - #53702 (Fix stabilisation version for macro_vis_matcher.) - #53727 (Do not suggest dereferencing in macro) - #53732 (save-analysis: Differentiate foreign functions and statics.) - #53740 (add llvm-readobj to llvm-tools-preview) - #53743 (fix a typo: taget_env -> target_env) - #53747 (Rustdoc fixes) - #53753 (expand keep-stage --help text) - #53756 (Fix typo in comment) - #53768 (move file-extension based .gitignore down to src/) - #53785 (Fix a comment in src/libcore/slice/mod.rs) - #53786 (Replace usages of 'bad_style' with 'nonstandard_style'.) - #53806 (Fix UI issues on Implementations on Foreign types) Failed merges: r? @ghost
Fixes #53458.