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

use of round/floor/ceil on known integer value #39

Closed
llogiq opened this issue May 3, 2015 · 10 comments · Fixed by #8866
Closed

use of round/floor/ceil on known integer value #39

llogiq opened this issue May 3, 2015 · 10 comments · Fixed by #8866
Assignees
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types

Comments

@llogiq
Copy link
Contributor

llogiq commented May 3, 2015

E.g. ceil(1f32)

@Manishearth
Copy link
Member

FWIW I don't think we'll have too many issues with constants -- I think programmers will be smart enough to not do stuff like this (but footguns like the other issues on this repo would be harder to catch manually)

Still, I don't mind having stuff like this here.

@llogiq
Copy link
Contributor Author

llogiq commented May 3, 2015

I just nicked it from FindBugs. And yes, I don't think it should be high priority.

@Manishearth Manishearth added good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types A-lint Area: New lints labels Aug 11, 2015
@ThomasdenH
Copy link

What should this lint catch? For example, using std::f64::consts::PI.ceil() would be optimized by the compiler anyway and has more explanatory value than 4.

yaahc pushed a commit to yaahc/rust-clippy that referenced this issue Mar 14, 2019
@alekhrycaiko
Copy link

Happy to jump in and take this 🎈

@alekhrycaiko
Copy link

From what I can see, ceil(1f32) is not a valid way of calling ceil currently.

ceil() exists on float types (e.g. f64 and f32) and would be caught by the Rust compiler if called on an int type. I'm not certain there's actually anything to catch here for reasons also brought up by @ThomasdenH

Is there some more specific requirements for what this lint should catch @llogiq

If not, perhaps this issue be closed?

@ThomasdenH
Copy link

Perhaps this lint is just for cases where the operation doesn't change the value?

@Manishearth
Copy link
Member

From what I can see, ceil(1f32) is not a valid way of calling ceil currently.

Yes, that was pseudocode

This lint is about calling ceil on a known integer float value. So something like 1f32.ceil(). But as I mentioned earlier this seems pretty hard to do by accident.

@botahamec
Copy link
Contributor

botahamec commented May 17, 2022

@rustbot claim
I've been looking at this for practice, and I'd like to try it. I got it to work with literals. I'm gonna try to see it I can figure out how to make it work with constants and if variables are possible. Could somebody let me know if I'm on a goose chase?

@xFrednet
Copy link
Member

xFrednet commented May 17, 2022

For constants you can take a look at clippy_utils::consts. However, I could see some use cases for calling these functions on constant values. Therefore, I think it might be better to only lint literals in code 🙃

I've you're stuck on an implementation detail, feel free to reach out or create a draft PR 🙃

@botahamec
Copy link
Contributor

However, I could see some use cases for calling these functions on constant values. Therefore, I think it might be better to only lint literals in code 🙃

@xFrednet I think that's good point. I'll start working on a PR today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types
Projects
None yet
6 participants