Add new lint: `manual_highest_one` by qdot3 · Pull Request #17472 · rust-lang/rust-clippy

qdot3 · 2026-07-30T05:19:05Z

View all comments

implementation of #16985 (comment)

checklist

Followed lint naming conventions
Added passing UI tests (including committed .stderr file)
cargo test passes locally (except for cargo test --test dogfood because tikv_jemalloc_sys is not resolved)
Executed cargo dev update_lints
Added lint documentation
Run cargo dev fmt

changelog: [manual_highest_one]: add new lint to check manual implementation of highest_one()

rustbot · 2026-07-30T05:19:09Z

Thanks for the pull request, and welcome!

You should hear from one of our reviewers after this PR gets at least 2 reviews from the community.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

qdot3 · 2026-07-30T05:29:13Z

Sorry, I confused with cargo test and uitest.

Gri-ffin

Also can you please add

let _ = u32::BITS - u.leading_zeros() - 1; //~ manual_highest_one
let _ = u32::BITS - (u.leading_zeros() + 1); //~ manual_highest_one

to the test suite.

View changes since this review

qdot3 · 2026-07-31T01:10:20Z

I added some test cases and found that u32::BITS - nz.leading_zeros() incorrectly triggers the mismatched_bit_width_type lint, so I'm ignoring it for now.

Gri-ffin · 2026-07-31T14:36:56Z

+    /// ```
+    #[clippy::version = "1.99.0"]
+    pub MANUAL_HIGHEST_ONE,
+    nursery,


It's best to choose a category other than nursery.

View changes since the review

I choose complexity.

Its best to change it here.

Fixed.

manual_highest_one and manual_ilog2 share some patterns. For now, I simply allow them, but is there a better choice?

Gri-ffin · 2026-08-03T09:21:17Z

+    let _ = identity!(31) - u.leading_zeros(); //~ manual_highest_one
+    let _ = 31 - identity!(u).leading_zeros(); //~ manual_highest_one
+    let _ = 31 - double!(u).leading_zeros(); //~ manual_highest_one
+    let _ = thirty_one!() - u.leading_zeros();


We should be more conservative about macro expanded subexpressions, especially if the lint is aiming to be MachineApplicable. So we should also skip receivers or const operands coming from expansion too.

View changes since the review

I finally understand which kinds of macro usages are allowed. For example, macro!().bit_widrh() - 1 should be linted because macro can be modified without side effect as long as the bit_width call remains valid.

To my understanding, Span carries no information about identity! macro because the macro does nothing and span is not updated during expansion. This is a corner case. IMO, we have two options:

Introduce EarlyLintPass and tackle the AST.

Ignore the case because it is unlikely to matter in practice. Appliciability can be also lowered.

Changing this lint to an early pass would be an unecessary complexity for an edge case, it's better to just not deal with macros.

Early passes also run after macro expansion so it wouldn't help anyway. There's currently no way to detect an identity macro ($e:expr => $e).

I will ignore identity! macro and leave it a false positive, lowering applicaibility to MaybeIncorrect.

Gri-ffin · 2026-08-03T10:01:18Z

+    /// ```
+    #[clippy::version = "1.99.0"]
+    pub MANUAL_HIGHEST_ONE,
+    nursery,


Its best to change it here.

Gri-ffin · 2026-08-03T10:20:07Z

+    // False positive: Currently, `integer_const` does not support `NonZero`
+    // and related aliases. It is better to enhance `integer_const`.
+    let _ = NonZeroU32::BITS - 1 - nz.leading_zeros();


This should be handled, Instead of relying only on integer_const, maybe we can have a small helper that first tries integer_const, and if that fails, recognizes T::BITS directly and gets the bit width from the type:

fn integer_const_or_bits(cx: &LateContext<'_>, expr: &Expr<'_>, ctxt: SyntaxContext) -> Option<u128> { integer_const(cx, expr, ctxt).or_else(|| { if let ExprKind::Path(QPath::TypeRelative(hir_ty, segment)) = expr.kind && segment.ident.name == sym::BITS { bit_width(cx, cx.typeck_results().node_type(hir_ty.hir_id)) } else { None } }) }

Then we can use it in the bit_width checks.

View changes since the review

CommanderStorm

