180 lines · cpp
1//===----------------------------------------------------------------------===//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-exception6//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 16using namespace clang::ast_matchers;17using clang::tidy::utils::hasPtrOrReferenceInFunc;18 19namespace clang::tidy::bugprone {20 21static const char CondVarStr[] = "cond_var";22static const char OuterIfStr[] = "outer_if";23static const char InnerIfStr[] = "inner_if";24static const char OuterIfVar1Str[] = "outer_if_var1";25static const char OuterIfVar2Str[] = "outer_if_var2";26static const char InnerIfVar1Str[] = "inner_if_var1";27static const char InnerIfVar2Str[] = "inner_if_var2";28static const char FuncStr[] = "func";29 30/// Returns whether `Var` is changed in range (`PrevS`..`NextS`).31static 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 42void 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 73void RedundantBranchConditionCheck::check(74 const MatchFinder::MatchResult &Result) {75 const auto *OuterIf = Result.Nodes.getNodeAs<IfStmt>(OuterIfStr);76 const auto *InnerIf = Result.Nodes.getNodeAs<IfStmt>(InnerIfStr);77 const auto *CondVar = Result.Nodes.getNodeAs<VarDecl>(CondVarStr);78 const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>(FuncStr);79 80 const DeclRefExpr *OuterIfVar = nullptr, *InnerIfVar = nullptr;81 if (const auto *Inner = Result.Nodes.getNodeAs<DeclRefExpr>(InnerIfVar1Str))82 InnerIfVar = Inner;83 else84 InnerIfVar = Result.Nodes.getNodeAs<DeclRefExpr>(InnerIfVar2Str);85 if (const auto *Outer = Result.Nodes.getNodeAs<DeclRefExpr>(OuterIfVar1Str))86 OuterIfVar = Outer;87 else88 OuterIfVar = Result.Nodes.getNodeAs<DeclRefExpr>(OuterIfVar2Str);89 90 if (OuterIfVar && InnerIfVar) {91 if (isChangedBefore(OuterIf->getThen(), InnerIfVar, OuterIfVar, CondVar,92 Result.Context))93 return;94 95 if (isChangedBefore(OuterIf->getCond(), InnerIfVar, OuterIfVar, CondVar,96 Result.Context))97 return;98 }99 100 // If the variable has an alias then it can be changed by that alias as well.101 // FIXME: could potentially support tracking pointers and references in the102 // future to improve catching true positives through aliases.103 if (hasPtrOrReferenceInFunc(Func, CondVar))104 return;105 106 auto Diag = diag(InnerIf->getBeginLoc(), "redundant condition %0") << CondVar;107 108 // For standalone condition variables and for "or" binary operations we simply109 // remove the inner `if`.110 const auto *BinOpCond =111 dyn_cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts());112 113 if (isa<DeclRefExpr>(InnerIf->getCond()->IgnoreParenImpCasts()) ||114 (BinOpCond && BinOpCond->getOpcode() == BO_LOr)) {115 const SourceLocation IfBegin = InnerIf->getBeginLoc();116 const Stmt *Body = InnerIf->getThen();117 const Expr *OtherSide = nullptr;118 if (BinOpCond) {119 const auto *LeftDRE =120 dyn_cast<DeclRefExpr>(BinOpCond->getLHS()->IgnoreParenImpCasts());121 if (LeftDRE && LeftDRE->getDecl() == CondVar)122 OtherSide = BinOpCond->getRHS();123 else124 OtherSide = BinOpCond->getLHS();125 }126 127 SourceLocation IfEnd = Body->getBeginLoc().getLocWithOffset(-1);128 129 // For compound statements also remove the left brace.130 if (isa<CompoundStmt>(Body))131 IfEnd = Body->getBeginLoc();132 133 // If the other side has side effects then keep it.134 if (OtherSide && OtherSide->HasSideEffects(*Result.Context)) {135 const SourceLocation BeforeOtherSide =136 OtherSide->getBeginLoc().getLocWithOffset(-1);137 const SourceLocation AfterOtherSide =138 Lexer::findNextToken(OtherSide->getEndLoc(), *Result.SourceManager,139 getLangOpts())140 ->getLocation();141 Diag << FixItHint::CreateRemoval(142 CharSourceRange::getTokenRange(IfBegin, BeforeOtherSide))143 << FixItHint::CreateInsertion(AfterOtherSide, ";")144 << FixItHint::CreateRemoval(145 CharSourceRange::getTokenRange(AfterOtherSide, IfEnd));146 } else {147 Diag << FixItHint::CreateRemoval(148 CharSourceRange::getTokenRange(IfBegin, IfEnd));149 }150 151 // For compound statements also remove the right brace at the end.152 if (isa<CompoundStmt>(Body))153 Diag << FixItHint::CreateRemoval(154 CharSourceRange::getTokenRange(Body->getEndLoc(), Body->getEndLoc()));155 156 // For "and" binary operations we remove the "and" operation with the157 // condition variable from the inner if.158 } else {159 const auto *CondOp =160 cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts());161 const auto *LeftDRE =162 dyn_cast<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts());163 if (LeftDRE && LeftDRE->getDecl() == CondVar) {164 const SourceLocation BeforeRHS =165 CondOp->getRHS()->getBeginLoc().getLocWithOffset(-1);166 Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(167 CondOp->getLHS()->getBeginLoc(), BeforeRHS));168 } else {169 const SourceLocation AfterLHS =170 Lexer::findNextToken(CondOp->getLHS()->getEndLoc(),171 *Result.SourceManager, getLangOpts())172 ->getLocation();173 Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(174 AfterLHS, CondOp->getRHS()->getEndLoc()));175 }176 }177}178 179} // namespace clang::tidy::bugprone180