clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp

   1 //===--- RedundantBranchConditionCheck.cpp - clang-tidy -------------------------===//
   2 //
   3 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
   4 // See https://llvm.org/LICENSE.txt for license information.
   5 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
   6 //
   7 //===----------------------------------------------------------------------===//
   8
   9 #include "RedundantBranchConditionCheck.h"
  10 #include "../utils/Aliasing.h"
  11 #include "clang/AST/ASTContext.h"
  12 #include "clang/ASTMatchers/ASTMatchFinder.h"
  13 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
  14 #include "clang/Lex/Lexer.h"
  15
  16 using namespace clang::ast_matchers;
  17 using clang::tidy::utils::hasPtrOrReferenceInFunc;
  18
  19 namespace clang::tidy::bugprone {
  20
  21 static const char CondVarStr[] = "cond_var";
  22 static const char OuterIfStr[] = "outer_if";
  23 static const char InnerIfStr[] = "inner_if";
  24 static const char OuterIfVar1Str[] = "outer_if_var1";
  25 static const char OuterIfVar2Str[] = "outer_if_var2";
  26 static const char InnerIfVar1Str[] = "inner_if_var1";
  27 static const char InnerIfVar2Str[] = "inner_if_var2";
  28 static const char FuncStr[] = "func";
  29
  30 /// Returns whether `Var` is changed in range (`PrevS`..`NextS`).
  31 static bool isChangedBefore(const Stmt *S, const Stmt *NextS, const Stmt *PrevS,
  32                             const VarDecl *Var, ASTContext *Context) {
  33   ExprMutationAnalyzer MutAn(*S, *Context);
  34   const auto &SM = Context->getSourceManager();
  35   const Stmt *MutS = MutAn.findMutation(Var);
  36   return MutS &&
  37          SM.isBeforeInTranslationUnit(PrevS->getEndLoc(),
  38                                       MutS->getBeginLoc()) &&
  39          SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc());
  40 }
  41
  42 void RedundantBranchConditionCheck::registerMatchers(MatchFinder *Finder) {
  43   const auto ImmutableVar =
  44       varDecl(anyOf(parmVarDecl(), hasLocalStorage()), hasType(isInteger()),
  45               unless(hasType(isVolatileQualified())))
  46           .bind(CondVarStr);
  47   Finder->addMatcher(
  48       ifStmt(
  49           hasCondition(anyOf(
  50               declRefExpr(hasDeclaration(ImmutableVar)).bind(OuterIfVar1Str),
  51               binaryOperator(
  52                   hasOperatorName("&&"),
  53                   hasEitherOperand(declRefExpr(hasDeclaration(ImmutableVar))
  54                                        .bind(OuterIfVar2Str))))),
  55           hasThen(hasDescendant(
  56               ifStmt(hasCondition(anyOf(
  57                          declRefExpr(hasDeclaration(
  58                                          varDecl(equalsBoundNode(CondVarStr))))
  59                              .bind(InnerIfVar1Str),
  60                          binaryOperator(
  61                              hasAnyOperatorName("&&", "||"),
  62                              hasEitherOperand(
  63                                  declRefExpr(hasDeclaration(varDecl(
  64                                                  equalsBoundNode(CondVarStr))))
  65                                      .bind(InnerIfVar2Str))))))
  66                   .bind(InnerIfStr))),
  67           forFunction(functionDecl().bind(FuncStr)))
  68           .bind(OuterIfStr),
  69       this);
  70   // FIXME: Handle longer conjunctive and disjunctive clauses.
  71 }
  72
  73 void RedundantBranchConditionCheck::check(const MatchFinder::MatchResult &Result) {
  74   const auto *OuterIf = Result.Nodes.getNodeAs<IfStmt>(OuterIfStr);
  75   const auto *InnerIf = Result.Nodes.getNodeAs<IfStmt>(InnerIfStr);
  76   const auto *CondVar = Result.Nodes.getNodeAs<VarDecl>(CondVarStr);
  77   const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>(FuncStr);
  78
  79   const DeclRefExpr *OuterIfVar = nullptr, *InnerIfVar = nullptr;
  80   if (const auto *Inner = Result.Nodes.getNodeAs<DeclRefExpr>(InnerIfVar1Str))
  81     InnerIfVar = Inner;
  82   else
  83     InnerIfVar = Result.Nodes.getNodeAs<DeclRefExpr>(InnerIfVar2Str);
  84   if (const auto *Outer = Result.Nodes.getNodeAs<DeclRefExpr>(OuterIfVar1Str))
  85     OuterIfVar = Outer;
  86   else
  87     OuterIfVar = Result.Nodes.getNodeAs<DeclRefExpr>(OuterIfVar2Str);
  88
  89   if (OuterIfVar && InnerIfVar) {
  90     if (isChangedBefore(OuterIf->getThen(), InnerIfVar, OuterIfVar, CondVar,
  91                         Result.Context))
  92       return;
  93
  94     if (isChangedBefore(OuterIf->getCond(), InnerIfVar, OuterIfVar, CondVar,
  95                         Result.Context))
  96       return;
  97   }
  98
  99   // If the variable has an alias then it can be changed by that alias as well.
 100   // FIXME: could potentially support tracking pointers and references in the
 101   // future to improve catching true positives through aliases.
 102   if (hasPtrOrReferenceInFunc(Func, CondVar))
 103     return;
 104
 105   auto Diag = diag(InnerIf->getBeginLoc(), "redundant condition %0") << CondVar;
 106
 107   // For standalone condition variables and for "or" binary operations we simply
 108   // remove the inner `if`.
 109   const auto *BinOpCond =
 110       dyn_cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts());
 111
 112   if (isa<DeclRefExpr>(InnerIf->getCond()->IgnoreParenImpCasts()) ||
 113       (BinOpCond && BinOpCond->getOpcode() == BO_LOr)) {
 114     SourceLocation IfBegin = InnerIf->getBeginLoc();
 115     const Stmt *Body = InnerIf->getThen();
 116     const Expr *OtherSide = nullptr;
 117     if (BinOpCond) {
 118       const auto *LeftDRE =
 119           dyn_cast<DeclRefExpr>(BinOpCond->getLHS()->IgnoreParenImpCasts());
 120       if (LeftDRE && LeftDRE->getDecl() == CondVar)
 121         OtherSide = BinOpCond->getRHS();
 122       else
 123         OtherSide = BinOpCond->getLHS();
 124     }
 125
 126     SourceLocation IfEnd = Body->getBeginLoc().getLocWithOffset(-1);
 127
 128     // For compound statements also remove the left brace.
 129     if (isa<CompoundStmt>(Body))
 130       IfEnd = Body->getBeginLoc();
 131
 132     // If the other side has side effects then keep it.
 133     if (OtherSide && OtherSide->HasSideEffects(*Result.Context)) {
 134       SourceLocation BeforeOtherSide =
 135           OtherSide->getBeginLoc().getLocWithOffset(-1);
 136       SourceLocation AfterOtherSide =
 137           Lexer::findNextToken(OtherSide->getEndLoc(), *Result.SourceManager,
 138                                getLangOpts())
 139               ->getLocation();
 140       Diag << FixItHint::CreateRemoval(
 141                   CharSourceRange::getTokenRange(IfBegin, BeforeOtherSide))
 142            << FixItHint::CreateInsertion(AfterOtherSide, ";")
 143            << FixItHint::CreateRemoval(
 144                   CharSourceRange::getTokenRange(AfterOtherSide, IfEnd));
 145     } else {
 146       Diag << FixItHint::CreateRemoval(
 147           CharSourceRange::getTokenRange(IfBegin, IfEnd));
 148     }
 149
 150     // For compound statements also remove the right brace at the end.
 151     if (isa<CompoundStmt>(Body))
 152       Diag << FixItHint::CreateRemoval(
 153           CharSourceRange::getTokenRange(Body->getEndLoc(), Body->getEndLoc()));
 154
 155     // For "and" binary operations we remove the "and" operation with the
 156     // condition variable from the inner if.
 157   } else {
 158     const auto *CondOp =
 159         cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts());
 160     const auto *LeftDRE =
 161         dyn_cast<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts());
 162     if (LeftDRE && LeftDRE->getDecl() == CondVar) {
 163       SourceLocation BeforeRHS =
 164           CondOp->getRHS()->getBeginLoc().getLocWithOffset(-1);
 165       Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
 166           CondOp->getLHS()->getBeginLoc(), BeforeRHS));
 167     } else {
 168       SourceLocation AfterLHS =
 169           Lexer::findNextToken(CondOp->getLHS()->getEndLoc(),
 170                                *Result.SourceManager, getLangOpts())
 171               ->getLocation();
 172       Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
 173           AfterLHS, CondOp->getRHS()->getEndLoc()));
 174     }
 175   }
 176 }
 177
 178 } // namespace clang::tidy::bugprone