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(LHS: PrevS->getEndLoc(), |
38 | RHS: MutS->getBeginLoc()) && |
39 | SM.isBeforeInTranslationUnit(LHS: MutS->getEndLoc(), RHS: NextS->getBeginLoc()); |
40 | } |
41 | |
42 | void RedundantBranchConditionCheck::registerMatchers(MatchFinder *Finder) { |
43 | const auto ImmutableVar = |
44 | varDecl(anyOf(parmVarDecl(), hasLocalStorage()), hasType(InnerMatcher: isInteger()), |
45 | unless(hasType(InnerMatcher: isVolatileQualified()))) |
46 | .bind(ID: CondVarStr); |
47 | Finder->addMatcher( |
48 | NodeMatch: ifStmt( |
49 | hasCondition(InnerMatcher: anyOf( |
50 | declRefExpr(hasDeclaration(InnerMatcher: ImmutableVar)).bind(ID: OuterIfVar1Str), |
51 | binaryOperator( |
52 | hasOperatorName(Name: "&&" ), |
53 | hasEitherOperand(InnerMatcher: declRefExpr(hasDeclaration(InnerMatcher: ImmutableVar)) |
54 | .bind(ID: OuterIfVar2Str))))), |
55 | hasThen(InnerMatcher: hasDescendant( |
56 | ifStmt(hasCondition(InnerMatcher: anyOf( |
57 | declRefExpr(hasDeclaration( |
58 | InnerMatcher: varDecl(equalsBoundNode(ID: CondVarStr)))) |
59 | .bind(ID: InnerIfVar1Str), |
60 | binaryOperator( |
61 | hasAnyOperatorName("&&" , "||" ), |
62 | hasEitherOperand( |
63 | InnerMatcher: declRefExpr(hasDeclaration(InnerMatcher: varDecl( |
64 | equalsBoundNode(ID: CondVarStr)))) |
65 | .bind(ID: InnerIfVar2Str)))))) |
66 | .bind(ID: InnerIfStr))), |
67 | forFunction(InnerMatcher: functionDecl().bind(ID: FuncStr))) |
68 | .bind(ID: OuterIfStr), |
69 | Action: 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>(ID: OuterIfStr); |
75 | const auto *InnerIf = Result.Nodes.getNodeAs<IfStmt>(ID: InnerIfStr); |
76 | const auto *CondVar = Result.Nodes.getNodeAs<VarDecl>(ID: CondVarStr); |
77 | const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>(ID: FuncStr); |
78 | |
79 | const DeclRefExpr *OuterIfVar = nullptr, *InnerIfVar = nullptr; |
80 | if (const auto *Inner = Result.Nodes.getNodeAs<DeclRefExpr>(ID: InnerIfVar1Str)) |
81 | InnerIfVar = Inner; |
82 | else |
83 | InnerIfVar = Result.Nodes.getNodeAs<DeclRefExpr>(ID: InnerIfVar2Str); |
84 | if (const auto *Outer = Result.Nodes.getNodeAs<DeclRefExpr>(ID: OuterIfVar1Str)) |
85 | OuterIfVar = Outer; |
86 | else |
87 | OuterIfVar = Result.Nodes.getNodeAs<DeclRefExpr>(ID: 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(Loc: InnerIf->getBeginLoc(), Description: "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>(Val: InnerIf->getCond()->IgnoreParenImpCasts()); |
111 | |
112 | if (isa<DeclRefExpr>(Val: 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>(Val: 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(Offset: -1); |
127 | |
128 | // For compound statements also remove the left brace. |
129 | if (isa<CompoundStmt>(Val: Body)) |
130 | IfEnd = Body->getBeginLoc(); |
131 | |
132 | // If the other side has side effects then keep it. |
133 | if (OtherSide && OtherSide->HasSideEffects(Ctx: *Result.Context)) { |
134 | SourceLocation BeforeOtherSide = |
135 | OtherSide->getBeginLoc().getLocWithOffset(-1); |
136 | SourceLocation AfterOtherSide = |
137 | Lexer::findNextToken(Loc: OtherSide->getEndLoc(), SM: *Result.SourceManager, |
138 | LangOpts: getLangOpts()) |
139 | ->getLocation(); |
140 | Diag << FixItHint::CreateRemoval( |
141 | RemoveRange: CharSourceRange::getTokenRange(B: IfBegin, E: BeforeOtherSide)) |
142 | << FixItHint::CreateInsertion(InsertionLoc: AfterOtherSide, Code: ";" ) |
143 | << FixItHint::CreateRemoval( |
144 | RemoveRange: CharSourceRange::getTokenRange(B: AfterOtherSide, E: IfEnd)); |
145 | } else { |
146 | Diag << FixItHint::CreateRemoval( |
147 | RemoveRange: CharSourceRange::getTokenRange(B: IfBegin, E: IfEnd)); |
148 | } |
149 | |
150 | // For compound statements also remove the right brace at the end. |
151 | if (isa<CompoundStmt>(Val: Body)) |
152 | Diag << FixItHint::CreateRemoval( |
153 | RemoveRange: CharSourceRange::getTokenRange(B: Body->getEndLoc(), E: 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>(Val: InnerIf->getCond()->IgnoreParenImpCasts()); |
160 | const auto *LeftDRE = |
161 | dyn_cast<DeclRefExpr>(Val: 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(Loc: CondOp->getLHS()->getEndLoc(), |
170 | SM: *Result.SourceManager, LangOpts: getLangOpts()) |
171 | ->getLocation(); |
172 | Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( |
173 | AfterLHS, CondOp->getRHS()->getEndLoc())); |
174 | } |
175 | } |
176 | } |
177 | |
178 | } // namespace clang::tidy::bugprone |
179 | |