Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -38,6 +38,7 @@ #include "NotNullTerminatedResultCheck.h" #include "ParentVirtualCallCheck.h" #include "PosixReturnCheck.h" +#include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" #include "SignedCharMisuseCheck.h" #include "SizeofContainerCheck.h" @@ -119,6 +120,8 @@ "bugprone-move-forwarding-reference"); CheckFactories.registerCheck( "bugprone-multiple-statement-macro"); + CheckFactories.registerCheck( + "bugprone-redundant-branch-condition"); CheckFactories.registerCheck( "bugprone-narrowing-conversions"); CheckFactories.registerCheck("bugprone-no-escape"); Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -33,6 +33,7 @@ NotNullTerminatedResultCheck.cpp ParentVirtualCallCheck.cpp PosixReturnCheck.cpp + RedundantBranchConditionCheck.cpp ReservedIdentifierCheck.cpp SignedCharMisuseCheck.cpp SizeofContainerCheck.cpp Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h @@ -0,0 +1,35 @@ +//===--- RedundantBranchConditionCheck.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_BUGPRONE_REDUNDANTBRANCHCONDITIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_REDUNDANTBRANCHCONDITIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// 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/bugprone-redundant-branch-condition.html +class RedundantBranchConditionCheck : public ClangTidyCheck { +public: + RedundantBranchConditionCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_REDUNDANTBRANCHCONDITIONCHECK_H Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp @@ -0,0 +1,153 @@ +//===--- RedundantBranchConditionCheck.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 "RedundantBranchConditionCheck.h" +#include "../utils/Aliasing.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; +using clang::tidy::utils::hasPtrOrReferenceInFunc; + +namespace clang { +namespace tidy { +namespace bugprone { + +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(); + const Stmt *MutS = MutAn.findMutation(Var); + return MutS && + SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc()); +} + +void RedundantBranchConditionCheck::registerMatchers(MatchFinder *Finder) { + const auto ImmutableVar = + varDecl(anyOf(parmVarDecl(), hasLocalStorage()), hasType(isInteger()), + unless(hasType(isVolatileQualified()))) + .bind(CondVarStr); + Finder->addMatcher( + ifStmt( + hasCondition(ignoringParenImpCasts(anyOf( + declRefExpr(hasDeclaration(ImmutableVar)), + binaryOperator(hasOperatorName("&&"), + hasEitherOperand(ignoringParenImpCasts(declRefExpr( + hasDeclaration(ImmutableVar)))))))), + hasThen(hasDescendant( + ifStmt(hasCondition(ignoringParenImpCasts( + anyOf(declRefExpr(hasDeclaration( + varDecl(equalsBoundNode(CondVarStr)))), + binaryOperator( + hasAnyOperatorName("&&", "||"), + hasEitherOperand(ignoringParenImpCasts( + declRefExpr(hasDeclaration(varDecl( + equalsBoundNode(CondVarStr))))))))))) + .bind(InnerIfStr))), + forFunction(functionDecl().bind(FuncStr))) + .bind(OuterIfStr), + this); + // FIXME: Handle longer conjunctive and disjunctive clauses. +} + +void RedundantBranchConditionCheck::check(const MatchFinder::MatchResult &Result) { + const auto *OuterIf = Result.Nodes.getNodeAs(OuterIfStr); + const auto *InnerIf = Result.Nodes.getNodeAs(InnerIfStr); + const auto *CondVar = Result.Nodes.getNodeAs(CondVarStr); + const auto *Func = Result.Nodes.getNodeAs(FuncStr); + + // If the variable has an alias then it can be changed by that alias as well. + // FIXME: could potentially support tracking pointers and references in the + // future to improve catching true positives through aliases. + if (hasPtrOrReferenceInFunc(Func, CondVar)) + return; + + if (isChangedBefore(OuterIf->getThen(), 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(InnerIf->getCond()); + if (isa(InnerIf->getCond()->IgnoreParenImpCasts()) || + (BinOpCond && BinOpCond->getOpcode() == BO_LOr)) { + SourceLocation IfBegin = InnerIf->getBeginLoc(); + const Stmt *Body = InnerIf->getThen(); + const Expr *OtherSide = nullptr; + if (BinOpCond) { + const auto *LeftDRE = + dyn_cast(BinOpCond->getLHS()->IgnoreParenImpCasts()); + if (LeftDRE && LeftDRE->getDecl() == CondVar) + OtherSide = BinOpCond->getRHS(); + else + OtherSide = BinOpCond->getLHS(); + } + + SourceLocation IfEnd = Body->getBeginLoc().getLocWithOffset(-1); + + // For compound statements also remove the left brace. + if (isa(Body)) + IfEnd = Body->getBeginLoc(); + + // If the other side has side effects then keep it. + if (OtherSide && OtherSide->HasSideEffects(*Result.Context)) { + SourceLocation BeforeOtherSide = + OtherSide->getBeginLoc().getLocWithOffset(-1); + SourceLocation AfterOtherSide = + Lexer::findNextToken(OtherSide->getEndLoc(), *Result.SourceManager, + getLangOpts()) + ->getLocation(); + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(IfBegin, BeforeOtherSide)) + << FixItHint::CreateInsertion(AfterOtherSide, ";") + << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(AfterOtherSide, IfEnd)); + } else { + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(IfBegin, IfEnd)); + } + + // For comound statements also remove the right brace at the end. + if (isa(Body)) + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(Body->getEndLoc(), Body->getEndLoc())); + + // For "and" binary operations we remove the "and" operation with the + // condition variable from the inner if. + } else { + const auto *CondOp = cast(InnerIf->getCond()); + const auto *LeftDRE = + dyn_cast(CondOp->getLHS()->IgnoreParenImpCasts()); + if (LeftDRE && LeftDRE->getDecl() == CondVar) { + SourceLocation BeforeRHS = + CondOp->getRHS()->getBeginLoc().getLocWithOffset(-1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + CondOp->getLHS()->getBeginLoc(), BeforeRHS)); + } else { + SourceLocation AfterLHS = + Lexer::findNextToken(CondOp->getLHS()->getEndLoc(), + *Result.SourceManager, getLangOpts()) + ->getLocation(); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + AfterLHS, CondOp->getRHS()->getEndLoc())); + } + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,11 @@ Improvements to clang-tidy -------------------------- -The improvements are... +- New :doc:`bugprone-redundant-branch-condition + ` check. + + Finds condition variables in nested ``if`` statements that were also checked + in the outer ``if`` statement and were not changed. Improvements to include-fixer ----------------------------- Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-redundant-branch-condition.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/bugprone-redundant-branch-condition.rst @@ -0,0 +1,79 @@ +.. title:: clang-tidy - bugprone-redundant-branch-condition + +bugprone-redundant-branch-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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -70,10 +70,11 @@ `bugprone-misplaced-widening-cast `_, `bugprone-move-forwarding-reference `_, "Yes" `bugprone-multiple-statement-macro `_, - `bugprone-no-escape `_, "Yes" + `bugprone-no-escape `_, `bugprone-not-null-terminated-result `_, "Yes" `bugprone-parent-virtual-call `_, "Yes" `bugprone-posix-return `_, "Yes" + `bugprone-redundant-branch-condition `_, "Yes" `bugprone-reserved-identifier `_, "Yes" `bugprone-signed-char-misuse `_, `bugprone-sizeof-container `_, Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp @@ -0,0 +1,1153 @@ +// RUN: %check_clang_tidy %s bugprone-redundant-branch-condition %t + +extern unsigned peopleInTheBuilding; +extern unsigned fireFighters; + +bool isBurning(); +bool isReallyBurning(); +bool isCollapsing(); +void tryToExtinguish(bool&); +void tryPutFireOut(); +bool callTheFD(); +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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-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' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {{^\ *$}} + tryToExtinguish(onFire); + scream(); + } + // CHECK-FIXES: {{^\ *$}} + } +} + +void positive_or_lhs_with_side_effect() { + bool onFire = isBurning(); + if (onFire) { + if (callTheFD() || onFire) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition] + // CHECK-FIXES: callTheFD() ; + scream(); + } + // CHECK-FIXES: {{^\ *$}} + } +} + +void positive_or_rhs_with_side_effect() { + bool onFire = isBurning(); + if (onFire) { + if (onFire || callTheFD()) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition] + // CHECK-FIXES: callTheFD(); + scream(); + } + // CHECK-FIXES: {{^\ *$}} + } +} + +// GNU Expression Statements + +void do_something(); + +void positive_gnu_expression_statement() { + bool onFire = isBurning(); + if (({ do_something(); onFire; })) { + if (({ do_something(); onFire; })) { + // FIXME: Handle GNU epxression statements + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition] + // CHCK-FIXES-NOT: do_something(); + scream(); + } + } +} + +// Comma after Condition + +void positive_comma_after_condition() { + bool onFire = isBurning(); + if (do_something(), onFire) { + if (do_something(), onFire) { + // FIXME: Handle comma operator + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition] + // CHCK-FIXES-NOT: do_something(); + scream(); + } + } +} + +//===--- 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(); + } + } +} + +void negative_else_branch(bool isHot) { + bool onFire = isBurning(); + if (onFire) { + tryPutFireOut(); + } else { + if (isHot && onFire) { + // NO-MESSAGE: new check is in the `else` branch + // FIXME: handle `else` branches and negated conditions + scream(); + } + } +} + +// GNU Expression Statements + +void negative_gnu_expression_statement() { + bool onFire = isBurning(); + if (({ do_something(); onFire; })) { + tryToExtinguish(onFire); + if (({ do_something(); onFire; })) { + // NO-MESSAGE: fire may have been extinguished + scream(); + } + } +} + +// Comma after Condition + +void negative_comma_after_condition() { + bool onFire = isBurning(); + if (do_something(), onFire) { + tryToExtinguish(onFire); + if (do_something(), onFire) { + // NO-MESSAGE: fire may have been extinguished + scream(); + } + } +}