Skip to content
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

Fix false positive with cast_sign_loss lint #4883

Conversation

krishna-veerareddy
Copy link
Contributor

@krishna-veerareddy krishna-veerareddy commented Dec 5, 2019

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

`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.
@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Dec 6, 2019
@krishna-veerareddy krishna-veerareddy changed the title [WIP] Fix false positive with cast_sign_loss lint Fix false positive with cast_sign_loss lint Dec 6, 2019
@jhpratt
Copy link
Member

jhpratt commented Dec 6, 2019

Does this also work with type widening, such as (-1i8).rem_euclid(1i8) as u16?

Other than that question, looks good to me.

@krishna-veerareddy
Copy link
Contributor Author

@jhpratt It allows widening as well. Should I add test cases for that as well?

@jhpratt
Copy link
Member

jhpratt commented Dec 6, 2019

It certainly wouldn't hurt!

@krishna-veerareddy
Copy link
Contributor Author

Should I add test cases for all the sized types or just one for u8 -> u16 widening?

@krishna-veerareddy
Copy link
Contributor Author

@flip1995 Please review when you have some time.

@flip1995
Copy link
Member

Sorry, was absent for the past few days. Can be merged as is, awesome! 🎉

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 12, 2019

📌 Commit c0fb74b has been approved by flip1995

bors added a commit that referenced this pull request Dec 12, 2019
…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
@bors
Copy link
Collaborator

bors commented Dec 12, 2019

⌛ Testing commit c0fb74b with merge d82debb...

@bors
Copy link
Collaborator

bors commented Dec 12, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing d82debb to master...

@bors bors merged commit c0fb74b into rust-lang:master Dec 12, 2019
@krishna-veerareddy krishna-veerareddy deleted the issue-4818-cast-sign-loss-false-positive branch December 12, 2019 01:36
@krishna-veerareddy
Copy link
Contributor Author

@flip1995 No worries. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Euclidean operations combined with sign casting cannot result in loss of a sign
4 participants