Add readability-simplify-boolean-expr check to clang-tidy

[LegalizeAdulthood]

Summary

This check looks for comparisons between boolean expressions and boolean constants and simplifies them to just use the appropriate boolean expression directly.

if (b == true) becomes if (b)
if (b == false) becomes if (!b)
if (b && true) becomes if (b)
if (b && false) becomes if (false)
if (b || true) becomes if (true)
if (b || false) becomes if (b)
e ? true : false becomes e
e ? false : true becomes !e
if (true) t(); else f(); becomes t();
if (false) t(); else f(); becomes f();
if (e) return true; else return false; becomes return (e);
if (e) return false; else return true; becomes return !(e);
if (e) b = true; else b = false; becomes b = e;
if (e) b = false; else b = true; becomes b = !(e);

[LegalizeAdulthood]

Hmm... seems like it has a problem with an expression like false || (true && false). The two generated edits for the same line trample on each other.

[ismailp]

Richard,

I think this check should allow expressions originating from macros:

#define HAS_XYZ_FEATURE true
static /*constexpr*/ bool returnTrue() { return true; }
if (returnTrue() && HAS_XYZ_FEATURE);

[LegalizeAdulthood]

This corrects the erroneous double edit on a single line that creates invalid code. It is less aggressive in simplifying the boolean expression, but prevents invalid fixits.

[LegalizeAdulthood]

In D7648#123947, @ismailp wrote:

Richard,

I think this check should allow expressions originating from macros:

#define HAS_XYZ_FEATURE true
static /*constexpr*/ bool returnTrue() { return true; }
if (returnTrue() && HAS_XYZ_FEATURE);

I added these test cases:

static constexpr bool truthy()
{
    return true;
}

#define HAS_XYZ_FEATURE true

void macros_and_constexprs()
{
    int i = 0;
    bool b = (i == 0);
    if (b && truthy()) {
        i = 1;
    }
    i = 2;
    if (b && HAS_XYZ_FEATURE) {
        i = 3;
    }
    i = 4;
}

The second test case was automatically handled by the existing code:

