Skip to content

Add new lint manual_isolate_lowest_one#17010

Open
b9nn wants to merge 2 commits into
rust-lang:masterfrom
b9nn:lint/16967-manual-isolate-lowest-one
Open

Add new lint manual_isolate_lowest_one#17010
b9nn wants to merge 2 commits into
rust-lang:masterfrom
b9nn:lint/16967-manual-isolate-lowest-one

Conversation

@b9nn
Copy link
Copy Markdown

@b9nn b9nn commented May 13, 2026

What it does

Closes #16967.

Adds a new lint manual_isolate_lowest_one (category: complexity) that flags x & -x or x & x.wrapping_neg() (and the reversed-operand variants) and suggests x.isolate_lowest_one().

Rationale

isolate_lowest_one was stabilized in Rust 1.97.0 (MSRV constant added in clippy_utils::msrvs). The manual x & -x trick is opaque to readers unfamiliar with it, and the - form overflows when x == T::MIN for signed types — the method does not. The issue body has a full motivation including the NonZero<T> case.

Scope of this PR

  • Detects the integer case: x & -x, x & x.wrapping_neg(), and both reversed orderings.
  • Requires structural equality of the two operands via SpanlessEq (matches the pattern used by manual_is_power_of_two).
  • Restricted to integral types — floats don't compile with &, but the explicit is_integral() guard is defensive in case adjustments produce something unexpected.
  • Skips macro expansions (whole expr + both operands checked).
  • MSRV-gated to 1.97.0.

The NonZero<T> case (NonZeroU32::new(x.get() & x.get().wrapping_neg()).unwrap()) mentioned in the issue is intentionally left out of this initial PR — it requires matching a multi-node pattern across NonZeroXXX::new(...).unwrap() and is best as a follow-up to keep the initial review focused.

Files

  • clippy_lints/src/manual_isolate_lowest_one.rs (new) — lint implementation
  • clippy_lints/src/lib.rsmod and pass registration
  • clippy_lints/src/declared_lints.rs — lint info registered
  • clippy_utils/src/msrvs.rs — added ISOLATE_LOWEST_ONE (1.97.0)
  • clippy_utils/src/sym.rs — added wrapping_neg
  • tests/ui/manual_isolate_lowest_one.rs + .fixed — UI test (covers signed, unsigned, both orderings, different-operand negative case, macro non-expansion)

Known limitation

I could not run cargo uitest --bless locally — my Windows toolchain is missing the MSVC linker (link.exe), and Clippy depends on the host rustc toolchain to build. The .stderr snapshot is therefore not included in this PR. Please bless it as part of review, or let me know and I'll generate it from a Linux box. The lint logic itself is modeled directly on manual_is_power_of_two (SpanlessEq + Sugg::hir_with_context) and manual_ilog2 (MSRV + from_expansion guards).

AI disclosure

I used AI assistance to study the existing manual_is_power_of_two and manual_ilog2 lints, design the matching shape for isolate_lowest_one, and draft the implementation + ui-test. I have reviewed all changes myself and believe they are correct.

changelog: new lint [manual_isolate_lowest_one]

@rustbot 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 May 13, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 13, 2026

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, llogiq, samueltardieu

@rustbot

This comment has been minimized.

@b9nn b9nn force-pushed the lint/16967-manual-isolate-lowest-one branch from 61de550 to bbd262a Compare May 13, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lint suggestion: manual_isolate_lowest_one

3 participants