RedundantBranchConditionCheck.cpp 7.4 KB

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