[clang-tidy] New check `bugprone-redundant-branch-condition`

[baloghadamsoftware]

Summary

Checking the same condition again in a nested if usually make no sense, except if the value of the expression could have been changed between the two checks. Although compilers may optimize this out, such code is suspicious: the programmer may have meant to check something else. Therefore it is worth to find such places in the code and notify the user about the problem.

This patch implements a basic check for this problem. Currently it only detects redundant conditions where the condition is a variable of integral type. It also detects the possible bug if the variable is in an or or and logical expression in the inner if and/or the variable is in an and logical expression in the outer if statement. Negated cases are not handled yet.

[baloghadamsoftware]

This check was made upon user request. I think it is a good base that can later be extended also for the negated cases (where the code inside the inner if never executes). That case is even more suspicious. I put it into misc because it is not such a bug that it should go into bugprone but rather a suspicious thing. The type of the bug is somewhat similar to the bugs detected by misc-redundant-expression so I think misc is the best place also for this one.

clang-tools-extra/docs/clang-tidy/checks/list.rst

135

What are these changes? I surely did not make them? Maybe the check adder Python script?

[njames93]

I feel the refactoring of Aliasing should be in its own PR, with this being a child of it.

clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp

39

Small nit, but using string literals for bound nodes is error prone. may I suggest defining some static strings and using those to prevent any typos.

40

Recursive matchers are a pain, but this will miss:

if (A && B && ... Y && Z)

64

This logic can be moved to the matcher.

varDecl(anyOf(parmVarDecl(), hasLocalStorage()))

68

Ditto:

varDecl(hasType(isVolatileQualified()))

72

Ditto, you get the point

88–89

use llvm::dyn_cast.

98

Again CharSourceRange::getTokenRange(Body->getEndLoc())

110

use llvm::dyn_cast.

116

That's because you are getting the character range, if you get the token range it'll get the correct range: CharSourceRange::getTokenRange

clang-tools-extra/docs/clang-tidy/checks/list.rst

135

Yeah, just undo those unrelated changes, maybe one day the python script will get sorted

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

78

Please keep alphabetical order in new checks list.

81

Please use double back-ticks for language constructs. Same in documentation.

[baloghadamsoftware]

Updated according to the comments.

[baloghadamsoftware]

clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp

40

I suppose that for this purpose we have to make our own matcher? Right? This also makes the fixits more complex. I think this could be done in a later phase. I added a FIXME now.

[Eugene.Zelenko]

clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst

6

Please use double back-ticks for language constructs. Same below.

[njames93]

These test cases don't show it, from what i can see this will erroneously match:

if (onFire) {
  tryPutFireOut();
} else {
  if (isHot && onFire) {
    scream();
  }
}

It appears that this will change the second if to:

if (isHot) {
  scream();
}

Rather than simply deleting the if as it will always evaluate to false. To fix this you could wrap the hasDescendant matcher inside a hasThen matcher.

Another case is if the condition variable is on the RHS of a redundant inner if condition, you can't delete the inner if when the LHS potentially has side effects.

if (onFire) {
  if (callTheFD() || onFire) {
    scream();
  }
}

If callTheFD has side effects this can't be replaced with this:

if (onFire) {
  scream();
}

You could however replace it with:

if (onFire) {
  callTheFD();
  scream();
}

For this simply check if the LHS has side effects.

[baloghadamsoftware]

Thank you for your help, @njames93! I updated the patch according to your comments. (And of course, also thank you, @Eugene.Zelenko, I also incorporated the changes you suggested.)

[baloghadamsoftware]

I tested this check on several open-source projects: BitCoin, CURL, OpenSSL, PostGreS/, TMux/ and Xerces. I only got two warnings issued by this checker, the first one in BitCoin:

src/checkqueue.h:93:25: redundant condition 'fMaster' [misc-redundant-condition]
                        if (fMaster)
                        ^

And indeed, this was a true positive that is fixed by now by this Commit. (I do not update these projects very often since I only use them for testing checks.)

The other one is in PostGreS:

src/backend/access/transam/xlog.c:7231:6: redundant condition 'switchedTLI' [misc-redundant-condition]
          if (switchedTLI && AllowCascadeReplication())
          ^

Althought the line numbers are changed by now, the bug is still there.

Thus the result is zero false positives and two true positives so far.

[whisperity]

As a future work, when something support ifs, it should also support ?:, and eliminate redundant conditionals from there, too.

I believe this check (together with misc-redundant-expr) should go into the readability- group.

[baloghadamsoftware]

True positive confirmed in PostGreS: Mailing List Archive. It is fixed by now, so you cannot see it on the link in my previous comment anymore.

[baloghadamsoftware]

6 findings in the LLVM Project. All of them confirmed as trues positives, 5 of them already fixed. Fix pending for the last one.

D82555, D8556, D82557, D82558, D82559, D82563

@aaron.ballman, @gribozavr2 or someone please review this patch.

