Skip to content

Conversation

botahamec
Copy link
Contributor

fixes #39

changelog: added a [`unused_rounding`] lint to check for the rounding of whole-number literals

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 21, 2022
@botahamec botahamec marked this pull request as ready for review May 21, 2022 21:25
} else {
""
};
if f.fract() < f64::EPSILON {
Copy link

@ghost ghost May 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the test that f is a whole number? f64::EPSILON is 2-53 but it's possible for a positive float to be smaller than that. Try let _ = 2E-54f64.floor(); in your tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Thanks! This should be fixed now

Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! 👍

I'd also like to see the test @mikerite proposed. And I wonder if the lint should be omitted in test code (we have an in_test function in clippy_utils, but it requires a LateContext).

#[clippy::version = "1.62.0"]
pub UNUSED_ROUNDING,
nursery,
"Rounding a whole number literal, which is useless"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Rounding a whole number literal, which is useless"
"Uselessly rounding a whole number floating-point literal"

@botahamec
Copy link
Contributor Author

@llogiq I made both changes. I'm not sure about omitting it in tests, so I left that the same for now. What would be the reason for doing that?

@bors
Copy link
Contributor

bors commented May 25, 2022

☔ The latest upstream changes (presumably #8882) made this pull request unmergeable. Please resolve the merge conflicts.

@botahamec
Copy link
Contributor Author

botahamec commented May 25, 2022

Ah shoot, I messed up with the rebase, didn't I?
Edit: I later found out how to solve that (whew)

@llogiq
Copy link
Contributor

llogiq commented May 26, 2022

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented May 26, 2022

📌 Commit f489954 has been approved by llogiq

@bors
Copy link
Contributor

bors commented May 26, 2022

⌛ Testing commit f489954 with merge bc4d39e...

@bors
Copy link
Contributor

bors commented May 26, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing bc4d39e to master...

@bors bors merged commit bc4d39e into rust-lang:master May 26, 2022
@botahamec botahamec deleted the unused-rounding branch May 26, 2022 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use of round/floor/ceil on known integer value
4 participants