From 687195c24281b8f5fb6587e8af7aa762c278fc91 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Fri, 12 Sep 2025 12:41:32 +0200 Subject: [PATCH 1/3] clean-up and add comments RE: comments: I quickly got lost in all the `op`s and `left`s and `right`s, so I wrote down them down to help me. I imagine they could be helpful to the next person that looks at this --- clippy_lints/src/operators/bit_mask.rs | 19 ++++++++------ tests/ui/bit_masks.rs | 11 +++------ tests/ui/bit_masks.stderr | 34 +++++++++++++------------- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/clippy_lints/src/operators/bit_mask.rs b/clippy_lints/src/operators/bit_mask.rs index e87cfd103c30..1c687f2d9889 100644 --- a/clippy_lints/src/operators/bit_mask.rs +++ b/clippy_lints/src/operators/bit_mask.rs @@ -15,9 +15,12 @@ pub(super) fn check<'tcx>( right: &'tcx Expr<'_>, ) { if op.is_comparison() { + // ` ` if let Some(cmp_opt) = fetch_int_literal(cx, right) { check_compare(cx, left, op, cmp_opt, e.span); + // ` ` } else if let Some(cmp_val) = fetch_int_literal(cx, left) { + // turn into ` ` check_compare(cx, right, invert_cmp(op), cmp_val, e.span); } } @@ -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 ` ` + // is `( ) ` + // is `( ) ` (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); } } diff --git a/tests/ui/bit_masks.rs b/tests/ui/bit_masks.rs index 87dcdb3084d0..d4b6e7473367 100644 --- a/tests/ui/bit_masks.rs +++ b/tests/ui/bit_masks.rs @@ -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; @@ -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; diff --git a/tests/ui/bit_masks.stderr b/tests/ui/bit_masks.stderr index 666ad671edee..e15184cb63c1 100644 --- a/tests/ui/bit_masks.stderr +++ b/tests/ui/bit_masks.stderr @@ -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; | ^^^^^^^^^^ @@ -8,7 +8,7 @@ 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; | ^^^^^ @@ -16,73 +16,73 @@ 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; | ^^^^^^^^^ @@ -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; | ^^^^^^^^^^ From 1c9c249cffdb8dc4e7bd31e4a90eab1e3e8f1ed5 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Fri, 12 Sep 2025 12:42:31 +0200 Subject: [PATCH 2/3] make the docs less condescending --- clippy_lints/src/operators/mod.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index bdbbb3475cd5..55f4f22da075 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -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; From 1d0027e355546f05c92a1385f0ec5eeeb5a07470 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Fri, 12 Sep 2025 12:37:25 +0200 Subject: [PATCH 3/3] fix(ConstEvalCtxt): check if the const item is behind `#[cfg]` --- clippy_utils/src/consts.rs | 32 ++++++++++++++++++++++++++------ tests/ui/bit_masks.rs | 12 ++++++++++++ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index ecd88daa6b39..b49cb5aa956d 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -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); diff --git a/tests/ui/bit_masks.rs b/tests/ui/bit_masks.rs index d4b6e7473367..46b8b32860e4 100644 --- a/tests/ui/bit_masks.rs +++ b/tests/ui/bit_masks.rs @@ -86,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; +}