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

Update clippy version in CI and fix new lints #1203

Merged
merged 3 commits into from
Aug 23, 2023
Merged

Update clippy version in CI and fix new lints #1203

merged 3 commits into from
Aug 23, 2023

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Aug 21, 2023

Closes #1187

@newpavlov newpavlov marked this pull request as ready for review August 21, 2023 14:02
@tarcieri
Copy link
Member

The der related problems are actually a regression/missing feature in clippy, now that arithmetic_side_effects has subsumed the integer_arithmetic lint: rust-lang/rust-clippy#11220

It assumes any math with core::ops traits is unsafe, even on user-defined types which prevent those problems (in the case of der, with fallible results to arithmetic operations).

There's a suggestion to add a feature to clippy to hint to it that a given type's arithmetic is safe, but right now there's no solution other than disabling the lint.

@newpavlov
Copy link
Member Author

newpavlov commented Aug 23, 2023

@tarcieri
It does not look like the Clippy issue will be resolved in the near future. Can I merge this PR or would you like to make some changes?

@tarcieri
Copy link
Member

Ideally I'd like to see arithmetic_side_effects fixed so it provides the same functionality integer_arithmetic had before it was replaced. Otherwise merging will be a regression.

But if you want to merge it, I guess that's fine. It's just an annoying regression.

@newpavlov
Copy link
Member Author

Maybe it's worth to ask Clippy developers to return integer_arithmetic as a subset of arithmetic_side_effects? I would prefer to merge this PR now, since warnings in IDE are quite annoying for me and I would prefer to fix the lints to tweaking toolchain version or disabling Clippy.

@newpavlov newpavlov merged commit 961e756 into master Aug 23, 2023
157 checks passed
@newpavlov newpavlov deleted the clippy branch August 23, 2023 12:31
@tarcieri
Copy link
Member

It just needs a new annotation for arithmetic-safe newtypes as described in rust-lang/rust-clippy#11220 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Clippy and fix lints
2 participants