community review: Except Gri-ffin's suggestions, not much I could find. Seems well scoped overall.

I have a few questions about some of the false negatives, but nothing major I think

View changes since this review

CommanderStorm

(community review)
Lgtm from my side.
Make sure to squash the changes into one commit because of rebase merging ^^

View changes since this review

rustbot · 2026-08-12T13:34:13Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rustbot · 2026-08-12T14:35:16Z

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

rustbot added the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Jul 30, 2026

rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 30, 2026

Gri-ffin reviewed Jul 30, 2026

View reviewed changes

Comment thread custom.toml Outdated

Comment thread clippy_lints/src/manual_highest_one.rs Outdated

Comment thread clippy_lints/src/manual_highest_one.rs Outdated

Comment thread clippy_lints/src/manual_highest_one.rs Outdated

Comment thread clippy_lints/src/manual_highest_one.rs Outdated

qdot3 requested a review from Gri-ffin July 31, 2026 10:24

Gri-ffin reviewed Jul 31, 2026

View reviewed changes

CommanderStorm reviewed Aug 1, 2026

View reviewed changes

Comment thread clippy_lints/src/manual_highest_one.rs Outdated

qdot3 force-pushed the wip_isolate_highest_one branch from ea630fa to d8ed010 Compare August 2, 2026 09:24

This comment has been minimized.

Sign in to view

qdot3 requested review from CommanderStorm and Gri-ffin August 2, 2026 10:08

Gri-ffin reviewed Aug 3, 2026

View reviewed changes

CommanderStorm suggested changes Aug 3, 2026

View reviewed changes

Comment thread clippy_lints/src/manual_highest_one.rs Outdated

Comment thread tests/ui/manual_highest_one.rs Outdated

Comment thread tests/ui/manual_highest_one.rs Outdated

Comment thread tests/ui/manual_highest_one.rs Outdated

Comment thread tests/ui/manual_highest_one.rs Outdated

qdot3 force-pushed the wip_isolate_highest_one branch from 9d05837 to a822976 Compare August 6, 2026 23:24

This comment has been minimized.

Sign in to view

qdot3 mentioned this pull request Aug 6, 2026

Extend integer_const to cover NonZero type and NonZero* type aliases #17481

Open

qdot3 requested review from CommanderStorm and Gri-ffin August 7, 2026 01:58

qdot3 force-pushed the wip_isolate_highest_one branch from f95a04b to 1017af2 Compare August 7, 2026 15:03

This comment has been minimized.

Sign in to view

qdot3 mentioned this pull request Aug 11, 2026

New lint: manual_next_multiple_of #17517

Open

CommanderStorm approved these changes Aug 11, 2026

View reviewed changes

changelog: new lint: [manual_highest_one]

6f14e7b

qdot3 force-pushed the wip_isolate_highest_one branch from 1017af2 to 6f14e7b Compare August 12, 2026 13:34

Uh oh!

Uh oh!

Conversation

qdot3 commented Jul 30, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 30, 2026

Uh oh!

qdot3 commented Jul 30, 2026

Uh oh!

Gri-ffin left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qdot3 commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gri-ffin Jul 31, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qdot3 Aug 2, 2026

Choose a reason for hiding this comment

Uh oh!

Gri-ffin Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qdot3 Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Gri-ffin Aug 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qdot3 Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qdot3 Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Gri-ffin Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Jarcho Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

qdot3 Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Gri-ffin Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Gri-ffin Aug 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qdot3 Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

CommanderStorm left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qdot3 commented Jul 30, 2026 •

edited by rustbot

Loading

Gri-ffin left a comment •

edited by rustbot

Loading

qdot3 commented Jul 31, 2026 •

edited

Loading

Gri-ffin Jul 31, 2026 •

edited by rustbot

Loading

Gri-ffin Aug 3, 2026 •

edited

Loading

Gri-ffin Aug 3, 2026 •

edited by rustbot

Loading

qdot3 Aug 3, 2026 •

edited

Loading

Gri-ffin Aug 3, 2026 •

edited

Loading

Gri-ffin Aug 3, 2026 •

edited by rustbot

Loading

CommanderStorm left a comment •

edited by rustbot

Loading

CommanderStorm left a comment •

edited by rustbot

Loading