Skip to content

bool_comparison mangles macro invocations #15497

@ada4a

Description

@ada4a

Description

This code:

#![warn(clippy::bool_comparison)]
#![allow(unused)]

fn func() -> bool {
    true
}

macro_rules! m {
    ($func:ident) => {
        $func()
    };
}

fn foo(x: bool) -> bool {
    x > m!(func)
}

Caused the following diagnostics:

warning: order comparisons between booleans can be simplified
  --> src/lib.rs:15:5
   |
15 |     x > m!(func)
   |     ^^^^^^^^^^^^ help: try simplifying it as shown: `x & !$func()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison

As you can already see, the output won't compile, because, instead of copying m!(func) into the suggestion verbatim, the lint inserts the macro contents (and does even that incorrectly, since the $ remains for some reason).

This issue was previously noticed for expressions of the form !x == y/x == !y, and fixed, in #11991. But that PR missed the cases x > y and x < y.

Fixing those is a bit complicated:

  1. The lint currently has a check_comparison function, which looks at whether the sides of a binary expressions are literals. It takes a no_literal closure for the case where there aren't any.

  2. For the binops != and ==, the closure is None, i.e. no suggestion happens (but there's a catch, see below)

  3. For > and < on the other hand, the closure is |l: Sugg, r: Sugg| l.bit_and(&(!r)) and |l: Sugg, r: Sugg| (!l).bit_and(&r), respectively, so e.g. x > y turns into x & !y.

  4. There is one expection to (2): when one of the sides of == is !a, the lint turns !x == y into x != y and x == !y into x != y.

    • The PR modified exactly this phase -- the snippet for each side used to be generated from just side.span, but the PR turned that into side.span.source_callsite()
  5. So I guess one way to solve this would be to:

    1. change the </> closures to operate on snippets as well
    2. make them construct {x} & !{y} manually, using format!

    One downside of this is that we lose all the precedence- and paren-handling logic of Span::bit_and. Though one could argue that the current implementation of (4) suffers from the same problem.

One more complication is that I'm actually going to remove the special-handling that is (4) in an upcoming PR -- see it for the motivation^^

Version

playground:
- stable 1.89.0
- nightly 1.91.0 (2025-08-13)

Additional Labels

@rustbot label I-suggestion-causes-error

Metadata

Metadata

Assignees

Labels

I-suggestion-causes-errorIssue: The suggestions provided by this Lint cause an ICE/error when applied

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions