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