-
Notifications
You must be signed in to change notification settings - Fork 13k
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
more tiny clippy cleanups #77037
more tiny clippy cleanups #77037
Conversation
r? @oli-obk (rust_highfive has picked a reviewer for you, use r? to override) |
r? @Dylan-DPC ui tests need to be updated (missing a --bless i guess :P ) |
26eb3f8
to
5ed5ad9
Compare
@@ -77,9 +77,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | |||
let res = match bin_op { | |||
Eq => l == r, | |||
Ne => l != r, | |||
Lt => l < r, | |||
Lt => !l & r, // equals l < r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't there be an allow here instead of the new code?
-
regardless whether you think whether
l < r
is clearer than!l & r
or not, this specific piece of code concerns the implementation ofl < r
in the constant evaluator. I think it's most clear to implementl < r
vial < r
instead via something that clippy thinks is better. -
If
l < r
should be replaced with not and and operations, why notl <= r
as well?
The bool comparison lint description in clippy's docs makes sense, x < true
is code smell. But x < y
in an implementation of x < y
is IMO not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I removed changes from the pr.
Sprinkling the rustc code with clippy allow attributes is probably something most people would object though.
5ed5ad9
to
7fc3699
Compare
@bors r+ rollup |
📌 Commit 7fc36998fc7d53e444de570764196eacb2594769 has been approved by |
☔ The latest upstream changes (presumably #77102) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
…y::useless_conversion)
…t()) (clippy::mem_replace_with_default)
7fc3699
to
d7a5c57
Compare
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author |
@bors r+ rollup |
📌 Commit d7a5c57 has been approved by |
more tiny clippy cleanups commits stand alone and can be reviewed one by one
…as-schievink Rollup of 12 pull requests Successful merges: - rust-lang#77037 (more tiny clippy cleanups) - rust-lang#77233 (BTreeMap: keep an eye out on the size of the main components) - rust-lang#77280 (Ensure that all LLVM components requested by tests are available on CI) - rust-lang#77284 (library: Forward compiler-builtins "mem" feature) - rust-lang#77296 (liveness: Use Option::None to represent absent live nodes) - rust-lang#77322 (Add unstable book docs for `-Zunsound-mir-opts`) - rust-lang#77328 (Use `rtassert!` instead of `assert!` from the child process after fork() in std::sys::unix::process::Command::spawn()) - rust-lang#77331 (Add test for async/await combined with const-generics.) - rust-lang#77338 (Fix typo in alloc vec comment) - rust-lang#77340 (Alloc vec use imported path) - rust-lang#77345 (Add test for issue rust-lang#74761) - rust-lang#77348 (Update books) Failed merges: r? `@ghost`
commits stand alone and can be reviewed one by one