llvm/tools/clang/tools/extra/test/clang-tidy/readability-simplify-bool-expr.cpp:297:14: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
    if (b && HAS_XYZ_FEATURE) {
             ^
        b
llvm/tools/clang/tools/extra/test/clang-tidy/readability-simplify-bool-expr.cpp:287:25: note: expanded from macro 'HAS_XYZ_FEATURE'
#define HAS_XYZ_FEATURE true
                        ^

I will look into the constexpr case.

[LegalizeAdulthood]

Add some test cases for macros and constexpr functions.
Re-enable some temporarily commented out checking on other test cases.

[LegalizeAdulthood]

I looked at the constexpr case and while it could be done, it's quite a bit of work for what feels like an edge condition.

The primary intent of this check is to look for cases where we used boolean expressions in conjunction with boolean literals. The extra literals are simply redundant.

However, with constexpr functions it isn't so cut-and-dried. It seems that if you have a constexpr function that is hardcoded to always return true or false, that the better fixup would be to Inline Function on those constexpr functions. At that point you could run this existing simplification and the usage would be detected and simplified.

[LegalizeAdulthood]

Thinking about this a little more, I'm conflicted as to whether or not it should allow the simplification if the boolean constant came from a macro. If the macro is a feature detection macro, then I'm enabling/disabling chunks of code at runtime based on the resolution of the feature detection and I shouldn't be so quick to simplify that to always true or always false.

I think this is similar to the constexpr case where if the desired result is to simplify the application of the macro, then the macro should be inlined first.

[LegalizeAdulthood]

Guard against false positives from macro expansion resulting in a boolean literal

[LegalizeAdulthood]

In article <CAPtQZ7-7UQAN9KcoQ8XGKht16tiFaRBHq0yM7=[e-mail]>,

Ismail Pazarbasi <[e-mail]> writes:

Yes, that was my concern. I think we should avoid correcting or
warning about macros eagerly.

I agree.

// foo.h
#define EXPECT_FALSE(x) ((x) == false) // macro written in a header file

// foo.cpp
#include "foo.h"
#define MY_EXPECT_FALSE(x) ((x) == false)
static void f() {
  if (EXPECT_FALSE(0 == 1)); // ok
  if (MY_EXPECT_FALSE(0 == 1)); // warn
}

I will add this (or something similar) to my FileCheck test suite.

I was using isExpansionInMainFile() to avoid matching any system
included headers, but perhaps I was misunderstanding how that
narrowing matcher works.

[alexfh]

This is going to be a really nice check if it's done right. Thanks for the contribution!

There's another pattern (similar to the examples with ternary expressions in the patch description) that annoys me:

if (<something>)
  return true;
else
  return false;

Could you add a support for it?

And a few style issues.

clang-tidy/readability/SimplifyBooleanExpr.cpp

16

I'd remove this and start namespace clang right after the next line.

34

I'd go with

const char Something[] = "...";

as it's more readable due to fewer consts.

282

Please use the style of Clang warnings: first letter is lower-case, no trailing period.

clang-tidy/readability/SimplifyBooleanExpr.h

26

Please clang-format the code with the LLVM style.

test/clang-tidy/readability-simplify-bool-expr.cpp

11

Please leave each distinct error message completely only once, and shorten all other copies of it.

62

Please clang-format the test with LLVM style and infinite column limit (to avoid breaking the CHECK lines).

[sbenza]

clang-tidy/readability/SimplifyBooleanExpr.cpp

21

Don't force a conversion to std::string. Keep this as StringRef to avoid the cost of the string.

51

There is no need to mark this inline.
Are you doing it to avoid ODR-problems? or to make the code "faster"?

53

Use StringRef instead of 'const char*'.
Here and all other 'const char*' arguments.

77

Why does it matter if the RHS has a bool literal in it?
This would prevent something like this from matching:

if (true && SomeFunction("arg", true))

Also, hasDescedant() and alike are somewhat expensive. Something to take into account when using them.

97

How are these different from matchBoolBinOpExpr ?

210

StringRef

221

All these replace look the same.
The are replacing the full expression with a subexpression.
They seem like they all could be:

void SimplifyBooleanExpr::replaceWithSubexpression(
        const ast_matchers::MatchFinder::MatchResult &Result,
        const Expr *Subexpr, StringRef Prefix = "")
{
    const Expr* Expression = Result.Nodes.getNodeAs<Expr>(ExpressionId);
    diag(Subexpr->getLocStart(), SimplifyDiagnostic)
        << FixItHint::CreateReplacement(Expression->getSourceRange(),
                                        (Prefix + getText(Result, *Subexpr)).str());
}

If all the replace calls look the same, then you can also consolidate the bind ids to simplify check().
It could be like:

if (const Expr* Sub = result.Nodes.getNodeAs<Expr>("ReplaceWithSub")) {
  replaceWithSubexpression(Result, Sub);
} else if (...

and that might actually support all the uses cases, in which case no if/else is needed. Then check() becomes:

void SimplifyBooleanExpr::check(const ast_matchers::MatchFinder::MatchResult &Result)
{
  const Expr* Full = Result.Nodes.getNodeAs<Expr>(ExpressionId);
  const Expr* Sub = Result.Nodes.getNodeAs<Expr>(SubexpressionId);
  StringRef Prefix = Result.Nodes.getNodeAs<Expr>(AddNotId) ? "!" : "";
  diag(Sub->getLocStart(), SimplifyDiagnostic)
      << FixItHint::CreateReplacement(Full->getSourceRange(),
                                      (Prefix + getText(Result, *Sub)).str());
}

clang-tidy/readability/SimplifyBooleanExpr.h

19

Comment is a copy-paste from some other check.

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

51

I suppose it's my own coding habit to mark functions containing one or two statements as inline. There is nothing intrinsically important about this function being inline.

53

Yes, this came up in the review of readability-remove-void-arg and that change will be folded in here as well. Thanks.

77

There are two cases: either the boolean literal is the LHS or it is the RHS of a binary operator. This is the matcher that handles the LHS being the literal and the RHS being the expression.

This code specifically excludes the case where both LHS and RHS are boolean literals so that complex expressions of constants like:

if (false && (true || false))

are handled without error -- see the nested_booleans code in the test file. If I don't narrow the matcher in this way, then the above expression gets two "fixes" written for it, one that replaces the inner expression and another that replaces the outer expression. Unfortunately, when this happens the combined edits produce invalid code.

I am not certain if this is an error in the engine that applies the fixes or not, but that's what you currently get. So to prevent invalid code from being generated, I disallow one of the matches.

This check could be enhanced to do more elaborate boolean expression elimination, but realistically this is just a corner case for this checker and not something that you should expect to see in real-world code.

I'll look at seeing if there is an alternative to hasDescendant() that I can use that will do the same job.

97

I tried to name the functions after what they were matching. One is matching <bool> <binop> <expr> and the other is matching <expr> <binop> <bool>. (Sorry for the crappy notation there.)

If you didn't see the difference immediately with the names of the two functions, then I need better names! I'm open to suggestions :)

221

I'll look into seeing if I can simplify it further, but the differences come down to either the fact that they have different matchers or that the replacement text is different in the different cases.

Honestly, I abhor duplication and I've pounded on this for many hours trying to make it as small as I can and that's what you're seeing in these reviews.

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.h

19

D'oh! Thanks for catching this and your other comments as well, they've been very helpful.

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

16

Is it acceptable for me to add anonymous namespaces inside namespace clang?

test/clang-tidy/readability-simplify-bool-expr.cpp

62

Many of the tests are specifically designed to show that the user's whitespace is unmolested.

If I reformat the test file using clang-format, it will break all these tests.

A user can always run clang-format on the output of clang-tidy if they want LLVM formatting on the result.

[LegalizeAdulthood]

Corrected documentation comments.
Support simplification of if (e) return true; else return false;
Fixed formatting of diagnostic messages to be consistent with the rest of clang.
Eliminated duplication of warning messages in tests.
clang-format -style=LLVM applied to the new files
Prefer StringRefs over std::string.
Prefer auto when initializing variables.
Let the compiler decide what to inline.

[LegalizeAdulthood]

Drop some unnecessary namespace qualifications.

[alexfh]

clang-tidy/readability/SimplifyBooleanExpr.cpp

237

Any luck replacing replaceWith* methods with a single one as Samuel suggested?

test/clang-tidy/readability-simplify-bool-expr.cpp

62

I agree that the tests that verify whitespace handling can have any formatting that suites their needs.

But do you need, for example, the lines in this test be indented by 4 spaces and opening braces be placed on the next line after function header? I think, we should follow the LLVM style in tests (except for the few places where other formatting is required for the purposes of the test).

[alexfh]

clang-tidy/readability/SimplifyBooleanExpr.cpp

21

Is this function used only once? Maybe inline it then?

33

This form is rather exotic in Clang and LLVM code. I'd still prefer const char X[] = "...";.

54

Here and in a few other places in this patch: please use const X * instead of X const * (and the same for references) for consistency with other clang-tidy code.

55

Please use StringRef instead of const char* wherever it makes sense.

58

nit: It looks better with parentheses and makes operator precedence clear:

return (Literal &&
        Result.SourceManager->isMacroBodyExpansion(Literal->getLocStart()))
           ? nullptr
           : Literal;

67

How about putting this into the ReturnsBool function?

auto SimpleReturnsBool =
    returnStmt(has(boolLiteral(equals(Value)).bind(Id ? Id : "ignored")));

301

We use auto when it's clear what's behind it or the alternative isn't better. The most frequent use of auto is when the type (or its important part) is spelled in the RHS of the initialization. It also frequently makes sense to use auto in range-based for loops.

Here I would clearly use StringRef instead.

The comment applies to other usages of auto in this file as well.

302

isa<CompoundStmt>(If->getElse()) ? ";" : "";

305

And here the use of auto is simply dangerous: this creates a Twine object that is only supposed to be used as a temporary (because if your getText() returned std::string, the Twine object would point to a deleted memory region right after the end of the expression). Please use std::string instead.

307

Also not the best use for auto. Please use SourceLocation instead.

clang-tidy/readability/SimplifyBooleanExpr.h

24–35

Some beutification wouldn't harm, e.g.:

///   `if (b == true)`                           becomes `if (b)`
///   `if (b == false)`                          becomes `if (!b)`
///   `if (b && true)`                           becomes `if (b)`
///   `if (b && false)`                          becomes `if (false)`
///   `if (b || true)`                           becomes `if (true)`
///   `if (b || false)`                          becomes `if (b)`
///   `e ? true : false`                         becomes `e`
///   `e ? false : true`                         becomes `!e`
///   `if (true) t(); else f();`                 becomes `t();`
///   `if (false) t(); else f();`                becomes `f();`
///   `if (e) return true; else return false;`   becomes `return (e);`
///   `if (e) return false; else return true;`   becomes `return !(e);`

[sbenza]

clang-tidy/readability/SimplifyBooleanExpr.cpp

77

There are a few ways to do this.
A "simple" one is to not restrict the matchers and keep a list of fixed SourceRanges in the check.
Then check the list to see if you are about to fix a subrange of something you already fixed.
(if you do this, you must reset this local state at the end of the translation unit).
The good this about this approach is that the matchers are simple and it is easier to not have false negatives.

97

I noticed the difference in the name, but the body is pretty much the same thing.
If you think the name is good documentation, then it is fine to keep them separate.

221

From what I can see, they (most?) all:

• Replace the full expression with a subexpression.
• Optionally add a prefix (a '!') to the replacement.

For all the cases that do this, you can handle them with the same check() logic. Each case would need to:

• bind() the full expression that needs to be replaced.
• bind() the subexpression that will be the replacement.
• optionally mention whether it needs the "!" prefix. You can do this by binding any arbitrary node.

Then you can have code like:

void SimplifyBooleanExpr::matchBoolBinOpExpr(MatchFinder *Finder, bool Value,
                                             const char *const OperatorName) {
  Finder->addMatcher(
      binaryOperator(expr().bind("Full"),
                     isExpansionInMainFile(), hasOperatorName(OperatorName),
                     hasLHS(boolLiteral(equals(Value)),
                     hasRHS(expr().bind("Replacement")),
                     unless(hasRHS(hasDescendant(boolLiteral())))),
      this);

void SimplifyBooleanExpr::matchExprCompOpBool(MatchFinder *Finder, bool Value,
                                              const char *const OperatorName) {
  Finder->addMatcher(
      binaryOperator(
          expr().bind("Full"),
          expr().bind("Negate"),
          isExpansionInMainFile(), hasOperatorName(OperatorName),
          hasLHS(expr().bind(ExpressionId)).bind("Replacement"),
          unless(hasLHS(boolLiteral())),
          unless(hasLHS(hasDescendant(boolLiteral()))),
          hasRHS(hasDescendant(boolLiteral(equals(Value))))),
      this);
}


void SimplifyBooleanExpr::check(const ast_matchers::MatchFinder::MatchResult &Result)
{
  // Handle all the simple cases (are they all like this?)
  if (const Expr* Replacement = Result.Nodes.getNodeAs<Expr>("Replacement")) {
    const Expr* Full = Result.Nodes.getNodeAs<Expr>("Full");
    StringRef Prefix = Result.Nodes.getNodeAs<Expr>("Negate") ? "!" : "";
    diag(Replacement->getLocStart(), SimplifyDiagnostic)
        << FixItHint::CreateReplacement(Full->getSourceRange(),
                                        (Prefix + getText(Result, *Replacement)).str());
  } else {
    // Handle other cases that do not have simple replacements.
  }
}

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

21

I'm getting conflicting reviews here.

One reviewer wants me not to write inline on small functions and you're asking me to write inline.

I honestly don't care which, but please tell me which request I should follow :-).

54

Darnit, I thought I got all those :-). My fingers put the const on the right (how else do you write const pointer to const character?).

55

I'm happy to do that with some guidelines on what "makes sense". In this case it is character literals coming down from the place where you were saying you preferred const char Id[] = ....

What am I gaining by using StringRef here?

58

Talk to clang-format. I'm not using any of my own whitespace preferences, just ramming it through clang-format and taking what it gives me.

67

I went through a couple iterations on this where I had some auto lambdas and then went to named functions with explicit return types. For this one I don't think the auto is less readable.

237

I haven't done the experimenting yet, but I'm not hopeful. They really are all just different enough that I haven't yet seen a way to unify them.

302

Nice! I will be using that.

305

My head is spinning with all this farbling around between std::string, StringRef and Twine. Is the proper usage of these string classes documented somewhere so that I can refer to it while preparing a patch and spend less time churning the patch just to make these adjustments?

test/clang-tidy/readability-simplify-bool-expr.cpp

62

It's a lot of manual rework for no functional benefit.

What are we gaining?

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

97

While they may be "pretty much the same", the differences are important because the replacements are very different due to short-circuiting with && and ||.

If there were no short-circuiting then it would be the simpler thing you're suggesting.

221

I'll take a look at this and see what I can do.

[alexfh]

clang-tidy/readability/SimplifyBooleanExpr.cpp

21

We're talking about different ways to inline a function. I mean inlining on the source code level: take the function body and use it in the single place that invokes it.

Sorry for the ambiguity ;)

55

"Makes sense" here means roughly "everywhere where you pass a string, and not a raw pointer to const char".

What am I gaining by using StringRef here?

Peace of mind (in some sense): when you see a StringRef argument, you know that there is no expensive copying or calculation of the length going on (except where you create a StringRef from a char pointer) . Just use StringRef everywhere where you need a constant string (or a fragment of a string), and there's a buffer with sufficient lifetime (string literal, std::string) somewhere.

You also get fewer code review comments to deal with ;)

