-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow casts from the result of checked_abs
to unsigned
#4764
Conversation
tests/ui/cast.rs
Outdated
(-1i8).checked_abs() as u8; | ||
(-1i16).checked_abs() as u16; | ||
(-1i32).checked_abs() as u32; | ||
(-1i64).checked_abs() as u64; | ||
(-1isize).checked_abs() as usize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked_abs
returns Option<T>
so we should use unwrap
or something else to get contained values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review, i will fix that
triggering a new build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This #4764 (comment) code duplication should still be addressed.
79ec213
to
c3b0ece
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'm working on a rustup. After that, we can merge this.
@flip1995 Thank you!! |
@bors r+ rollup |
📌 Commit c3b0ece has been approved by |
Allow casts from the result of `checked_abs` to unsigned Fixes rust-lang#4743.
Rollup of 6 pull requests Successful merges: - #4730 (Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body) - #4764 (Allow casts from the result of `checked_abs` to unsigned) - #4766 (Fix false positive in derive_hash_xor_eq) - #4811 (Literal Representation Restructure) - #4820 (doc: fix the comment above the lint function) - #4830 (use more efficient code to generate repeated string) Failed merges: r? @ghost changelog: none
// `abs` is an inherent impl of `i{N}`, so a method call with ident `abs` will always | ||
// resolve to that spesific method | ||
if_chain! { | ||
if let ExprKind::MethodCall(ref path, _, _) = op.kind; | ||
if path.ident.name.as_str() == "abs"; | ||
if ["abs", "checked_abs"].contains(path.ident.name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path.indent.name
gives a type error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
@bors r- |
…lse-positive, r=flip1995 Fix false positive with cast_sign_loss lint `cast_sign_loss` lint incorrectly suggests that the result of `checked_abs`, `rem_euclid` and `checked_rem_euclid` cannot be casted to an unsigned integer without loss. Fixes #4818 #4764 #4743 changelog: Fix false positives in `cast_sign_loss` lint
☔ The latest upstream changes (presumably #4883) made this pull request unmergeable. Please resolve the merge conflicts. |
Fixes #4743.