[aaron.ballman]

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

I think this check should probably live in the bugprone module, WDYT?

clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp

33

This code can be simplified into:

const Stmt *MutS = MutAn.findMutation(Var);
return MutS && SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc());

74

This FIXME makes me worried that what we really want is a clang static analyzer check, since the check is already flow sensitive and needs to be data sensitive as well. Have you considered implementing this as a static analyzer check?

clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp

1098

Can you add some tests that exercise GNU expression statements, just to make sure we get the behavior correct?

if (({foo;})) {
} else if (({foo;})) {
}

if (foo) ({bar;});
else if (foo) ({bar;});

Another thing that's interesting to test is whether the redundant expression is in a subexpression which doesn't contribute to the value of the control expression:

if (foo(), val) {
} else if (foo(), other_val) {
}

(Similar can be tested with GNU statement expressions.)

[baloghadamsoftware]

clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp

74

Actually, it is only a proposal for improvement. Maybe FIXME is not the best wording for it. This is not a bug to fix, just a possibility to find more true positives. This check is very similar to the infinite loop check which is also in Tidy. In the Static Analyzer this check would be too heavyweight and would probably produce false positives.

clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp

1098

Do you mean that I should handle these cases as well? (Detect a bug, provide a fix.)

[aaron.ballman]

clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp

74

Ah, thank you for clarifying (though I think it's interesting that we come down on opposite sides of the FP question -- I would argue that the static analyzer check would have considerably fewer false positives than the tidy check when handling this case).

If the idea is that pointers and references need to be supported in a future patch, I think FIXME is a fine comment. If the idea is that pointers and references are a potential improvement, I'd probably clarify the comment to something more like FIXME: could potentially support tracking pointers and references in the future to improve catching true positives through aliases.

clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp

1098

If the check misses these cases, I think that's reasonable (just comment that the tests aren't currently handled). If the check diagnoses these cases but the fixit causes an issue, then I think that should be corrected in this patch. If it's trivial to support these cases (diagnosing or fixing), it's your call on whether you want to do so. I mostly just want the test coverage so I can know what to expect.

[baloghadamsoftware]

Updated according to the comments.

[baloghadamsoftware]

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

Based on my experience, bugpronbe is for checks whose findings are bugs that lead to undefined illegal memory access, behavior etc. This one is somewhere between that and readability. For example, redundant-expression is also in misc. But if you wish, I can move this checker into bugprone.

clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp

1098

The check currently misses these cases completely, so I left a FIXME there. I can do it in the near future, but first I would like to commit a first version so I can tell the user who requested it that his request is fulfilled.

[aaron.ballman]

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

The bugprone module has less to do with memory access or undefined behavior specifically and more to do with checks that should expose bugs in your code but don't belong to other categories. We try to keep checks out of misc as much as possible these days and this code pattern is attempting to find cases where the user potentially has a bug, so I think bugprone is the correct home for it.

However, bugprone has a similar check and I sort of wonder whether we should be extending that check rather than adding a separate one. See bugprone-branch-clone which catches the highly related situation where you have a chain of conditionals and one of the conditions is repeated. e.g.,

if (foo) {
  if (foo) { // Caught by misc-redundant-condition
  }
} else if (foo) { // Caught by bugprone-branch-clone
}

Even if we don't combine the checks, we should ensure their behaviors work well together (catch the same scenarios, don't repeat diagnostics, etc).

clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp

1098

Sounds great to me, thanks!

[baloghadamsoftware]

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

OK, I will put this into bugprone. The two checks may look similar, but this one is more complex because it does not check for the same condition in multiple branches of the same branch statement but checks whether the condition expression could be mutated between the two branch statements. Therefore the the whole logic is totally different, I see no point in merging the two. Should I create a test case then, where both are enabled?

[aaron.ballman]

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

Therefore the the whole logic is totally different, I see no point in merging the two.

I'm approaching the question from the perspective of the user, not a check author. These two checks do the same thing (find redundant conditions in flow control which look like they could be a logical mistake), so why should they be two separate checks? "Because the code looks different" isn't super compelling from that perspective, so I'm trying to figure out what the underlying principles are for the checks. If they're the same principle, they should be the same check. If they're fundamentally different principles, we should be able to explain when to use each check as part of their documentation without it sounding contrived. (Note, I'm not saying the checks have to be combined, but I am pushing back on adding an entirely new check that seems to be redundant from a user perspective.)

As a litmus test: can you think of a situation where you'd want only one of these two checks enabled? I can't think of a case where I'd care about redundancy in nested conditionals but not in chained conditionals (or vice versa) unless one of the checks had a really high false positive rate (which isn't much of a reason to split the checks anyway).

Should I create a test case then, where both are enabled?

If we wind up keeping the checks separate, then probably yes (also, the documentation for the checks should be updated to explain how they're different and why that's useful).

[baloghadamsoftware]

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