58

clang-format doesn't add or remove parentheses. But if you add parentheses here yourself, you'll get the formatting as in the comment above.

So I ask you to add parentheses and re-run clang-format on this line.

Sorry again for not being clear.

67

Note, there's no lambda needed, and no function as well. You just need a certain matcher to use in a more complex matcher. auto SomeMatcher = someMatcher(...); is just the simplest way to write it. The function would make sense if you called it with different arguments, e.g.:

return anyOf(
    SimpleReturnsBool(Value, SomeId),
    compoundStmt(statementCountIs(1), has(SimpleReturnsBool(Value, SomeOtherId))));

Otherwise, it's just simpler and more readable without a separate function:

auto SimpleReturnsBool =                                                          
    returnStmt(has(boolLiteral(equals(Value)).bind(Id ? Id : "ignored")));        
return anyOf(SimpleReturnsBool,                                                   
             compoundStmt(statementCountIs(1), has(SimpleReturnsBool)));

305

Some documentation is here:
http://llvm.org/docs/ProgrammersManual.html#passing-strings-the-stringref-and-twine-classes

You can also read class comments on StringRef and Twine.

What's important here, is that any StringRef + something that compiles yields a Twine. And one dangerous case is StringRef + temporary std::string. This creates a Twine pointing to a temporary std::string. The temporary string gets deleted right at the end of the expression, and the Twine continues to point to the deleted memory. So one should (almost) never store a Twine. The correct way to use it is:

std::string Result = (llvm::Twine("...") + "..." + someFunctionReturningAStdStringOrAStringRef() + ...).str();

[LegalizeAdulthood]

Fuss around with some types to get closer to clang style.
Simplify replacement of matching expressions by eliminating duplication.

[alexfh]

clang-tidy/readability/SimplifyBooleanExpr.cpp

21

What about this?

55

ditto

58

ditto

307

ditto

[sbenza]

clang-tidy/readability/SimplifyBooleanExpr.cpp

63

This type is an implementation detail.
Why not simply internal::Matcher<Stmt> ?

237

I'm sorry, but I have not seen one example that is really different.
They all do the same:

diag(<SomeLocationInTheExpression>, SimplifyOperatorDiagnostic)
    << FixItHint::CreateReplacement(
           <TheRangeOfTheFullExpression>,
           <OptionalPrefix> + getText(Result, <TheSubExpressionWeSelectedAsReplacement>));

I posted code on how to do this generically for all (most?) the cases. It does not hard code binary operations or anything else. Only 3 things are needed here and the matcher can bind to generic ids.

[LegalizeAdulthood]

Thanks for the comments. I apologize for having to make the same comment repeatedly; I find it difficult to match up all the open issues in phabricator. I'm used to review board where such things are easier to manage. I try to work through the open issues, but since they are mixed in with earlier diffs and there's no way for me to mark an issue as fixed in phabricator, it makes me manually go through everything and make sure I got them and I end up missing some items. Please bear with me and have patience :).

As I've been working through the comments, I thought of another simplification that this check should do:

if (e) x = true; else x = false;
if (e) { x = true; } else { x = false; }

These should be simplified to x = e; and their negated counterparts.

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

21

Fixed.

21

OK, so the first getText overload is on a source range.

The next getText is an overload on the template argument, so you get one instantiation per template argument type. They all call the first overload after extracting the source range for the node type.

Effectively, the first overload is the non-template parameter varying part of the algorithm and the second overload is the template parameter varying part.

33

Fixed

34

Fixed.

51

Fixed

53

Fixed

54

Fixed.

55

Fixed

58

Fixed

63

Fixed.

67

Fixed

77

I'm investigating alternatives to the current implementation.

210

Fixed

221

Fixed

237

Fixed

282

Fixed

301

Fixed

302

Fixed

305

Fixed

307

Fixed

clang-tidy/readability/SimplifyBooleanExpr.h

19

Fixed

24–35

Fixed.

26

Fixed.

test/clang-tidy/readability-simplify-bool-expr.cpp

11

Fixed

[LegalizeAdulthood]

Add conditional assignment simplification.
Review all oustanding comments and address the issues raised.

[alexfh]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

254 ↗

