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

extend modulo_one lint with modulo_zero (because it also fails) #663

Closed
llogiq opened this issue Feb 14, 2016 · 8 comments
Closed

extend modulo_one lint with modulo_zero (because it also fails) #663

llogiq opened this issue Feb 14, 2016 · 8 comments
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy T-AST Type: Requires working with the AST

Comments

@llogiq
Copy link
Contributor

llogiq commented Feb 14, 2016

Also who here knows whether/how modulo works with negative numbers? 😄

@llogiq llogiq added good-first-issue These issues are a good way to get started with Clippy T-AST Type: Requires working with the AST A-lint Area: New lints labels Feb 14, 2016
@oli-obk
Copy link
Contributor

oli-obk commented Feb 14, 2016

negative numbers on which side of the operator?

@oli-obk
Copy link
Contributor

oli-obk commented Feb 14, 2016

Here's my relevant const-eval code (plus the two impls below that function): https://github.com/rust-lang/rust/pull/30587/files#diff-f442b936625c0bc4782fd6b386b17b6cR367

@oli-obk
Copy link
Contributor

oli-obk commented Feb 14, 2016

Note that INT_MIN%-1 panics in rust, but mathematically it should be 0

@Mark-Simulacrum
Copy link
Member

It appears that modulo zero is already checked by Rust: http://is.gd/Jj4zlg.
warning: attempted remainder with a divisor of zero, #[warn(const_err)] on by default

@martiansideofthemoon
Copy link
Contributor

@llogiq , @Manishearth : Is this issue still valid? Would like to work on it!

@Manishearth
Copy link
Member

Nope.

@mcarton
Copy link
Member

mcarton commented Mar 3, 2016

@martiansideofthemoon but the are plenty of other lints you can start with 😄. Don’t hesitate to ask if you have any question.

@Manishearth
Copy link
Member

Yeah, I offered him https://github.com/Manishearth/rust-clippy/issues/476 in IRC 😄

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-AST Type: Requires working with the AST
Projects
None yet
Development

No branches or pull requests

6 participants