There are many checks that users almost always keep enabled together, but they are still separate checks. Now I looked into the branch clone check, combining them means simply copying them together because the logic is so much different.

Even from the user's perspective I see that branches with identical conditions are different from redundant checks. While the first one is a more serious bug (the second branch with the same condition is never executed) this one is slightly more than a readability error.

[aaron.ballman]

Poking @alexfh for more opinions about check similarity.

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

There are many checks that users almost always keep enabled together, but they are still separate checks.

I cannot find an instance with two checks that are this strongly related. The closest I can come are some of the C++ Core Guideline checks, but those are a different beast because they're part of a set of guidelines.

Now I looked into the branch clone check, combining them means simply copying them together because the logic is so much different.

This is not a very compelling reason to make a decision to split the checks, to me. We have plenty of checks with complex matchers and checking logic.

Even from the user's perspective I see that branches with identical conditions are different from redundant checks. While the first one is a more serious bug (the second branch with the same condition is never executed) this one is slightly more than a readability error.

I don't view the proposed check as having anything to do with readability. Readability is "how do I make the code do the same thing but look prettier?" and other stylistic choices. This check is finding a case where the programmer has potentially made a logical mistake with their code and is considerably more serious than a matter of style. To me, these are identical problems of programmer confusion.

The more I consider this, the more strongly I feel about combining the checks. I would have a hard time understanding why this code should require two different checks to be enabled to catch what amounts to the same logical confusion:

if (!foo) {
} else if (foo) { // This is a chain of conditionals with a redundant check
}

if (!foo) {
} else {
  if (foo) { // This is not a chain of conditionals, but it still has a redundant check
  }
}

@alexfh do you have thoughts on this?

[baloghadamsoftware]

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

I almost started to copy the two checks together when one of my colleagues told me that bugprone-branch-clone is not at all about redundant conditions but redundant bodies in different branches. (It was created by a formal student intern of our team.) Thus even from the user's perspective these checkers have nothing to do with each other.

[baloghadamsoftware]

Check renamed.

[baloghadamsoftware]

It seems that release notes were not renamed automatically. I updated it manually now.

[aaron.ballman]

Thanks to the new info, I think the check basically LGTM. Can you add some negative tests and documentation wording to make it clear that the check doesn't currently handle all logically equivalent predicates, like:

if (foo) {
} else {
  if (!foo) {
  }
}

// or
if (foo > 5) {
  if (foo > 3) {
  }
}

// or
if (foo > 5) {
  if (5 < foo) {
  }
}