(On Diff #21425)

Please use "const auto *" for pointers.

315 ↗

(On Diff #21425)

1. Parentheses are not always needed. I'd suggest adding a couple of helper functions to format negation correctly. These may be missing something, but should be mostly correct:

  bool needsParensAfterUnaryNegation(const Expr &E) {
    if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
      return true;
    if (const auto &Op = dyn_cast<CXXOperatorCallExpr>(E))
      return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && Op->getOperator() != OO_Subscript;
    return false;
  }

std::string negateExpression(const MatchFinder::MatchResult &Result, const Expr &E) {
  StringRef Text = getText(Result, E);
  return (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")" : "!" + Text).str();
}

This applies to other replaceWith* methods as well.

2. How about moving out all non-conditional parts to shorten the expression?
   
   (VariableName + " = " + (Negated ? "!(" + Condition + ")" : Condition) + Terminator).str();

test/clang-tidy/readability-simplify-bool-expr.cpp

52

Why?

63

Please make the test code closer to LLVM style, except for the places where special formatting is required for testing whitespace handling. In particular: 2 space indentation, braces on the same line as function headers or control flow statements, function names should be camelCaseStartingWithALowerCaseLetter, comments should use proper capitalization and punctuation.

171

Why?

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

254 ↗

(On Diff #21425)

Fixed

315 ↗

(On Diff #21425)

Fixed

test/clang-tidy/readability-simplify-bool-expr.cpp

52

Fixed

63

Fixed

171

Fixed

[LegalizeAdulthood]

Extract Function replacementExpression
Reformat test file.

[alexfh]

Another couple of nits. Otherwise looks good!

Please fix the issues and have someone submit this for you. I won't be able to submit the patch in the next few days.

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

244 ↗

(On Diff #22241)

I had to look up what auto is here. Let's use const CXXBoolLiteralExpr *.

250 ↗

(On Diff #22241)

const auto *, please.

[LegalizeAdulthood]

Prefer explicit declarations over auto when initializing expression doesn't mention the type explicitly.
Prefer const auto * declaration when declaring a node type from getNodeAs<T>.
Unwrap parenthesis around replacement expression where possible.

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

244 ↗

(On Diff #22241)

Fixed

250 ↗

(On Diff #22241)

Fixed

[alexfh]

I'm finally back to work and got around to look at this patch again. The latest changes introduced a few issues (see inline comments). I can fix these and submit the patch for you.

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

89 ↗

(On Diff #22423)

nit: In LLVM style there should be no braces around a single-line if body.

101 ↗

(On Diff #22423)

nit: Don't use else after a return.

105 ↗

(On Diff #22423)

nit: This style seems to be more common in LLVM and more idiomatic: assert(false && "...").

116 ↗

(On Diff #22423)

nit: Use const auto *.

118 ↗

(On Diff #22423)

Text should be an std::string, otherwise you're storing a reference to a temporary string.

123 ↗

(On Diff #22423)

nit: Use Text.empty() instead.

[alexfh]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

105 ↗

(On Diff #22423)

This function can replace isNegatable, if it returns an empty StringRef instead of asserting. It will result in less code duplication.

[alexfh]

Fixed the issues and committed as r234626.

Thank you for contributing the check!

[LegalizeAdulthood]

In article <[e-mail]>,

Alexander Kornienko <[e-mail]> writes:

I'm finally back to work and got around to look at this patch again. The
latest changes introduced a few issues (see inline comments). I can fix
these and submit the patch for you.

I'm finally able to revisit this, thanks for fixing those issues and
submitting it!

I have a few more polishing patches I'd like to apply to this to
make it more robust. One is in review right now (chained conditional
assignment/return). The other bit of polish I would like to apply is
handling when 'static_cast<bool>(e)' should be applied to the resulting
expression in order to preserve semantics in some edge cases.

clang-tidy/readability/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyReadabilityModule
   BracesAroundStatementsCheck.cpp
   ContainerSizeEmpty.cpp
   ElseAfterReturnCheck.cpp
   FunctionSize.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantSmartptrGet.cpp
   ShrinkToFitCheck.cpp
+  SimplifyBooleanExpr.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTooling
   )

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmpty.h"
 #include "ElseAfterReturnCheck.h"
 #include "FunctionSize.h"
 #include "RedundantSmartptrGet.h"
 #include "ShrinkToFitCheck.h"
+#include "SimplifyBooleanExpr.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<BracesAroundStatementsCheck>(
         "readability-braces-around-statements");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
     CheckFactories.registerCheck<ElseAfterReturnCheck>(
         "readability-else-after-return");
     CheckFactories.registerCheck<FunctionSizeCheck>(
         "readability-function-size");
     CheckFactories.registerCheck<RedundantSmartptrGet>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<ShrinkToFitCheck>(
         "readability-shrink-to-fit");
+    CheckFactories.registerCheck<SimplifyBooleanExpr>(
+        "readability-simplify-boolean-expr");
   }
 };
 
 } // namespace readability
 
 // Register the MiscTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<readability::ReadabilityModule>
 X("readability-module", "Adds readability-related checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the MiscModule.
 volatile int ReadabilityModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/readability/SimplifyBooleanExpr.h

+//===--- SimplifyBooleanExpr.h clang-tidy -----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Replace copy and swap tricks on shrinkable containers with the
+/// \c shrink_to_fit() method call.
+///
+/// The \c shrink_to_fit() method is more readable and more effective than
+/// the copy and swap trick to reduce the capacity of a shrinkable container.
+/// Note that, the \c shrink_to_fit() method is only available in C++11 and up.
+class SimplifyBooleanExpr : public ClangTidyCheck {
+public:
+    SimplifyBooleanExpr(StringRef Name, ClangTidyContext *Context)
+            : ClangTidyCheck(Name, Context) {}
+    void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+    void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+    void matchBoolBinOpExpr(
+            ast_matchers::MatchFinder *Finder,
+            bool Value,
+            char const *const OperatorName,
+            char const *const BooleanId);
+
+    void matchExprBinOpBool(
+            ast_matchers::MatchFinder *Finder,
+            bool Value,
+            char const *const OperatorName,
+            char const *const BooleanId);
+
+    void matchBoolCompOpExpr(
+            ast_matchers::MatchFinder *Finder,
+            bool Value,
+            char const *const OperatorName,
+            char const *const BooleanId);
+
+    void matchExprCompOpBool(
+            ast_matchers::MatchFinder *Finder,
+            bool Value,
+            char const *const OperatorName,
+            char const *const BooleanId);
+
+    void matchBoolCondition(
+            ast_matchers::MatchFinder *Finder,
+            bool Value,
+            char const *const BooleanId);
+
+    void matchTernaryResult(
+            ast_matchers::MatchFinder *Finder,
+            bool Value,
+            char const *const TernaryId);
+
+    void replaceWithRightExpression(
+            const ast_matchers::MatchFinder::MatchResult &Result,
+            Expr const *BoolLiteral,
+            std::string Prefix = "");
+
+    void replaceWithLeftBooleanLiteral(
+            const ast_matchers::MatchFinder::MatchResult &Result,
+            CXXBoolLiteralExpr const *BoolLiteral);
+
+    void replaceWithRightBooleanLiteral(
+            const ast_matchers::MatchFinder::MatchResult &Result,
+            CXXBoolLiteralExpr const *BoolLiteral);
+
+    void replaceWithLeftExpression(
+            const ast_matchers::MatchFinder::MatchResult &Result,
+            CXXBoolLiteralExpr const *BoolLiteral,
+            std::string Prefix = "");
+
+    void replaceWithThenStatement(
+            const ast_matchers::MatchFinder::MatchResult &Result,
+            CXXBoolLiteralExpr const *BoolLiteral);
+
+    void replaceWithElseStatement(
+            const ast_matchers::MatchFinder::MatchResult &Result,
+            CXXBoolLiteralExpr const *FalseConditionRemoved);
+
+    void replaceWithCondition(
+            const ast_matchers::MatchFinder::MatchResult &Result,
+            ConditionalOperator const *Ternary,
+            std::string Prefix = "");
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H

clang-tidy/readability/SimplifyBooleanExpr.cpp

+//===--- SimplifyBooleanExpr.cpp clang-tidy ---------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SimplifyBooleanExpr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include <string>
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace {
+
+inline std::string getText(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        SourceRange Range) {
+    return Lexer::getSourceText(
+            CharSourceRange::getTokenRange(Range),
+            *Result.SourceManager, Result.Context->getLangOpts());
+}
+
+template <typename T>
+std::string getText(const ast_matchers::MatchFinder::MatchResult &Result, T &Node) {
+    return getText(Result, Node.getSourceRange());
+}
+
+char const *const LeftBooleanLiteralId = "bool-op-expr-yields-bool";
+char const *const RightExpressionId = "bool-op-expr-yields-expr";
+char const *const RightBooleanLiteralId = "expr-op-bool-yields-bool";
+char const *const LeftExpressionId = "expr-op-bool-yields-expr";
+char const *const NegatedRightExpressionId = "bool-op-expr-yields-not-expr";
+char const *const NegatedLeftExpressionId = "expr-op-bool-yields-not-expr";
+char const *const ConditionThenStmtId = "if-bool-yields-then";
+char const *const ConditionElseStmtId = "if-bool-yields-else";
+char const *const TernaryId = "ternary-bool-yields-condition";
+char const *const TernaryNegatedId = "ternary-bool-yields-not-condition";
+
+char const *const IfStmtId = "if";
+char const *const ExpressionId = "expr";
+
+char const *const SimplifyDiagnostic = "Redundant boolean constant supplied to boolean operator.";
+char const *const SimplifyConditionDiagnostic = "Redundant boolean constant in if statement condition.";
+
+inline CXXBoolLiteralExpr const *getBoolLiteral(
+        MatchFinder::MatchResult const &Result,
+        const char *const Id)
+{
+    CXXBoolLiteralExpr const *Literal = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(Id);
+    return Literal && Result.SourceManager->isMacroBodyExpansion(Literal->getLocStart()) ?
+        nullptr : Literal;
+}
+
+} // namespace
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void SimplifyBooleanExpr::matchBoolBinOpExpr(
+        ast_matchers::MatchFinder *Finder,
+        bool Value,
+        char const *const OperatorName,
+        char const *const BooleanId) {
+    Finder->addMatcher(
+        binaryOperator(
+            isExpansionInMainFile(),
+            hasOperatorName(OperatorName),
+            hasLHS(boolLiteral(equals(Value)).bind(BooleanId)),
+            hasRHS(expr().bind(ExpressionId)),
+            unless(hasRHS(hasDescendant(boolLiteral())))
+        ), this);
+}
+
+void SimplifyBooleanExpr::matchExprBinOpBool(
+        ast_matchers::MatchFinder *Finder,
+        bool Value,
+        char const *const OperatorName,
+        char const *const BooleanId) {
+    Finder->addMatcher(
+        binaryOperator(
+            isExpansionInMainFile(),
+            hasOperatorName(OperatorName),
+            hasLHS(expr().bind(ExpressionId)),
+            unless(hasLHS(boolLiteral())),
+            unless(hasLHS(hasDescendant(boolLiteral()))),
+            hasRHS(boolLiteral(equals(Value)).bind(BooleanId))
+        ), this);
+}
+
+void SimplifyBooleanExpr::matchBoolCompOpExpr(
+        ast_matchers::MatchFinder *Finder,
+        bool Value,
+        char const *const OperatorName,
+        char const *const BooleanId) {
+    Finder->addMatcher(
+        binaryOperator(
+            isExpansionInMainFile(),
+            hasOperatorName(OperatorName),
+            hasLHS(hasDescendant(boolLiteral(equals(Value)).bind(BooleanId))),
+            hasRHS(expr().bind(ExpressionId)),
+            unless(hasRHS(hasDescendant(boolLiteral())))
+        ), this);
+}
+
+void SimplifyBooleanExpr::matchExprCompOpBool(
+        ast_matchers::MatchFinder *Finder,
+        bool Value,
+        char const *const OperatorName,
+        char const *const BooleanId) {
+    Finder->addMatcher(
+        binaryOperator(
+            isExpansionInMainFile(),
+            hasOperatorName(OperatorName),
+            hasLHS(expr().bind(ExpressionId)),
+            unless(hasLHS(boolLiteral())),
+            unless(hasLHS(hasDescendant(boolLiteral()))),
+            hasRHS(hasDescendant(boolLiteral(equals(Value)).bind(BooleanId)))
+        ), this);
+}
+
+void SimplifyBooleanExpr::matchBoolCondition(
+        ast_matchers::MatchFinder *Finder,
+        bool Value,
+        char const *const BooleanId) {
+    Finder->addMatcher(
+        ifStmt(
+            isExpansionInMainFile(),
+            hasCondition(boolLiteral(equals(Value)).bind(BooleanId))
+        ).bind(IfStmtId), this);
+}
+
+void SimplifyBooleanExpr::matchTernaryResult(
+        ast_matchers::MatchFinder *Finder,
+        bool Value,
+        char const *const TernaryId) {
+    Finder->addMatcher(
+            conditionalOperator(
+                    isExpansionInMainFile(),
+                    hasTrueExpression(boolLiteral(equals(Value))),
+                    hasFalseExpression(boolLiteral(equals(!Value)))
+            ).bind(TernaryId), this);
+}
+
+void SimplifyBooleanExpr::registerMatchers(ast_matchers::MatchFinder *Finder)
+{
+    matchBoolBinOpExpr(Finder, true, "&&", RightExpressionId);
+    matchBoolBinOpExpr(Finder, false, "||", RightExpressionId);
+    matchBoolCompOpExpr(Finder, true, "==", RightExpressionId);
+    matchBoolCompOpExpr(Finder, false, "!=", RightExpressionId);
+
+    matchBoolBinOpExpr(Finder, false, "&&", LeftBooleanLiteralId);
+    matchBoolBinOpExpr(Finder, true, "||", LeftBooleanLiteralId);
+
+    matchExprBinOpBool(Finder, false, "&&", RightBooleanLiteralId);
+    matchExprBinOpBool(Finder, true, "||", RightBooleanLiteralId);
+
+    matchExprBinOpBool(Finder, true, "&&", LeftExpressionId);
+    matchExprBinOpBool(Finder, false, "||", LeftExpressionId);
+    matchExprCompOpBool(Finder, true, "==", LeftExpressionId);
+    matchExprCompOpBool(Finder, false, "!=", LeftExpressionId);
+
+    matchBoolCompOpExpr(Finder, false, "==", NegatedRightExpressionId);
+    matchBoolCompOpExpr(Finder, true, "!=", NegatedRightExpressionId);
+
+    matchExprCompOpBool(Finder, false, "==", NegatedLeftExpressionId);
+    matchExprCompOpBool(Finder, true, "!=", NegatedLeftExpressionId);
+
+    matchBoolCondition(Finder, true, ConditionThenStmtId);
+    matchBoolCondition(Finder, false, ConditionElseStmtId);
+
+    matchTernaryResult(Finder, true, TernaryId);
+    matchTernaryResult(Finder, false, TernaryNegatedId);
+}
+
+void SimplifyBooleanExpr::check(const ast_matchers::MatchFinder::MatchResult &Result)
+{
+    if (auto LeftRemoved = getBoolLiteral(Result, RightExpressionId)) {
+        replaceWithRightExpression(Result, LeftRemoved);
+    } else if (auto LeftReplacing = getBoolLiteral(Result, LeftBooleanLiteralId)) {
+        replaceWithLeftBooleanLiteral(Result, LeftReplacing);
+    } else if (auto RightReplacing = getBoolLiteral(Result, RightBooleanLiteralId)) {
+        replaceWithRightBooleanLiteral(Result, RightReplacing);
+    } else if (auto RightRemoved = getBoolLiteral(Result, LeftExpressionId)) {
+        replaceWithLeftExpression(Result, RightRemoved);
+    } else if (auto NegatedLeftRemoved = getBoolLiteral(Result, NegatedRightExpressionId)) {
+        replaceWithRightExpression(Result, NegatedLeftRemoved, "!");
+    } else if (auto NegatedRightRemoved = getBoolLiteral(Result, NegatedLeftExpressionId)) {
+        replaceWithLeftExpression(Result, NegatedRightRemoved, "!");
+    } else if (auto TrueConditionRemoved = getBoolLiteral(Result, ConditionThenStmtId)) {
+        replaceWithThenStatement(Result, TrueConditionRemoved);
+    } else if (auto FalseConditionRemoved = getBoolLiteral(Result, ConditionElseStmtId)) {
+        replaceWithElseStatement(Result, FalseConditionRemoved);
+    } else if (auto Ternary = Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId)) {
+        replaceWithCondition(Result, Ternary);
+    } else if (auto TernaryNegated = Result.Nodes.getNodeAs<ConditionalOperator>(TernaryNegatedId)) {
+        replaceWithCondition(Result, TernaryNegated, "!");
+    }
+}
+
+void SimplifyBooleanExpr::replaceWithRightExpression(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        Expr const *BoolLiteral,
+        std::string const Prefix)
+{
+    auto const *const Expression = Result.Nodes.getNodeAs<Expr>(ExpressionId);
+    diag(BoolLiteral->getLocStart(), SimplifyDiagnostic)
+        << FixItHint::CreateReplacement(
+            SourceRange(BoolLiteral->getLocStart(), Expression->getLocEnd()),
+            Prefix + getText(Result, *Expression));
+}
+
+void SimplifyBooleanExpr::replaceWithLeftBooleanLiteral(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        CXXBoolLiteralExpr const *BoolLiteral)
+{
+    auto const *const Expression = Result.Nodes.getNodeAs<Expr>(ExpressionId);
+    diag(BoolLiteral->getLocStart(), SimplifyDiagnostic)
+        << FixItHint::CreateReplacement(
+            SourceRange(BoolLiteral->getLocStart(), Expression->getLocEnd()),
+            getText(Result, *BoolLiteral));
+}
+
+void SimplifyBooleanExpr::replaceWithRightBooleanLiteral(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        CXXBoolLiteralExpr const *BoolLiteral)
+{
+    auto const *const Expression = Result.Nodes.getNodeAs<Expr>(ExpressionId);
+    diag(BoolLiteral->getLocStart(), SimplifyDiagnostic)
+        << FixItHint::CreateReplacement(
+            SourceRange(Expression->getLocStart(), BoolLiteral->getLocEnd()),
+            getText(Result, *BoolLiteral));
+}
+
+void SimplifyBooleanExpr::replaceWithLeftExpression(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        CXXBoolLiteralExpr const *BoolLiteral,
+        std::string const Prefix)
+{
+    auto const *const Expression = Result.Nodes.getNodeAs<Expr>(ExpressionId);
+    diag(BoolLiteral->getLocStart(), SimplifyDiagnostic)
+        << FixItHint::CreateReplacement(
+            SourceRange(Expression->getLocStart(), BoolLiteral->getLocEnd()),
+            Prefix + getText(Result, *Expression));
+}
+
+void SimplifyBooleanExpr::replaceWithThenStatement(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        CXXBoolLiteralExpr const *TrueConditionRemoved)
+{
+    auto const *const IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
+    diag(TrueConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
+        << FixItHint::CreateReplacement(
+            IfStatement->getSourceRange(),
+            getText(Result, *IfStatement->getThen()));
+}
+
+void SimplifyBooleanExpr::replaceWithElseStatement(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        CXXBoolLiteralExpr const *FalseConditionRemoved)
+{
+    auto const *const IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
+    auto const *const ElseStatement = IfStatement->getElse();
+    diag(FalseConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
+        << FixItHint::CreateReplacement(
+            IfStatement->getSourceRange(),
+            ElseStatement ? getText(Result, *ElseStatement) : "");
+}
+
+void SimplifyBooleanExpr::replaceWithCondition(
+        const ast_matchers::MatchFinder::MatchResult &Result,
+        ConditionalOperator const *Ternary,
+        std::string Prefix)
+{
+    diag(Ternary->getTrueExpr()->getLocStart(),
+            "Redundant boolean literal in ternary expression result.")
+        << FixItHint::CreateReplacement(
+            Ternary->getSourceRange(),
+            Prefix + getText(Result, *Ternary->getCond()));
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

test/clang-tidy/readability-simplify-bool-expr.cpp

+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-simplify-boolean-expr %t
+// REQUIRES: shell
+
+bool a1 = false;
+
+//=-=-=-=-=-=-= operator ==
+bool aa = false == a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool aa = !a1;{{$}}
+bool ab = true == a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool ab = a1;{{$}}
+bool a2 = a1 == false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool a2 = !a1;{{$}}
+bool a3 = a1 == true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool a3 = a1;{{$}}
+
+//=-=-=-=-=-=-= operator !=
+bool n1 = a1 != false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool n1 = a1;{{$}}
+bool n2 = a1 != true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool n2 = !a1;{{$}}
+bool n3 = false != a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool n3 = a1;{{$}}
+bool n4 = true != a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool n4 = !a1;{{$}}
+
+//=-=-=-=-=-=-= operator ||
+bool a4 = a1 || false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool a4 = a1;{{$}}
+bool a5 = a1 || true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool a5 = true;{{$}}
+bool a6 = false || a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool a6 = a1;{{$}}
+bool a7 = true || a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool a7 = true;{{$}}
+
+//=-=-=-=-=-=-= operator &&
+bool a8 = a1 && false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// X-CHECK-FIXES: {{^}}bool a8 = false;{{$}}
+bool a9 = a1 && true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool a9 = a1;{{$}}
+bool ac = false && a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool ac = false;{{$}}
+bool ad = true && a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool ad = a1;{{$}}
+
+void if_with_bool_literal_condition()
+{
+    int i = 0;
+    if (false) {
+        i = 1;
+    } else {
+        i = 2;
+    }
+    i = 3;
+    // CHECK-MESSAGES: :[[@LINE-6]]:9: warning: Redundant boolean constant in if statement condition. [readability-simplify-boolean-expr]
+    // CHECK-FIXES: {{^    }}int i = 0;{{$}}
+    // CHECK-FIXES-NEXT: {{^    }}{{{$}}
+    // CHECK-FIXES-NEXT: {{^        }}i = 2;{{$}}
+    // CHECK-FIXES-NEXT: {{^    }}}{{$}}
+    // CHECK-FIXES-NEXT: {{^    }}i = 3;{{$}}
+
+    i = 4;
+    if (true) {
+        i = 5;
+    } else {
+        i = 6;
+    }
+    i = 7;
+    // CHECK-MESSAGES: :[[@LINE-6]]:9: warning: Redundant boolean constant in if statement condition. [readability-simplify-boolean-expr]
+    // CHECK-FIXES: {{^    }}i = 4;{{$}}
+    // CHECK-FIXES-NEXT: {{^    }}{{{$}}
+    // CHECK-FIXES-NEXT: {{^        }}i = 5;{{$}}
+    // CHECK-FIXES-NEXT: {{^    }}}{{$}}
+    // CHECK-FIXES-NEXT: {{^    }}i = 7;{{$}}
+
+    i = 8;
+    if (false) {
+        i = 9;
+    }
+    i = 11;
+    // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Redundant boolean constant in if statement condition. [readability-simplify-boolean-expr]
+    // CHECK-FIXES: {{^    }}i = 8;{{$}}
+    // CHECK-FIXES-NEXT: {{^    $}}
+    // CHECK-FIXES-NEXT: {{^    }}i = 11;{{$}}
+}
+
+void operator_equals()
+{
+    int i = 0;
+    bool b1 = (i > 2);
+    if (b1 == true) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (b1) {{{$}}
+        i = 5;
+    } else {
+        i = 6;
+    }
+    bool b2 = (i > 4);
+    if (b2 == false) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (!b2) {{{$}}
+        i = 7;
+    } else {
+        i = 9;
+    }
+    bool b3 = (i > 6);
+    if (true == b3) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (b3) {{{$}}
+        i = 10;
+    } else {
+        i = 11;
+    }
+    bool b4 = (i > 8);
+    if (false == b4) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (!b4) {{{$}}
+        i = 12;
+    } else {
+        i = 13;
+    }
+}
+
+void operator_or()
+{
+    int i = 0;
+    bool b5 = (i > 10);
+    if (b5 || false) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (b5) {{{$}}
+        i = 14;
+    } else {
+        i = 15;
+    }
+    bool b6 = (i > 10);
+    if (b6 || true) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (true) {{{$}}
+        i = 16;
+    } else {
+        i = 17;
+    }
+    bool b7 = (i > 10);
+    if (false || b7) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // X-CHECK-FIXES: {{^    }}if (b4) {{{$}}
+        i = 18;
+    } else {
+        i = 19;
+    }
+    bool b8 = (i > 10);
+    if (true || b8) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // X-CHECK-FIXES: {{^    }}if (true) {{{$}}
+        i = 20;
+    } else {
+        i = 21;
+    }
+}
+
+void operator_and()
+{
+    int i = 0;
+    bool b9 = (i > 20);
+    if (b9 && false) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (false) {{{$}}
+        i = 22;
+    } else {
+        i = 23;
+    }
+    bool ba = (i > 20);
+    if (ba && true) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (ba) {{{$}}
+        i = 24;
+    } else {
+        i = 25;
+    }
+    bool bb = (i > 20);
+    if (false && bb) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (false) {{{$}}
+        i = 26;
+    } else {
+        i = 27;
+    }
+    bool bc = (i > 20);
+    if (true && bc) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (bc) {{{$}}
+        i = 28;
+    } else {
+        i = 29;
+    }
+}
+
+void ternary_operator()
+{
+    int i = 0;
+    bool bd = (i > 20) ? true : false;
+    // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: Redundant boolean literal in ternary expression result. [readability-simplify-boolean-expr]
+    // CHECK-FIXES: {{^    }}bool bd = (i > 20);{{$}}
+    bool be = (i > 20) ? false : true;
+    // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: Redundant boolean literal in ternary expression result. [readability-simplify-boolean-expr]
+    // CHECK-FIXES: {{^    }}bool be = !(i > 20);{{$}}
+}
+
+void operator_not_equal()
+{
+    int i = 0;
+    bool bf = (i > 20);
+    if (false != bf) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (bf) {{{$}}
+        i = 30;
+    } else {
+        i = 31;
+    }
+    bool bg = (i > 20);
+    if (true != bg) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (!bg) {{{$}}
+        i = 32;
+    } else {
+        i = 33;
+    }
+    bool bh = (i > 20);
+    if (bh != false) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (bh) {{{$}}
+        i = 34;
+    } else {
+        i = 35;
+    }
+    bool bi = (i > 20);
+    if (bi != true) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (!bi) {{{$}}
+        i = 36;
+    } else {
+        i = 37;
+    }
+}
+
+void nested_booleans()
+{
+    if (false || (true || false)) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (false || (true)) {{{$}}
+    }
+    if (true && (true || false)) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (true && (true)) {{{$}}
+    }
+    if (false || (true && false)) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (false || (false)) {{{$}}
+    }
+    if (true && (true && false)) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+        // CHECK-FIXES: {{^    }}if (true && (false)) {{{$}}
+    }
+}
+
+static constexpr bool truthy()
+{
+    return true;
+}
+
+#define HAS_XYZ_FEATURE true
+
+void macros_and_constexprs()
+{
+    int i = 0;
+    bool b = (i == 0);
+    if (b && truthy()) {
+        // leave this alone; if you want it simplified, then you should
+        // inline the constexpr function first.
+        i = 1;
+    }
+    i = 2;
+    if (b && HAS_XYZ_FEATURE) {
+        // leave this alone; if you want it simplified, then you should
+        // inline the macro first.
+        i = 3;
+    }
+    i = 4;
+}