New lint idea: suggest ^ when comparing bools

[mgr-inz-rafal]

Hello,
During the review process of my recent PR there's been an idea to create a new lint for suggesting the usage of XOR when comparing bools, like so:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b5bbd3c8e28e981aa80e02a372f242e9

I'd like to work on it, but before I start let me get your opinion. Is it worth implementing? Are the other similar "optimizations" that can be included in such a lint come to your mind?

Any ideas are appreciated.

Original comment:

Wait, these are bools. So we could use a XOR here

                            if lhs_negative ^ rhs_negative;

Originally posted by @flip1995 in https://github.com/_render_node/MDIzOlB1bGxSZXF1ZXN0UmV2aWV3VGhyZWFkMjIwMTAzMDk5OnYy/pull_request_review_threads/discussion

Alternative link: #4867 (comment)

[m-ober]

I think this might be a bit "too much". Code should be readable in the first place, and comprehending xor will take longer than !=. You can literally read "if a is not equal to b" in the second case, whereas with "if a xor b", you have to remember what xor does first. Sure, experienced coders will know, but I guess there are a lot of people who don't know about bitwise operators at all. You could also use bitwise operators & instead of && and | instead of ||, but in the end I think this will only confuse people.

[llogiq]

Yes, but reducing (a && !b) || (!a && b) to a != b would improve readability (and possibly perf) nonetheless. However, the same caveats re side effects as in eq_op apply.

[flip1995]

I think this might be a bit "too much"

Yeah I agree, that suggesting ^ for bool != bool is a bit too much (even though I suggested it in the PR). I only saw this in the PR because the != expression originally was bool == !bool, which is always more readable as bool != bool.

---

So when implementing a lint for bool == !bool, we should suggest != and maybe for teaching purposes also ^.

[mgr-inz-rafal]

Thanks for your feedback. I also had a feeling that suggestion to use ^ instead of bool != bool might be a little bit too much, hence I decided to open this issue before starting an implementation.

However, suggesting bool != bool for bool == !bool and mentioning ^ as an alternative might end up as an useful lint.

I'm not sure about (a && !b) || (!a && b) to a != b though. Need to take a closer look into eq_op.

[flip1995]

I'm not sure about (a && !b) || (!a && b) to a != b though. Need to take a closer look into eq_op.

This could maybe be an extension for nonminimal_bool lint.

[mgr-inz-rafal]

OK, so if that's ok, I'll start crafting a lint suggesting bool != bool for bool == !bool.

[llogiq]

@flip1995 is right, this would be a natural extension to nonminimal_bool, however, this might end up needing to extend the quine_mckluskey implementation with xors/nxors. No idea how complicated that is.

[flip1995]

OK, so if that's ok, I'll start crafting a lint suggesting bool != bool for bool == !bool.

Great! 🎉

extend the quine_mckluskey implementation with xors/nxors. No idea how complicated that is.

Oh yeah... Probably not a good first issue difficulty.

[mgr-inz-rafal]

OK, I finally had some time for this issue. My idea is to extend the needless_bool.rs lint as coded in the draft PR. I got stuck at the point when I'd like to suggest a corrected expression so that, if I understood correctly, it can be automatically fixed by cargo.

Can someone shed some light on span_lint_and_sugg usage?

And, as usual, any other comments are appreciated, since I'm not sure if the overall approach is correct.