(I'm assuming these cases aren't handled currently and that handling them isn't necessary to land the patch.)

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

46

Whoa, I had not noticed that about bugprone-branch-clone, but I reread the docs and you're absolutely right. Great catch and I'm sorry for the accidental noise.

[baloghadamsoftware]

In D81272#2218050, @aaron.ballman wrote:

Thanks to the new info, I think the check basically LGTM. Can you add some negative tests and documentation wording to make it clear that the check doesn't currently handle all logically equivalent predicates, like:

if (foo) {
} else {
  if (!foo) {
  }
}

// or
if (foo > 5) {
  if (foo > 3) {
  }
}

// or
if (foo > 5) {
  if (5 < foo) {
  }
}

(I'm assuming these cases aren't handled currently and that handling them isn't necessary to land the patch.)

Not even equality is handled yet, just single booleans.

[aaron.ballman]

In D81272#2218090, @baloghadamsoftware wrote:

In D81272#2218050, @aaron.ballman wrote:

Thanks to the new info, I think the check basically LGTM. Can you add some negative tests and documentation wording to make it clear that the check doesn't currently handle all logically equivalent predicates, like:

if (foo) {
} else {
  if (!foo) {
  }
}

// or
if (foo > 5) {
  if (foo > 3) {
  }
}

// or
if (foo > 5) {
  if (5 < foo) {
  }
}

(I'm assuming these cases aren't handled currently and that handling them isn't necessary to land the patch.)

Not even equality is handled yet, just single booleans.

That's what I was understanding too, thanks! LG with additional negative tests + doc changes.

[whisperity]

In D81272#2218050, @aaron.ballman wrote:

Thanks to the new info, I think the check basically LGTM. Can you add some negative tests and documentation wording to make it clear that the check doesn't currently handle all logically equivalent predicates, like:

if (foo) {
} else {
  if (!foo) {
  }
}

// or
if (foo > 5) {
  if (foo > 3) {
  }
}

// or
if (foo > 5) {
  if (5 < foo) {
  }
}

(I'm assuming these cases aren't handled currently and that handling them isn't necessary to land the patch.)

I'm afraid handling such cases will eventually invoke the same problems the RangeConstraintSolver has, and then the discussion about whether this is good in Tidy instead of CSA will be resurrected... 😦

One could come up with even more elaborate cases. Just hit something like below a few minutes ago while working:

const CallExpr* const C;

if (auto* CalledFun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
  // ...
  if (const auto* Fun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
    // ...
  }
  // ...
}

and the rabbit hole goes deeper. But it's pretty interesting how many cases the current check found as-is.

[aaron.ballman]

In D81272#2218173, @whisperity wrote:

In D81272#2218050, @aaron.ballman wrote:

Thanks to the new info, I think the check basically LGTM. Can you add some negative tests and documentation wording to make it clear that the check doesn't currently handle all logically equivalent predicates, like:

if (foo) {
} else {
  if (!foo) {
  }
}

// or
if (foo > 5) {
  if (foo > 3) {
  }
}

// or
if (foo > 5) {
  if (5 < foo) {
  }
}

(I'm assuming these cases aren't handled currently and that handling them isn't necessary to land the patch.)

I'm afraid handling such cases will eventually invoke the same problems the RangeConstraintSolver has, and then the discussion about whether this is good in Tidy instead of CSA will be resurrected... 😦

One could come up with even more elaborate cases. Just hit something like below a few minutes ago while working:

const CallExpr* const C;

if (auto* CalledFun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
  // ...
  if (const auto* Fun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
    // ...
  }
  // ...
}

and the rabbit hole goes deeper. But it's pretty interesting how many cases the current check found as-is.

While I agree with your observation about data and flow sensitivity, I approved on the belief that the check as-is provides enough utility to warrant adding it as-is. If someone wants to improve the check into being a CSA check, we can always deprecate this one at that point. However, if there are strong opinions that the check should start out as a CSA check because it requires that sensitivity for your needs, now's a good time to bring up those concerns.

[baloghadamsoftware]

Tests andd for not yet handled cases. Known limiatations included in the docs.

[whisperity]

In D81272#2218175, @aaron.ballman wrote:

While I agree with your observation about data and flow sensitivity, I approved on the belief that the check as-is provides enough utility to warrant adding it as-is. If someone wants to improve the check into being a CSA check, we can always deprecate this one at that point. However, if there are strong opinions that the check should start out as a CSA check because it requires that sensitivity for your needs, now's a good time to bring up those concerns.

It's generally harder to create big logic mistakes when it comes to more complex expressions, assuming the user does't copy-paste (which I might have done, in the above example). We do not need to solve every potentially equivalent conditional (it is unsolvable in the generic case anyways). I'm sure this check can be improved later with handling trivial comparisons (such as standard library int results not being 0, -1, etc.).

Edit: what I'm trying to say is catching cases like foo > 5 and then check for foo > 3 being trivial, or catching foo > 5 and then foo == 5 being impossible is a useful addition later on to this check. But having the ability to catch things like bar >= 2u and then foo > 5u and then if (foo - bar < 4u) is rather extravagant, and highly unneeded. I'm not even sure if directly catching this latter extravagant case (or the thing with cast pointers in my previous example!) by itself is needed... SA could deal with such things inside the engine if and only if their use later down the line results in something terrible (such as giving foo - bar as a file descriptor, etc.).

[aaron.ballman]

In D81272#2218246, @whisperity wrote:

In D81272#2218175, @aaron.ballman wrote:

While I agree with your observation about data and flow sensitivity, I approved on the belief that the check as-is provides enough utility to warrant adding it as-is. If someone wants to improve the check into being a CSA check, we can always deprecate this one at that point. However, if there are strong opinions that the check should start out as a CSA check because it requires that sensitivity for your needs, now's a good time to bring up those concerns.

It's generally harder to create big logic mistakes when it comes to more complex expressions, assuming the user does't copy-paste (which I might have done, in the above example). We do not need to solve every potentially equivalent conditional (it is unsolvable in the generic case anyways). I'm sure this check can be improved later with handling trivial comparisons (such as standard library int results not being 0, -1, etc.).

Great, then we're on the same page!

The new documentation and test cases LG, so re-approving. Thank you!

[sylvestre.ledru]

Thanks for this checker. FYI, it found (at least) 3 defects in Firefox code (fixed with the autofix):
https://hg.mozilla.org/mozilla-central/rev/651e68f628d0

https://bugzilla.mozilla.org/show_bug.cgi?id=1664747
(unlikely that it was causing any actual bugs in the product)

[sylvestre.ledru]

This comment was removed by sylvestre.ledru.

[baloghadamsoftware]

In D81272#2276342, @sylvestre.ledru wrote:

Thanks for this checker. FYI, it found (at least) 3 defects in Firefox code (fixed with the autofix):
https://hg.mozilla.org/mozilla-central/rev/651e68f628d0

https://bugzilla.mozilla.org/show_bug.cgi?id=1664747
(unlikely that it was causing any actual bugs in the product)

Thank you for your info. These defects do not cause any bug directly, but they make code understanding harder which may cause bugs in the future.

clang-tools-extra/clang-tidy/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyMiscModule
   DefinitionsInHeadersCheck.cpp
   MiscTidyModule.cpp
   MisplacedConstCheck.cpp
   NewDeleteOverloadsCheck.cpp
   NoRecursionCheck.cpp
   NonCopyableObjects.cpp
   NonPrivateMemberVariablesInClassesCheck.cpp
+  RedundantConditionCheck.cpp
   RedundantExpressionCheck.cpp
   StaticAssertCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   UniqueptrResetReleaseCheck.cpp
   UnusedAliasDeclsCheck.cpp
   UnusedParametersCheck.cpp
   UnusedUsingDeclsCheck.cpp

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NoRecursionCheck.h"
 #include "NonCopyableObjects.h"
 #include "NonPrivateMemberVariablesInClassesCheck.h"
+#include "RedundantConditionCheck.h"
 #include "RedundantExpressionCheck.h"
 #include "StaticAssertCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedUsingDeclsCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
     CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
         "misc-new-delete-overloads");
     CheckFactories.registerCheck<NoRecursionCheck>("misc-no-recursion");
     CheckFactories.registerCheck<NonCopyableObjectsCheck>(
         "misc-non-copyable-objects");
     CheckFactories.registerCheck<NonPrivateMemberVariablesInClassesCheck>(
         "misc-non-private-member-variables-in-classes");
+    CheckFactories.registerCheck<RedundantConditionCheck>(
+        "misc-redundant-condition");
     CheckFactories.registerCheck<RedundantExpressionCheck>(
         "misc-redundant-expression");
     CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
     CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
         "misc-throw-by-value-catch-by-reference");
     CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
         "misc-unconventional-assign-operator");
     CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(

clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.h

+//===--- RedundantConditionCheck.h - clang-tidy -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTCONDITIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTCONDITIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds condition variables in nested `if` statements that were also checked
+/// in the outer `if` statement and were not changed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-condition.html
+class RedundantConditionCheck : public ClangTidyCheck {
+public:
+  RedundantConditionCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTCONDITIONCHECK_H

clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp

+//===--- RedundantConditionCheck.cpp - clang-tidy -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RedundantConditionCheck.h"
+#include "../utils/Aliasing.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+
+using namespace clang::ast_matchers;
+using clang::tidy::utils::hasPtrOrReferenceInFunc;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+static const char CondVarStr[] = "cond_var";
+static const char OuterIfStr[] = "outer_if";
+static const char InnerIfStr[] = "inner_if";
+static const char FuncStr[] = "func";
+
+/// Returns whether `Var` is changed in `S` before `NextS`.
+static bool isChangedBefore(const Stmt *S, const Stmt *NextS,
+                            const VarDecl *Var, ASTContext *Context) {
+  ExprMutationAnalyzer MutAn(*S, *Context);
+  const auto &SM = Context->getSourceManager();
+  if (const Stmt *MutS = MutAn.findMutation(Var)) {
+    if (SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc()))
+      return true;
+  }
+
+  return false;
+}
+
+void RedundantConditionCheck::registerMatchers(MatchFinder *Finder) {
+  const auto ImmutableVar =
+      varDecl(anyOf(parmVarDecl(), hasLocalStorage()), hasType(isInteger()),
+              unless(hasType(isVolatileQualified())))
+          .bind(CondVarStr);
+  Finder->addMatcher(
+      ifStmt(hasCondition(
+                 anyOf(declRefExpr(hasDeclaration(ImmutableVar)),
+                       binaryOperator(hasOperatorName("&&"),
+                                      hasEitherOperand(declRefExpr(
+                                          hasDeclaration(ImmutableVar)))))),
+             hasDescendant(
+                 ifStmt(hasCondition(anyOf(
+                            declRefExpr(hasDeclaration(
+                                varDecl(equalsBoundNode(CondVarStr)))),
+                            binaryOperator(
+                                hasAnyOperatorName("&&", "||"),
+                                hasEitherOperand(declRefExpr(hasDeclaration(
+                                    varDecl(equalsBoundNode(CondVarStr)))))))))
+                     .bind(InnerIfStr)),
+             forFunction(functionDecl().bind(FuncStr)))
+          .bind(OuterIfStr),
+      this);
+  // FIXME: Handle longer conjunctive and disjunctive clauses.
+}
+
+void RedundantConditionCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *OuterIf = Result.Nodes.getNodeAs<IfStmt>(OuterIfStr);
+  const auto *InnerIf = Result.Nodes.getNodeAs<IfStmt>(InnerIfStr);
+  const auto *CondVar = Result.Nodes.getNodeAs<VarDecl>(CondVarStr);
+  const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>(FuncStr);
+
+  // If the variable has an alias then it can be changed by that alias as well.
+  // FIXME: Track pointers and references.
+  if (hasPtrOrReferenceInFunc(Func, CondVar))
+    return;
+
+  if (isChangedBefore(OuterIf, InnerIf, CondVar, Result.Context))
+    return;
+
+  auto Diag = diag(InnerIf->getBeginLoc(), "redundant condition %0") << CondVar;
+
+  // For standalone condition variables and for "or" binary operations we simply
+  // remove the inner `if`.
+  const auto *BinOpCond = dyn_cast<BinaryOperator>(InnerIf->getCond());
+  if (isa<DeclRefExpr>(InnerIf->getCond()->IgnoreParenImpCasts()) ||
+      (BinOpCond && BinOpCond->getOpcode() == BO_LOr)) {
+    SourceLocation IfBegin = InnerIf->getBeginLoc();
+    const Stmt *Body = InnerIf->getThen();
+
+    // For comound statements also remove the right brace at the end.
+    if (isa<CompoundStmt>(Body)) {
+      SourceLocation IfEnd = Body->getBeginLoc().getLocWithOffset(1);
+      Diag << FixItHint::CreateRemoval(
+                  CharSourceRange::getTokenRange(IfBegin, IfEnd))
+           << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+                  Body->getEndLoc(), Body->getEndLoc()));
+    } else {
+      SourceLocation IfEnd = Body->getBeginLoc().getLocWithOffset(-1);
+      Diag << FixItHint::CreateRemoval(
+          CharSourceRange::getTokenRange(IfBegin, IfEnd));
+    }
+
+    // For "and" binary operations we remove the "and" operation with the
+    // condition variable from the inner if.
+  } else {
+    const auto *CondOp = cast<BinaryOperator>(InnerIf->getCond());
+    const auto *LeftDRE =
+        dyn_cast<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts());
+    if (LeftDRE && LeftDRE->getDecl() == CondVar) {
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+          CondOp->getLHS()->getBeginLoc(),
+          CondOp->getRHS()->getBeginLoc().getLocWithOffset(-1)));
+    } else {
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+          CondOp->getLHS()->getEndLoc().getLocWithOffset(1),
+          CondOp->getRHS()->getBeginLoc()));
+    }
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 New module
 ^^^^^^^^^^
 - New module `llvmlibc`.
 
   This module contains checks related to the LLVM-libc coding standards.
 
 New checks
 ^^^^^^^^^^
 - New :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables>` check.
   Finds non-const global variables as described in check I.2 of C++ Core
   Guidelines.
 
 - New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
   <clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc>` check.
 
   Finds cases where an integer expression is added to or subtracted from the
   result of a memory allocation function (``malloc()``, ``calloc()``,
   ``realloc()``, ``alloca()``) instead of its argument.
 
