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`).
isChangedBefore(const Stmt * S,const Stmt * NextS,const Stmt * PrevS,const VarDecl * Var,ASTContext * Context)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
registerMatchers(MatchFinder * Finder)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
check(const MatchFinder::MatchResult & Result)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
179