Skip to content

fix(bad_bit_mask): don't lint on #[cfg]-d consts #15665

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(bad_bit_mask): don't lint on #[cfg]-d consts #15665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
687195c
clean-up and add comments
ada4a Sep 12, 2025
1c9c249
make the docs less condescending
ada4a Sep 12, 2025
1d0027e
fix(ConstEvalCtxt): check if the const item is behind `#[cfg]`
ada4a Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions clippy_lints/src/operators/bit_mask.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ pub(super) fn check<'tcx>(
right: &'tcx Expr<'_>,
) {
if op.is_comparison() {
// `<left> <op> <const_right>`
if let Some(cmp_opt) = fetch_int_literal(cx, right) {
check_compare(cx, left, op, cmp_opt, e.span);
// `<const_left> <op> <right>`
} else if let Some(cmp_val) = fetch_int_literal(cx, left) {
// turn into `<right> <inv(op)> <const_left>`
check_compare(cx, right, invert_cmp(op), cmp_val, e.span);
}
}
Expand All @@ -37,13 +40,15 @@ fn invert_cmp(cmp: BinOpKind) -> BinOpKind {
}

fn check_compare<'a>(cx: &LateContext<'a>, bit_op: &Expr<'a>, cmp_op: BinOpKind, cmp_value: u128, span: Span) {
if let ExprKind::Binary(op, left, right) = &bit_op.kind {
if op.node != BinOpKind::BitAnd && op.node != BinOpKind::BitOr || is_from_proc_macro(cx, bit_op) {
return;
}
if let Some(mask) = fetch_int_literal(cx, right).or_else(|| fetch_int_literal(cx, left)) {
check_bit_mask(cx, op.node, cmp_op, mask, cmp_value, span);
}
// whether `<bit_op> <cmp_op> <cmp_value>`
// is `(<left> <op> <right>) <cmp_op> <cmp_value>`
// is `(<masked> <op> <mask>) <cmp_op> <cmp_value>` (either `left` or `right` is the `mask`)
if let ExprKind::Binary(op, left, right) = &bit_op.kind
&& matches!(op.node, BinOpKind::BitAnd | BinOpKind::BitOr)
&& !is_from_proc_macro(cx, bit_op)
&& let Some(mask) = fetch_int_literal(cx, right).or_else(|| fetch_int_literal(cx, left))
{
check_bit_mask(cx, op.node, cmp_op, mask, cmp_value, span);
}
}

Expand Down
4 changes: 0 additions & 4 deletions clippy_lints/src/operators/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,6 @@ declare_clippy_lint! {
/// set to zero or one by the bit mask, the comparison is constant `true` or
/// `false` (depending on mask, compared value, and operators).
///
/// So the code is actively misleading, and the only reason someone would write
/// this intentionally is to win an underhanded Rust contest or create a
/// test-case for this lint.
///
/// ### Example
/// ```no_run
/// # let x = 1;
Expand Down
32 changes: 26 additions & 6 deletions clippy_utils/src/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,21 +638,41 @@ impl<'tcx> ConstEvalCtxt<'tcx> {
let res = self.typeck.qpath_res(qpath, id);
match res {
Res::Def(DefKind::Const | DefKind::AssocConst, def_id) => {
// Check if this constant is based on `cfg!(..)`,
// which is NOT constant for our purposes.
if let Some(node) = self.tcx.hir_get_if_local(def_id)
&& let Node::Item(Item {
kind: ItemKind::Const(.., body_id),
owner_id,
..
}) = node
&& let Node::Expr(Expr {
{
// check for `const C: _ = cfg!(foo);`
if let Node::Expr(Expr {
kind: ExprKind::Lit(_),
span,
..
}) = self.tcx.hir_node(body_id.hir_id)
&& is_direct_expn_of(*span, sym::cfg).is_some()
{
return None;
&& is_direct_expn_of(*span, sym::cfg).is_some()
{
return None;
}

// check for:
// ```
// #[cfg(foo)]
// const C: _ = _;
// ```
//
// NOTE: it shouldn't be possible to have `#[cfg]` applied to the initializer, because e.g.
// something like this wouldn't work:
// const C: _ = {
// #[cfg(foo)]
// 1
// #[cfg(bar)]
// 2
// };
if self.tcx.has_attr(owner_id.def_id, sym::cfg_trace) {
return None;
}
}

let args = self.typeck.node_args(id);
Expand Down
23 changes: 16 additions & 7 deletions tests/ui/bit_masks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
#![allow(clippy::identity_op, clippy::no_effect, clippy::unnecessary_operation)]

const THREE_BITS: i64 = 7;
const EVEN_MORE_REDIRECTION: i64 = THREE_BITS;

#[warn(clippy::bad_bit_mask)]
#[allow(
clippy::ineffective_bit_mask,
clippy::identity_op,
clippy::no_effect,
clippy::unnecessary_operation
)]
#[allow(clippy::ineffective_bit_mask)]
fn main() {
let x = 5;

Expand Down Expand Up @@ -68,7 +65,7 @@ fn main() {
}

#[warn(clippy::ineffective_bit_mask)]
#[allow(clippy::bad_bit_mask, clippy::no_effect, clippy::unnecessary_operation)]
#[allow(clippy::bad_bit_mask)]
fn ineffective() {
let x = 5;

Expand All @@ -89,3 +86,15 @@ fn ineffective() {
x | 3 > 4; // not an error (yet), better written as x >= 4
x | 4 <= 19;
}

fn issue14167() {
#[cfg(test)]
const FORCE_DYNAMIC_DETECTION: u8 = 0b00010000;

#[cfg(not(test))]
const FORCE_DYNAMIC_DETECTION: u8 = 0b0000000;

const CAPS_STATIC: u8 = 0b00001111;

CAPS_STATIC & FORCE_DYNAMIC_DETECTION == 0;
}
34 changes: 17 additions & 17 deletions tests/ui/bit_masks.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: &-masking with zero
--> tests/ui/bit_masks.rs:14:5
--> tests/ui/bit_masks.rs:11:5
|
LL | x & 0 == 0;
| ^^^^^^^^^^
Expand All @@ -8,81 +8,81 @@ LL | x & 0 == 0;
= help: to override `-D warnings` add `#[allow(clippy::bad_bit_mask)]`

error: this operation will always return zero. This is likely not the intended outcome
--> tests/ui/bit_masks.rs:14:5
--> tests/ui/bit_masks.rs:11:5
|
LL | x & 0 == 0;
| ^^^^^
|
= note: `#[deny(clippy::erasing_op)]` on by default

error: incompatible bit mask: `_ & 2` can never be equal to `1`
--> tests/ui/bit_masks.rs:20:5
--> tests/ui/bit_masks.rs:17:5
|
LL | x & 2 == 1;
| ^^^^^^^^^^

error: incompatible bit mask: `_ | 3` can never be equal to `2`
--> tests/ui/bit_masks.rs:26:5
--> tests/ui/bit_masks.rs:23:5
|
LL | x | 3 == 2;
| ^^^^^^^^^^

error: incompatible bit mask: `_ & 1` will never be higher than `1`
--> tests/ui/bit_masks.rs:29:5
--> tests/ui/bit_masks.rs:26:5
|
LL | x & 1 > 1;
| ^^^^^^^^^

error: incompatible bit mask: `_ | 2` will always be higher than `1`
--> tests/ui/bit_masks.rs:35:5
--> tests/ui/bit_masks.rs:32:5
|
LL | x | 2 > 1;
| ^^^^^^^^^

error: incompatible bit mask: `_ & 7` can never be equal to `8`
--> tests/ui/bit_masks.rs:44:5
--> tests/ui/bit_masks.rs:41:5
|
LL | x & THREE_BITS == 8;
| ^^^^^^^^^^^^^^^^^^^

error: incompatible bit mask: `_ | 7` will never be lower than `7`
--> tests/ui/bit_masks.rs:47:5
--> tests/ui/bit_masks.rs:44:5
|
LL | x | EVEN_MORE_REDIRECTION < 7;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: &-masking with zero
--> tests/ui/bit_masks.rs:50:5
--> tests/ui/bit_masks.rs:47:5
|
LL | 0 & x == 0;
| ^^^^^^^^^^

error: this operation will always return zero. This is likely not the intended outcome
--> tests/ui/bit_masks.rs:50:5
--> tests/ui/bit_masks.rs:47:5
|
LL | 0 & x == 0;
| ^^^^^

error: incompatible bit mask: `_ | 2` will always be higher than `1`
--> tests/ui/bit_masks.rs:57:5
--> tests/ui/bit_masks.rs:54:5
|
LL | 1 < 2 | x;
| ^^^^^^^^^

error: incompatible bit mask: `_ | 3` can never be equal to `2`
--> tests/ui/bit_masks.rs:60:5
--> tests/ui/bit_masks.rs:57:5
|
LL | 2 == 3 | x;
| ^^^^^^^^^^

error: incompatible bit mask: `_ & 2` can never be equal to `1`
--> tests/ui/bit_masks.rs:63:5
--> tests/ui/bit_masks.rs:60:5
|
LL | 1 == x & 2;
| ^^^^^^^^^^

error: ineffective bit mask: `x | 1` compared to `3`, is the same as x compared directly
--> tests/ui/bit_masks.rs:75:5
--> tests/ui/bit_masks.rs:72:5
|
LL | x | 1 > 3;
| ^^^^^^^^^
Expand All @@ -91,19 +91,19 @@ LL | x | 1 > 3;
= help: to override `-D warnings` add `#[allow(clippy::ineffective_bit_mask)]`

error: ineffective bit mask: `x | 1` compared to `4`, is the same as x compared directly
--> tests/ui/bit_masks.rs:78:5
--> tests/ui/bit_masks.rs:75:5
|
LL | x | 1 < 4;
| ^^^^^^^^^

error: ineffective bit mask: `x | 1` compared to `3`, is the same as x compared directly
--> tests/ui/bit_masks.rs:81:5
--> tests/ui/bit_masks.rs:78:5
|
LL | x | 1 <= 3;
| ^^^^^^^^^^

error: ineffective bit mask: `x | 1` compared to `8`, is the same as x compared directly
--> tests/ui/bit_masks.rs:84:5
--> tests/ui/bit_masks.rs:81:5
|
LL | x | 1 >= 8;
| ^^^^^^^^^^
Expand Down