- New :doc:`objc-dealloc-in-category
 
   Finds implementations of -dealloc in Objective-C categories.
 
 - New :doc:`misc-no-recursion
   <clang-tidy/checks/misc-no-recursion>` check.
 
   Finds recursive functions and diagnoses them.
 
+- New :doc:`misc-redundant-condition
+  <clang-tidy/checks/misc-redundant-condition>` check.
+
+  Finds condition variables in nested ``if`` statements that were also checked
+  in the outer ``if`` statement and were not changed.
+
 - New :doc:`objc-nsinvocation-argument-lifetime
   <clang-tidy/checks/objc-nsinvocation-argument-lifetime>` check.
 
   Finds calls to ``NSInvocation`` methods under ARC that don't have proper
   argument object lifetimes.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `clang-analyzer-osx.cocoa.Loops <clang-analyzer-osx.cocoa.Loops.html>`_,
    `clang-analyzer-osx.cocoa.MissingSuperCall <clang-analyzer-osx.cocoa.MissingSuperCall.html>`_,
    `clang-analyzer-osx.cocoa.NonNilReturnValue <clang-analyzer-osx.cocoa.NonNilReturnValue.html>`_,
    `clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak <clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak.html>`_,
    `clang-analyzer-valist.CopyToSelf <clang-analyzer-valist.CopyToSelf.html>`_,
    `clang-analyzer-valist.Uninitialized <clang-analyzer-valist.Uninitialized.html>`_,
    `clang-analyzer-valist.Unterminated <clang-analyzer-valist.Unterminated.html>`_,
    `cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
    `cppcoreguidelines-init-variables <cppcoreguidelines-init-variables.html>`_, "Yes"
    `cppcoreguidelines-interfaces-global-init <cppcoreguidelines-interfaces-global-init.html>`_,
    `cppcoreguidelines-macro-usage <cppcoreguidelines-macro-usage.html>`_,
    `cppcoreguidelines-narrowing-conversions <cppcoreguidelines-narrowing-conversions.html>`_,
    `cppcoreguidelines-no-malloc <cppcoreguidelines-no-malloc.html>`_,
    `cppcoreguidelines-owning-memory <cppcoreguidelines-owning-memory.html>`_,
    `cppcoreguidelines-pro-bounds-array-to-pointer-decay <cppcoreguidelines-pro-bounds-array-to-pointer-decay.html>`_,
    `cppcoreguidelines-pro-bounds-constant-array-index <cppcoreguidelines-pro-bounds-constant-array-index.html>`_, "Yes"
    `cppcoreguidelines-pro-bounds-pointer-arithmetic <cppcoreguidelines-pro-bounds-pointer-arithmetic.html>`_,
