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 ↗

(On Diff #19999)

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

34 ↗

(On Diff #19999)

I'd go with

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

as it's more readable due to fewer consts.

282 ↗

(On Diff #19999)

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

clang-tidy/readability/SimplifyBooleanExpr.h

26 ↗

(On Diff #19999)

Please clang-format the code with the LLVM style.

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

12

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

63

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 ↗

(On Diff #19999)

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

51 ↗

(On Diff #19999)

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

53 ↗

(On Diff #19999)

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

77 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

How are these different from matchBoolBinOpExpr ?

210 ↗

(On Diff #19999)

StringRef

221 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

Comment is a copy-paste from some other check.

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

51 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

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

77 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

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

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

16 ↗

(On Diff #19999)

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

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

63

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

236 ↗

(On Diff #20946)

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

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

63

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

20 ↗

(On Diff #20946)

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

32 ↗

(On Diff #20946)

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

53 ↗

(On Diff #20946)

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.

54 ↗

(On Diff #20946)

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

57 ↗

(On Diff #20946)

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

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

66 ↗

(On Diff #20946)

How about putting this into the ReturnsBool function?

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

300 ↗

(On Diff #20946)

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.

301 ↗

(On Diff #20946)

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

304 ↗

(On Diff #20946)

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.

306 ↗

(On Diff #20946)

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

clang-tidy/readability/SimplifyBooleanExpr.h

23–34 ↗

(On Diff #20946)

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 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

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

20 ↗

(On Diff #20946)

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 :-).

53 ↗

(On Diff #20946)

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?).

54 ↗

(On Diff #20946)

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?

57 ↗

(On Diff #20946)

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.

66 ↗

(On Diff #20946)

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.

236 ↗

(On Diff #20946)

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.

301 ↗

(On Diff #20946)

Nice! I will be using that.

304 ↗

(On Diff #20946)

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

63

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

What are we gaining?

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExpr.cpp

97 ↗

(On Diff #19999)

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 ↗

(On Diff #19999)

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

[alexfh]

clang-tidy/readability/SimplifyBooleanExpr.cpp

20 ↗

(On Diff #20946)

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 ;)

54 ↗

(On Diff #20946)

"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 ;)

57 ↗

(On Diff #20946)

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.

66 ↗

(On Diff #20946)

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)));

304 ↗

(On Diff #20946)

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

20 ↗

(On Diff #20946)

What about this?

54 ↗

(On Diff #20946)

ditto

57 ↗

(On Diff #20946)

ditto

306 ↗

(On Diff #20946)

ditto

[sbenza]

clang-tidy/readability/SimplifyBooleanExpr.cpp

62 ↗

(On Diff #21088)

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

236 ↗

(On Diff #20946)

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

62 ↗

(On Diff #21088)

Fixed.

20 ↗

(On Diff #20946)

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.

32 ↗

(On Diff #20946)

Fixed

53 ↗

(On Diff #20946)

Fixed.

54 ↗

(On Diff #20946)

Fixed

57 ↗

(On Diff #20946)

Fixed

66 ↗

(On Diff #20946)

Fixed

236 ↗

(On Diff #20946)

Fixed

300 ↗

(On Diff #20946)

Fixed

301 ↗

(On Diff #20946)

Fixed

304 ↗

(On Diff #20946)

Fixed

306 ↗

(On Diff #20946)

Fixed

21 ↗

(On Diff #19999)

Fixed.

34 ↗

(On Diff #19999)

Fixed.

51 ↗

(On Diff #19999)

Fixed

53 ↗

(On Diff #19999)

Fixed

77 ↗

(On Diff #19999)

I'm investigating alternatives to the current implementation.

210 ↗

(On Diff #19999)

Fixed

221 ↗

(On Diff #19999)

Fixed

282 ↗

(On Diff #19999)

Fixed

clang-tidy/readability/SimplifyBooleanExpr.h

23–34 ↗

(On Diff #20946)

Fixed.

19 ↗

(On Diff #19999)

Fixed

26 ↗

(On Diff #19999)

Fixed.

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

12

Fixed

[LegalizeAdulthood]

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

[alexfh]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

255

Please use "const auto *" for pointers.

316

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

255

Fixed

316

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

245

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

251

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

245

Fixed

251

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

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

101

nit: Don't use else after a return.

105

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

116

nit: Use const auto *.

118

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

123

nit: Use Text.empty() instead.

[alexfh]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

105

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
   ContainerSizeEmptyCheck.cpp
   ElseAfterReturnCheck.cpp
   FunctionSizeCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   ShrinkToFitCheck.cpp
+  SimplifyBooleanExprCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTooling
   )

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ElseAfterReturnCheck.h"
 #include "FunctionSizeCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "ShrinkToFitCheck.h"
+#include "SimplifyBooleanExprCheck.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<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<RedundantStringCStrCheck>(
         "readability-redundant-string-cstr");
-    CheckFactories.registerCheck<ShrinkToFitCheck>("readability-shrink-to-fit");
+    CheckFactories.registerCheck<ShrinkToFitCheck>(
+        "readability-shrink-to-fit");
+    CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
+        "readability-simplify-boolean-expr");
   }
 };
 
 // Register the ReadabilityModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<ReadabilityModule>
     X("readability-module", "Adds readability-related checks.");
 
 } // namespace readability
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the ReadabilityModule.
 volatile int ReadabilityModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/readability/SimplifyBooleanExprCheck.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 Looks for boolean expressions involving boolean constants and
+// simplifies them to use the appropriate boolean expression directly.
+///
+/// Examples:
+/// `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);`
+///
+class SimplifyBooleanExprCheck : public ClangTidyCheck {
+public:
+  SimplifyBooleanExprCheck(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,
+                          StringRef OperatorName, StringRef BooleanId);
+
+  void matchExprBinOpBool(ast_matchers::MatchFinder *Finder, bool Value,
+                          StringRef OperatorName, StringRef BooleanId);
+
+  void matchBoolCompOpExpr(ast_matchers::MatchFinder *Finder, bool Value,
+                           StringRef OperatorName, StringRef BooleanId);
+
+  void matchExprCompOpBool(ast_matchers::MatchFinder *Finder, bool Value,
+                           StringRef OperatorName, StringRef BooleanId);
+
+  void matchBoolCondition(ast_matchers::MatchFinder *Finder, bool Value,
+                          StringRef BooleanId);
+
+  void matchTernaryResult(ast_matchers::MatchFinder *Finder, bool Value,
+                          StringRef TernaryId);
+
+  void matchIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
+                          StringRef Id);
+
+  void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
+                          StringRef Id);
+
+  void
+  replaceWithExpression(const ast_matchers::MatchFinder::MatchResult &Result,
+                        const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS,
+                        bool Negated = false);
+
+  void
+  replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
+                           const CXXBoolLiteralExpr *BoolLiteral);
+
+  void
+  replaceWithElseStatement(const ast_matchers::MatchFinder::MatchResult &Result,
+                           const CXXBoolLiteralExpr *FalseConditionRemoved);
+
+  void
+  replaceWithCondition(const ast_matchers::MatchFinder::MatchResult &Result,
+                       const ConditionalOperator *Ternary,
+                       bool Negated = false);
+
+  void replaceWithReturnCondition(
+      const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
+      bool Negated = false);
+
+  void
+  replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
+                        const IfStmt *If, bool Negated = false);
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H

clang-tidy/readability/SimplifyBooleanExprCheck.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 "SimplifyBooleanExprCheck.h"
+#include "clang/Lex/Lexer.h"
+
+#include <cassert>
+#include <string>
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace {
+
+StringRef getText(const MatchFinder::MatchResult &Result, SourceRange Range) {
+  return Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
+                              *Result.SourceManager,
+                              Result.Context->getLangOpts());
+}
+
+template <typename T>
+StringRef getText(const MatchFinder::MatchResult &Result, T &Node) {
+  return getText(Result, Node.getSourceRange());
+}
+
+const char RightExpressionId[] = "bool-op-expr-yields-expr";
+const char LeftExpressionId[] = "expr-op-bool-yields-expr";
+const char NegatedRightExpressionId[] = "bool-op-expr-yields-not-expr";
+const char NegatedLeftExpressionId[] = "expr-op-bool-yields-not-expr";
+const char ConditionThenStmtId[] = "if-bool-yields-then";
+const char ConditionElseStmtId[] = "if-bool-yields-else";
+const char TernaryId[] = "ternary-bool-yields-condition";
+const char TernaryNegatedId[] = "ternary-bool-yields-not-condition";
+const char IfReturnsBoolId[] = "if-return";
+const char IfReturnsNotBoolId[] = "if-not-return";
+const char ThenLiteralId[] = "then-literal";
+const char IfAssignVariableId[] = "if-assign-lvalue";
+const char IfAssignLocId[] = "if-assign-loc";
+const char IfAssignBoolId[] = "if-assign";
+const char IfAssignNotBoolId[] = "if-assign-not";
+const char IfAssignObjId[] = "if-assign-obj";
+
+const char IfStmtId[] = "if";
+const char LHSId[] = "lhs-expr";
+const char RHSId[] = "rhs-expr";
+
+const char SimplifyOperatorDiagnostic[] =
+    "redundant boolean literal supplied to boolean operator";
+const char SimplifyConditionDiagnostic[] =
+    "redundant boolean literal in if statement condition";
+
+const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult &Result,
+                                         StringRef Id) {
+  const auto *Literal = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(Id);
+  return (Literal &&
+          Result.SourceManager->isMacroBodyExpansion(Literal->getLocStart()))
+             ? nullptr
+             : Literal;
+}
+
+internal::Matcher<Stmt> ReturnsBool(bool Value, StringRef Id = "") {
+  auto SimpleReturnsBool = returnStmt(
+      has(boolLiteral(equals(Value)).bind(Id.empty() ? "ignored" : Id)));
+  return anyOf(SimpleReturnsBool,
+               compoundStmt(statementCountIs(1), has(SimpleReturnsBool)));
+}
+
+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::pair<BinaryOperatorKind, BinaryOperatorKind> opposites[] = {
+    std::make_pair(BO_LT, BO_GE), std::make_pair(BO_GT, BO_LE),
+    std::make_pair(BO_EQ, BO_NE)};
+
+bool isNegatable(BinaryOperatorKind Opcode) {
+  for (auto NegatableOp : opposites) {
+    if (Opcode == NegatableOp.first || Opcode == NegatableOp.second) {
+      return true;
+    }
+  }
+  return false;
+}
+
+StringRef negatedOperator(const BinaryOperator *BinOp) {
+  const BinaryOperatorKind Opcode = BinOp->getOpcode();
+  for (auto NegatableOp : opposites) {
+    if (Opcode == NegatableOp.first) {
+      return BinOp->getOpcodeStr(NegatableOp.second);
+    } else if (Opcode == NegatableOp.second) {
+      return BinOp->getOpcodeStr(NegatableOp.first);
+    }
+  }
+  assert(!"Non-negatable operator given to negatedOperator");
+  return BinOp->getOpcodeStr();
+}
+
+std::string replacementExpression(const MatchFinder::MatchResult &Result,
+                                  bool Negated, const Expr &E) {
+  StringRef Text;
+  const Expr *Expression = &E;
+  while (const auto *Parenthesized = dyn_cast<ParenExpr>(Expression)) {
+    Expression = Parenthesized->getSubExpr();
+  }
+  if (const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(Expression)) {
+    if (BinOp && Negated && isNegatable(BinOp->getOpcode())) {
+      Text = (getText(Result, *BinOp->getLHS()) + " " + negatedOperator(BinOp) +
+              " " + getText(Result, *BinOp->getRHS())).str();
+      Negated = false;
+    }
+  }
+  if (Text.size() == 0) {
+    Text = getText(Result, *Expression);
+  }
+  return (Negated ? (needsParensAfterUnaryNegation(&E) ? "!(" + Text + ")"
+                                                       : "!" + Text)
+                  : Text).str();
+}
+
+} // namespace
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void SimplifyBooleanExprCheck::matchBoolBinOpExpr(MatchFinder *Finder,
+                                                  bool Value,
+                                                  StringRef OperatorName,
+                                                  StringRef BooleanId) {
+  Finder->addMatcher(
+      binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
+                     hasLHS(allOf(expr().bind(LHSId),
+                                  boolLiteral(equals(Value)).bind(BooleanId))),
+                     hasRHS(expr().bind(RHSId)),
+                     unless(hasRHS(hasDescendant(boolLiteral())))),
+      this);
+}
+
+void SimplifyBooleanExprCheck::matchExprBinOpBool(MatchFinder *Finder,
+                                                  bool Value,
+                                                  StringRef OperatorName,
+                                                  StringRef BooleanId) {
+  Finder->addMatcher(
+      binaryOperator(
+          isExpansionInMainFile(), hasOperatorName(OperatorName),
+          hasLHS(expr().bind(LHSId)),
+          unless(hasLHS(anyOf(boolLiteral(), hasDescendant(boolLiteral())))),
+          hasRHS(allOf(expr().bind(RHSId),
+                       boolLiteral(equals(Value)).bind(BooleanId)))),
+      this);
+}
+
+void SimplifyBooleanExprCheck::matchBoolCompOpExpr(MatchFinder *Finder,
+                                                   bool Value,
+                                                   StringRef OperatorName,
+                                                   StringRef BooleanId) {
+  Finder->addMatcher(
+      binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
+                     hasLHS(allOf(expr().bind(LHSId),
+                                  ignoringImpCasts(boolLiteral(equals(Value))
+                                                       .bind(BooleanId)))),
+                     hasRHS(expr().bind(RHSId)),
+                     unless(hasRHS(hasDescendant(boolLiteral())))),
+      this);
+}
+
+void SimplifyBooleanExprCheck::matchExprCompOpBool(MatchFinder *Finder,
+                                                   bool Value,
+                                                   StringRef OperatorName,
+                                                   StringRef BooleanId) {
+  Finder->addMatcher(
+      binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
+                     unless(hasLHS(hasDescendant(boolLiteral()))),
+                     hasLHS(expr().bind(LHSId)),
+                     hasRHS(allOf(expr().bind(RHSId),
+                                  ignoringImpCasts(boolLiteral(equals(Value))
+                                                       .bind(BooleanId))))),
+      this);
+}
+
+void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder,
+                                                  bool Value,
+                                                  StringRef BooleanId) {
+  Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+                            hasCondition(boolLiteral(equals(Value))
+                                             .bind(BooleanId))).bind(IfStmtId),
+                     this);
+}
+
+void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
+                                                  bool Value,
+                                                  StringRef TernaryId) {
+  Finder->addMatcher(
+      conditionalOperator(isExpansionInMainFile(),
+                          hasTrueExpression(boolLiteral(equals(Value))),
+                          hasFalseExpression(boolLiteral(equals(!Value))))
+          .bind(TernaryId),
+      this);
+}
+
+void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
+                                                  bool Value, StringRef Id) {
+  Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+                            hasThen(ReturnsBool(Value, ThenLiteralId)),
+                            hasElse(ReturnsBool(!Value))).bind(Id),
+                     this);
+}
+
+void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
+                                                  bool Value, StringRef Id) {
+  auto SimpleThen = binaryOperator(
+      hasOperatorName("="),
+      hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId)))),
+      hasLHS(expr().bind(IfAssignVariableId)),
+      hasRHS(boolLiteral(equals(Value)).bind(IfAssignLocId)));
+  auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1),
+                                             hasAnySubstatement(SimpleThen)));
+  auto SimpleElse = binaryOperator(
+      hasOperatorName("="),
+      hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId)))),
+      hasRHS(boolLiteral(equals(!Value))));
+  auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
+                                             hasAnySubstatement(SimpleElse)));
+  Finder->addMatcher(
+      ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
+      this);
+}
+
+void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
+  matchBoolBinOpExpr(Finder, true, "&&", RightExpressionId);
+  matchBoolBinOpExpr(Finder, false, "||", RightExpressionId);
+  matchExprBinOpBool(Finder, false, "&&", RightExpressionId);
+  matchExprBinOpBool(Finder, true, "||", RightExpressionId);
+  matchBoolCompOpExpr(Finder, true, "==", RightExpressionId);
+  matchBoolCompOpExpr(Finder, false, "!=", RightExpressionId);
+
+  matchExprBinOpBool(Finder, true, "&&", LeftExpressionId);
+  matchExprBinOpBool(Finder, false, "||", LeftExpressionId);
+  matchBoolBinOpExpr(Finder, false, "&&", LeftExpressionId);
+  matchBoolBinOpExpr(Finder, true, "||", 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);
+
+  matchIfReturnsBool(Finder, true, IfReturnsBoolId);
+  matchIfReturnsBool(Finder, false, IfReturnsNotBoolId);
+
+  matchIfAssignsBool(Finder, true, IfAssignBoolId);
+  matchIfAssignsBool(Finder, false, IfAssignNotBoolId);
+}
+
+void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const CXXBoolLiteralExpr *LeftRemoved =
+          getBoolLiteral(Result, RightExpressionId)) {
+    replaceWithExpression(Result, LeftRemoved, false);
+  } else if (const CXXBoolLiteralExpr *RightRemoved =
+                 getBoolLiteral(Result, LeftExpressionId)) {
+    replaceWithExpression(Result, RightRemoved, true);
+  } else if (const CXXBoolLiteralExpr *NegatedLeftRemoved =
+                 getBoolLiteral(Result, NegatedRightExpressionId)) {
+    replaceWithExpression(Result, NegatedLeftRemoved, false, true);
+  } else if (const CXXBoolLiteralExpr *NegatedRightRemoved =
+                 getBoolLiteral(Result, NegatedLeftExpressionId)) {
+    replaceWithExpression(Result, NegatedRightRemoved, true, true);
+  } else if (const CXXBoolLiteralExpr *TrueConditionRemoved =
+                 getBoolLiteral(Result, ConditionThenStmtId)) {
+    replaceWithThenStatement(Result, TrueConditionRemoved);
+  } else if (const CXXBoolLiteralExpr *FalseConditionRemoved =
+                 getBoolLiteral(Result, ConditionElseStmtId)) {
+    replaceWithElseStatement(Result, FalseConditionRemoved);
+  } else if (const auto *Ternary =
+                 Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId)) {
+    replaceWithCondition(Result, Ternary);
+  } else if (const auto *TernaryNegated =
+                 Result.Nodes.getNodeAs<ConditionalOperator>(
+                     TernaryNegatedId)) {
+    replaceWithCondition(Result, TernaryNegated, true);
+  } else if (const auto *If = Result.Nodes.getNodeAs<IfStmt>(IfReturnsBoolId)) {
+    replaceWithReturnCondition(Result, If);
+  } else if (const auto *IfNot =
+                 Result.Nodes.getNodeAs<IfStmt>(IfReturnsNotBoolId)) {
+    replaceWithReturnCondition(Result, IfNot, true);
+  } else if (const auto *IfAssign =
+                 Result.Nodes.getNodeAs<IfStmt>(IfAssignBoolId)) {
+    replaceWithAssignment(Result, IfAssign);
+  } else if (const auto *IfAssignNot =
+                 Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId)) {
+    replaceWithAssignment(Result, IfAssignNot, true);
+  }
+}
+
+void SimplifyBooleanExprCheck::replaceWithExpression(
+    const ast_matchers::MatchFinder::MatchResult &Result,
+    const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, bool Negated) {
+  const auto *LHS = Result.Nodes.getNodeAs<Expr>(LHSId);
+  const auto *RHS = Result.Nodes.getNodeAs<Expr>(RHSId);
+  std::string Replacement =
+      replacementExpression(Result, Negated, *(UseLHS ? LHS : RHS));
+  SourceLocation Start = LHS->getLocStart();
+  SourceLocation End = RHS->getLocEnd();
+  diag(BoolLiteral->getLocStart(), SimplifyOperatorDiagnostic)
+      << FixItHint::CreateReplacement(SourceRange(Start, End), Replacement);
+}
+
+void SimplifyBooleanExprCheck::replaceWithThenStatement(
+    const MatchFinder::MatchResult &Result,
+    const CXXBoolLiteralExpr *TrueConditionRemoved) {
+  const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
+  diag(TrueConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
+      << FixItHint::CreateReplacement(IfStatement->getSourceRange(),
+                                      getText(Result, *IfStatement->getThen()));
+}
+
+void SimplifyBooleanExprCheck::replaceWithElseStatement(
+    const MatchFinder::MatchResult &Result,
+    const CXXBoolLiteralExpr *FalseConditionRemoved) {
+  const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
+  const Stmt *ElseStatement = IfStatement->getElse();
+  diag(FalseConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
+      << FixItHint::CreateReplacement(
+          IfStatement->getSourceRange(),
+          ElseStatement ? getText(Result, *ElseStatement) : "");
+}
+
+void SimplifyBooleanExprCheck::replaceWithCondition(
+    const MatchFinder::MatchResult &Result, const ConditionalOperator *Ternary,
+    bool Negated) {
+  std::string Replacement =
+      replacementExpression(Result, Negated, *Ternary->getCond());
+  diag(Ternary->getTrueExpr()->getLocStart(),
+       "redundant boolean literal in ternary expression result")
+      << FixItHint::CreateReplacement(Ternary->getSourceRange(), Replacement);
+}
+
+void SimplifyBooleanExprCheck::replaceWithReturnCondition(
+    const MatchFinder::MatchResult &Result, const IfStmt *If, bool Negated) {
+  StringRef Terminator = isa<CompoundStmt>(If->getElse()) ? ";" : "";
+  std::string Condition =
+      replacementExpression(Result, Negated, *If->getCond());
+  std::string Replacement = ("return " + Condition + Terminator).str();
+  SourceLocation Start =
+      Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(ThenLiteralId)->getLocStart();
+  diag(Start, "redundant boolean literal in conditional return statement")
+      << FixItHint::CreateReplacement(If->getSourceRange(), Replacement);
+}
+
+void SimplifyBooleanExprCheck::replaceWithAssignment(
+    const MatchFinder::MatchResult &Result, const IfStmt *IfAssign,
+    bool Negated) {
+  SourceRange Range = IfAssign->getSourceRange();
+  StringRef VariableName =
+      getText(Result, *Result.Nodes.getNodeAs<Expr>(IfAssignVariableId));
+  StringRef Terminator = isa<CompoundStmt>(IfAssign->getElse()) ? ";" : "";
+  std::string Condition =
+      replacementExpression(Result, Negated, *IfAssign->getCond());
+  std::string Replacement =
+      (VariableName + " = " + Condition + Terminator).str();
+  SourceLocation Location =
+      Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(IfAssignLocId)->getLocStart();
+  this->diag(Location, "redundant boolean literal in conditional assignment")
+      << FixItHint::CreateReplacement(Range, Replacement);
+}
+
+} // 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 literal supplied to boolean operator [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^bool aa = !a1;$}}
+bool ab = true == a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool ab = a1;$}}
+bool a2 = a1 == false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a2 = !a1;$}}
+bool a3 = a1 == true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a3 = a1;$}}
+
+//=-=-=-=-=-=-= operator !=
+bool n1 = a1 != false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n1 = a1;$}}
+bool n2 = a1 != true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n2 = !a1;$}}
+bool n3 = false != a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n3 = a1;$}}
+bool n4 = true != a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n4 = !a1;$}}
+
+//=-=-=-=-=-=-= operator ||
+bool a4 = a1 || false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a4 = a1;$}}
+bool a5 = a1 || true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a5 = true;$}}
+bool a6 = false || a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a6 = a1;$}}
+bool a7 = true || a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a7 = true;$}}
+
+//=-=-=-=-=-=-= operator &&
+bool a8 = a1 && false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a8 = false;$}}
+bool a9 = a1 && true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a9 = a1;$}}
+bool ac = false && a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool ac = false;$}}
+bool ad = true && a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// 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]]:7: warning: {{.*}} in if statement condition
+  // 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]]:7: warning: {{.*}} in if statement condition
+  // 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]]:7: warning: {{.*}} in if statement condition
+  // 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]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(b1\) {$}}
+    i = 5;
+  } else {
+    i = 6;
+  }
+  bool b2 = (i > 4);
+  if (b2 == false) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(!b2\) {$}}
+    i = 7;
+  } else {
+    i = 9;
+  }
+  bool b3 = (i > 6);
+  if (true == b3) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(b3\) {$}}
+    i = 10;
+  } else {
+    i = 11;
+  }
+  bool b4 = (i > 8);
+  if (false == b4) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // 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]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(b5\) {$}}
+    i = 14;
+  } else {
+    i = 15;
+  }
+  bool b6 = (i > 10);
+  if (b6 || true) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(true\) {$}}
+    i = 16;
+  } else {
+    i = 17;
+  }
+  bool b7 = (i > 10);
+  if (false || b7) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(b7\) {$}}
+    i = 18;
+  } else {
+    i = 19;
+  }
+  bool b8 = (i > 10);
+  if (true || b8) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // 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]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(false\) {$}}
+    i = 22;
+  } else {
+    i = 23;
+  }
+  bool ba = (i > 20);
+  if (ba && true) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(ba\) {$}}
+    i = 24;
+  } else {
+    i = 25;
+  }
+  bool bb = (i > 20);
+  if (false && bb) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(false\) {$}}
+    i = 26;
+  } else {
+    i = 27;
+  }
+  bool bc = (i > 20);
+  if (true && bc) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // 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]]:24: warning: {{.*}} in ternary expression result
+  // CHECK-FIXES: {{^  bool bd = i > 20;$}}
+
+  bool be = (i > 20) ? false : true;
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: {{.*}} in ternary expression result
+  // CHECK-FIXES: {{^  bool be = i <= 20;$}}
+
+  bool bf = ((i > 20)) ? false : true;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: {{.*}} in ternary expression result
+  // CHECK-FIXES: {{^  bool bf = i <= 20;$}}
+}
+
+void operator_not_equal() {
+  int i = 0;
+  bool bf = (i > 20);
+  if (false != bf) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(bf\) {$}}
+    i = 30;
+  } else {
+    i = 31;
+  }
+  bool bg = (i > 20);
+  if (true != bg) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(!bg\) {$}}
+    i = 32;
+  } else {
+    i = 33;
+  }
+  bool bh = (i > 20);
+  if (bh != false) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(bh\) {$}}
+    i = 34;
+  } else {
+    i = 35;
+  }
+  bool bi = (i > 20);
+  if (bi != true) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(!bi\) {$}}
+    i = 36;
+  } else {
+    i = 37;
+  }
+}
+
+void nested_booleans() {
+  if (false || (true || false)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(false \|\| \(true\)\) {$}}
+  }
+  if (true && (true || false)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(true && \(true\)\) {$}}
+  }
+  if (false || (true && false)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{^  if \(false \|\| \(false\)\) {$}}
+  }
+  if (true && (true && false)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}} to boolean operator
+    // 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 == 1);
+  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;
+}
+
+bool conditional_return_statements(int i) {
+  if (i == 0) return true; else return false;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:22: warning: {{.*}} in conditional return statement
+// CHECK-FIXES:      {{^}}  return i == 0;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool conditional_return_statements_then_expr(int i, int j) {
+  if (i == j) return (i == 0); else return false;
+}
+
+bool conditional_return_statements_else_expr(int i, int j) {
+  if (i == j) return true; else return (i == 0);
+}
+
+bool negated_conditional_return_statements(int i) {
+  if (i == 0) return false; else return true;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:22: warning: {{.*}} in conditional return statement
+// CHECK-FIXES:      {{^}}  return i != 0;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool conditional_compound_return_statements(int i) {
+  if (i == 1) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return statement
+// CHECK-FIXES:      {{^}}bool conditional_compound_return_statements(int i) {{{$}}
+// CHECK-FIXES-NEXT: {{^}}  return i == 1;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool negated_conditional_compound_return_statements(int i) {
+  if (i == 1) {
+    return false;
+  } else {
+    return true;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return statement
+// CHECK-FIXES:      {{^}}bool negated_conditional_compound_return_statements(int i) {{{$}}
+// CHECK-FIXES-NEXT: {{^}}  return i != 1;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool conditional_return_statements_side_effects_then(int i) {
+  if (i == 2) {
+    macros_and_constexprs();
+    return true;
+  } else
+    return false;
+}
+
+bool negated_conditional_return_statements_side_effects_then(int i) {
+  if (i == 2) {
+    macros_and_constexprs();
+    return false;
+  } else
+    return true;
+}
+
+bool conditional_return_statements_side_effects_else(int i) {
+  if (i == 2)
+    return true;
+  else {
+    macros_and_constexprs();
+    return false;
+  }
+}
+
+bool negated_conditional_return_statements_side_effects_else(int i) {
+  if (i == 2)
+    return false;
+  else {
+    macros_and_constexprs();
+    return true;
+  }
+}
+
+void lambda_conditional_return_statements() {
+  auto lambda = [](int n) -> bool { if (n > 0) return true; else return false; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:55: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: {{^}}  auto lambda = [](int n) -> bool { return n > 0; };{{$}}
+
+  auto lambda2 = [](int n) -> bool {
+    if (n > 0) {
+        return true;
+    } else {
+        return false;
+    }
+  };
+  // CHECK-MESSAGES: :[[@LINE-5]]:16: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES:      {{^}}  auto lambda2 = [](int n) -> bool {{{$}}
+  // CHECK-FIXES-NEXT: {{^}}    return n > 0;{{$}}
+  // CHECK-FIXES-NEXT: {{^}}  };{{$}}
+
+  auto lambda3 = [](int n) -> bool { if (n > 0) {macros_and_constexprs(); return true; } else return false; };
+
+  auto lambda4 = [](int n) -> bool {
+    if (n > 0)
+        return true;
+    else {
+        macros_and_constexprs();
+        return false;
+    }
+  };
+
+  auto lambda5 = [](int n) -> bool { if (n > 0) return false; else return true; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:56: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: {{^}}  auto lambda5 = [](int n) -> bool { return n <= 0; };{{$}}
+
+  auto lambda6 = [](int n) -> bool {
+    if (n > 0) {
+        return false;
+    } else {
+        return true;
+    }
+  };
+  // CHECK-MESSAGES: :[[@LINE-5]]:16: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES:      {{^}}  auto lambda6 = [](int n) -> bool {{{$}}
+  // CHECK-FIXES-NEXT: {{^}}    return n <= 0;{{$}}
+  // CHECK-FIXES-NEXT: {{^}}  };{{$}}
+}
+
+void simple_conditional_assignment_statements(int i) {
+  bool b;
+  if (i > 10)
+    b = true;
+  else
+    b = false;
+  bool bb = false;
+  // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment
+  // CHECK-FIXES: bool b;
+  // CHECK-FIXES: {{^  }}b = i > 10;{{$}}
+  // CHECK-FIXES: bool bb = false;
+
+  bool c;
+  if (i > 20)
+    c = false;
+  else
+    c = true;
+  bool c2 = false;
+  // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment
+  // CHECK-FIXES: bool c;
+  // CHECK-FIXES: {{^  }}c = i <= 20;{{$}}
+  // CHECK-FIXES: bool c2 = false;
+
+  // unchanged; different variables
+  bool b2;
+  if (i > 12)
+    b = true;
+  else
+    b2 = false;
+
+  // unchanged; no else statement
+  bool b3;
+  if (i > 15)
+    b3 = true;
+
+  // unchanged; not boolean assignment
+  int j;
+  if (i > 17)
+    j = 10;
+  else
+    j = 20;
+
+  // unchanged; different variables assigned
+  int k = 0;
+  bool b4 = false;
+  if (i > 10)
+    b4 = true;
+  else
+    k = 10;
+}
+
+void complex_conditional_assignment_statements(int i) {
+  bool d;
+  if (i > 30) {
+    d = true;
+  } else {
+    d = false;
+  }
+  d = false;
+  // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in conditional assignment
+  // CHECK-FIXES: bool d;
+  // CHECK-FIXES: {{^  }}d = i > 30;{{$}}
+  // CHECK-FIXES: d = false;
+
+  bool e;
+  if (i > 40) {
+    e = false;
+  } else {
+    e = true;
+  }
+  e = false;
+  // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in conditional assignment
+  // CHECK-FIXES: bool e;
+  // CHECK-FIXES: {{^  }}e = i <= 40;{{$}}
+  // CHECK-FIXES: e = false;
+
+  // unchanged; no else statement
+  bool b3;
+  if (i > 15) {
+    b3 = true;
+  }
+
+  // unchanged; not a boolean assignment
+  int j;
+  if (i > 17) {
+    j = 10;
+  } else {
+    j = 20;
+  }
+
+  // unchanged; multiple statements
+  bool f;
+  if (j > 10) {
+    j = 10;
+    f = true;
+  } else {
+    j = 20;
+    f = false;
+  }
+
+  // unchanged; multiple statements
+  bool g;
+  if (j > 10)
+    f = true;
+  else {
+    j = 20;
+    f = false;
+  }
+
+  // unchanged; multiple statements
+  bool h;
+  if (j > 10) {
+    j = 10;
+    f = true;
+  } else
+    f = false;
+}