.. csv-table::
    `llvmlibc-implementation-in-namespace <llvmlibc-implementation-in-namespace.html>`_,
    `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
    `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
    `misc-misplaced-const <misc-misplaced-const.html>`_,
    `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `misc-no-recursion <misc-no-recursion.html>`_,
    `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
+   `misc-redundant-condition <misc-redundant-condition.html>`_, "Yes"
    `misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
    `misc-static-assert <misc-static-assert.html>`_, "Yes"
    `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
    `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
    `misc-uniqueptr-reset-release <misc-uniqueptr-reset-release.html>`_, "Yes"
    `misc-unused-alias-decls <misc-unused-alias-decls.html>`_, "Yes"
    `misc-unused-parameters <misc-unused-parameters.html>`_, "Yes"
    `misc-unused-using-decls <misc-unused-using-decls.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst

+.. title:: clang-tidy - misc-redundant-condition
+
+misc-redundant-condition
+========================
+
+Finds condition variables in nested `if` statements that were also checked in
+the outer `if` statement and were not changed.
+
+Simple example:
+
+.. code-block:: c
+
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire)
+      scream();
+  }
+
+Here `onFire` is checked both in the outer ``if`` and the inner ``if`` statement
+without a possible change between the two checks. The check warns for this code
+and suggests removal of the second checking of variable `onFire`.
+
+The checker also detects redundant condition checks if the condition variable
+is an operand of a logical "and" (``&&``) or a logical "or" (``||``) operator:
+
+.. code-block:: c
+
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire && peopleInTheBuilding > 0)
+      scream();
+  }
+
+.. code-block:: c
+
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire || isCollapsing())
+      scream();
+  }
+
+In the first case (logical "and") the suggested fix is to remove the redundant
+condition variable and keep the other side of the ``&&``. In the second case
+(logical "or") the whole ``if`` is removed similarily to the simple case on the
+top.
+
+The condition of the outer ``if`` statement may also be a logical "and" (``&&``)
+expression:
+
+.. code-block:: c
+
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      if (onFire)
+        scream();
+    }
+  }
+
+The error is also detected if both the outer statement is a logical "and"
+(``&&``) and the inner statement is a logical "and" (``&&``) or "or" (``||``).
+The inner ``if`` statement does not have to be a direct descendant of the outer
+one.
+
+No error is detected if the condition variable may have been changed between the
+two checks:
+
+.. code-block:: c
+
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (onFire && peopleInTheBuilding > 0)
+      scream();
+  }
+
+Every possible change is considered, thus if the condition variable is not
+a local variable of the function, it is a volatile or it has an alias (pointer
+or reference) then no warning is issued.
+

clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp

+// RUN: %check_clang_tidy %s misc-redundant-condition %t
+
+extern unsigned peopleInTheBuilding;
+extern unsigned fireFighters;
+
+bool isBurning();
+bool isReallyBurning();
+bool isCollapsing();
+void tryToExtinguish(bool&);
+void scream();
+
+bool someOtherCondition();
+
+//===--- Basic Positives --------------------------------------------------===//
+
+void positive_direct() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      if (onFire)
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+    }
+  }
+}
+
+void positive_direct_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire && peopleInTheBuilding > 0) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+      scream();
+    }
+  }
+}
+
+void positive_indirect_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      if (onFire && peopleInTheBuilding > 0) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+        scream();
+      }
+    }
+  }
+}
+
+void positive_direct_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (peopleInTheBuilding > 0 && onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+      scream();
+    }
+  }
+}
+
+void positive_indirect_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      if (peopleInTheBuilding > 0 && onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+        scream();
+      }
+    }
+  }
+}
+
+void positive_direct_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire || isCollapsing()) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      if (onFire || isCollapsing()) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+void positive_direct_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (isCollapsing() || onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      if (isCollapsing() || onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+void positive_direct_outer_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+    // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_outer_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      if (onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+void positive_direct_outer_and_lhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (onFire && peopleInTheBuilding > 0) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+      scream();
+    }
+  }
+}
+
+void positive_indirect_outer_and_lhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      if (onFire && peopleInTheBuilding > 0) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+        scream();
+      }
+    }
+  }
+}
+
+void positive_direct_outer_and_lhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (peopleInTheBuilding > 0 && onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+      scream();
+    }
+  }
+}
+
+void positive_indirect_outer_and_lhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      if (peopleInTheBuilding > 0 && onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+        scream();
+      }
+    }
+  }
+}
+
+void positive_direct_outer_and_lhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (onFire || isCollapsing()) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_outer_and_lhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      if (onFire || isCollapsing()) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+    // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+void positive_direct_outer_and_lhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (isCollapsing() || onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_outer_and_lhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      if (isCollapsing() || onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+void positive_direct_outer_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_outer_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      if (onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+void positive_direct_outer_and_rhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (onFire && peopleInTheBuilding > 0) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+      scream();
+    }
+  }
+}
+
+void positive_indirect_outer_and_rhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      if (onFire && peopleInTheBuilding > 0) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+        scream();
+      }
+    }
+  }
+}
+
+void positive_direct_inner_outer_and_rhs_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (peopleInTheBuilding > 0 && onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+      scream();
+    }
+  }
+}
+
+void positive_indirect_outer_and_rhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      if (peopleInTheBuilding > 0 && onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+        scream();
+      }
+    }
+  }
+}
+
+void positive_direct_outer_and_rhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (onFire || isCollapsing()) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_outer_and_rhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      if (onFire || isCollapsing()) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+void positive_direct_outer_and_rhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (isCollapsing() || onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_outer_and_rhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      if (isCollapsing() || onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+  }
+}
+
+//===--- Basic Negatives --------------------------------------------------===//
+
+void negative_direct() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (onFire && peopleInTheBuilding > 0) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire && peopleInTheBuilding > 0) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire && peopleInTheBuilding > 0) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (peopleInTheBuilding > 0 && onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (peopleInTheBuilding > 0 && onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (peopleInTheBuilding > 0 && onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (onFire || isCollapsing()) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire || isCollapsing()) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire || isCollapsing()) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (isCollapsing() || onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (isCollapsing() || onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (isCollapsing() || onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_lhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (onFire && peopleInTheBuilding > 0) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_lhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire && peopleInTheBuilding > 0) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_lhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire && peopleInTheBuilding > 0) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_lhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (peopleInTheBuilding > 0 && onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_lhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (peopleInTheBuilding > 0 && onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_lhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (peopleInTheBuilding > 0 && onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_lhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (onFire || isCollapsing()) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_lhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire || isCollapsing()) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_lhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire || isCollapsing()) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_lhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (isCollapsing() || onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_lhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (isCollapsing() || onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_lhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (isCollapsing() || onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_rhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (onFire && peopleInTheBuilding > 0) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_rhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire && peopleInTheBuilding > 0) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_rhs_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire && peopleInTheBuilding > 0) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_inner_outer_and_rhs_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (peopleInTheBuilding > 0 && onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_rhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (peopleInTheBuilding > 0 && onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_rhs_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (peopleInTheBuilding > 0 && onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_rhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (onFire || isCollapsing()) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_rhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (onFire || isCollapsing()) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_rhs_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (onFire || isCollapsing()) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_direct_outer_and_rhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (isCollapsing() || onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_indirect_outer_and_rhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    tryToExtinguish(onFire);
+    if (someOtherCondition()) {
+      if (isCollapsing() || onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_indirect2_outer_and_rhs_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (fireFighters < 10 && onFire) {
+    if (someOtherCondition()) {
+      tryToExtinguish(onFire);
+      if (isCollapsing() || onFire) {
+        // NO-MESSAGE: fire may have been extinguished
+        scream();
+      }
+    }
+  }
+}
+
+void negative_reassigned() {
+  bool onFire = isBurning();
+  if (onFire) {
+    onFire = isReallyBurning();
+    if (onFire) {
+      // NO-MESSAGE: it was a false alarm then
+      scream();
+    }
+  }
+}
+
+//===--- Special Positives ------------------------------------------------===//
+
+// Condition variable mutated in or after the inner loop
+
+void positive_direct_mutated_after_inner() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+    tryToExtinguish(onFire);
+  }
+}
+
+void positive_indirect_mutated_after_inner() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      if (onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+    }
+    tryToExtinguish(onFire);
+  }
+}
+
+void positive_indirect2_mutated_after_inner() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (someOtherCondition()) {
+      if (onFire) {
+        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+        // CHECK-FIXES: {{^\ *$}}
+        scream();
+      }
+      // CHECK-FIXES: {{^\ *$}}
+      tryToExtinguish(onFire);
+    }
+  }
+}
+
+void positive_mutated_in_inner() {
+  bool onFire = isBurning();
+  if (onFire) {
+    if (onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      tryToExtinguish(onFire);
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+//===--- Special Negatives ------------------------------------------------===//
+
+// Aliasing
+
+void negative_mutated_by_ptr() {
+  bool onFire = isBurning();
+  bool *firePtr = &onFire;
+  if (onFire) {
+    tryToExtinguish(*firePtr);
+    if (onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+void negative_mutated_by_ref() {
+  bool onFire = isBurning();
+  bool &fireRef = onFire;
+  if (onFire) {
+    tryToExtinguish(fireRef);
+    if (onFire) {
+      // NO-MESSAGE: fire may have been extinguished
+      scream();
+    }
+  }
+}
+
+// Volatile
+
+void negatvie_volatile() {
+  bool volatile onFire = isBurning();
+  if (onFire) {
+    if (onFire) {
+      // NO-MESSAGE: maybe some other thread extinguished the fire
+      scream();
+    }
+  }
+}