blob: 265cb385fdc366c1851ff4cca70fa44d2cec702b [file] [log] [blame]
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001//===--- RedundantExpressionCheck.cpp - clang-tidy-------------------------===//
2//
3// The LLVM Compiler Infrastructure
4//
5// This file is distributed under the University of Illinois Open Source
6// License. See LICENSE.TXT for details.
7//
8//===----------------------------------------------------------------------===//
9
10#include "RedundantExpressionCheck.h"
11#include "../utils/Matchers.h"
Etienne Bergeronc87599f2016-05-12 04:32:47 +000012#include "../utils/OptionsUtils.h"
Etienne Bergeronbda187d2016-04-26 17:30:30 +000013#include "clang/AST/ASTContext.h"
14#include "clang/ASTMatchers/ASTMatchFinder.h"
Eugene Zelenko90c117a2016-11-01 18:33:50 +000015#include "clang/Basic/LLVM.h"
16#include "clang/Basic/SourceLocation.h"
17#include "clang/Basic/SourceManager.h"
Etienne Bergeronc87599f2016-05-12 04:32:47 +000018#include "clang/Lex/Lexer.h"
Eugene Zelenko90c117a2016-11-01 18:33:50 +000019#include "llvm/ADT/APInt.h"
20#include "llvm/ADT/APSInt.h"
21#include "llvm/ADT/FoldingSet.h"
22#include "llvm/Support/Casting.h"
23#include <algorithm>
24#include <cassert>
25#include <cstdint>
Eugene Zelenko90c117a2016-11-01 18:33:50 +000026#include <string>
27#include <vector>
Etienne Bergeronbda187d2016-04-26 17:30:30 +000028
29using namespace clang::ast_matchers;
Etienne Bergeron00639bc2016-07-07 04:03:05 +000030using namespace clang::tidy::matchers;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000031
32namespace clang {
33namespace tidy {
34namespace misc {
35
Etienne Bergeron00639bc2016-07-07 04:03:05 +000036namespace {
37using llvm::APSInt;
38} // namespace
39
Gabor Horvathec87e172017-11-07 13:17:58 +000040static const llvm::StringSet<> KnownBannedMacroNames = {"EAGAIN", "EWOULDBLOCK",
41 "SIGCLD", "SIGCHLD"};
Etienne Bergeronc87599f2016-05-12 04:32:47 +000042
Etienne Bergeron00639bc2016-07-07 04:03:05 +000043static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
44 Result = Value;
45 ++Result;
46 return Value < Result;
47}
48
Etienne Bergeronc87599f2016-05-12 04:32:47 +000049static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
50 const NestedNameSpecifier *Right) {
51 llvm::FoldingSetNodeID LeftID, RightID;
52 Left->Profile(LeftID);
53 Right->Profile(RightID);
54 return LeftID == RightID;
55}
56
57static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
Etienne Bergeronbda187d2016-04-26 17:30:30 +000058 if (!Left || !Right)
59 return !Left && !Right;
60
61 Left = Left->IgnoreParens();
62 Right = Right->IgnoreParens();
63
64 // Compare classes.
65 if (Left->getStmtClass() != Right->getStmtClass())
66 return false;
67
68 // Compare children.
69 Expr::const_child_iterator LeftIter = Left->child_begin();
70 Expr::const_child_iterator RightIter = Right->child_begin();
71 while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
Etienne Bergeronc87599f2016-05-12 04:32:47 +000072 if (!areEquivalentExpr(dyn_cast<Expr>(*LeftIter),
73 dyn_cast<Expr>(*RightIter)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +000074 return false;
75 ++LeftIter;
76 ++RightIter;
77 }
78 if (LeftIter != Left->child_end() || RightIter != Right->child_end())
79 return false;
80
81 // Perform extra checks.
82 switch (Left->getStmtClass()) {
83 default:
84 return false;
85
86 case Stmt::CharacterLiteralClass:
87 return cast<CharacterLiteral>(Left)->getValue() ==
88 cast<CharacterLiteral>(Right)->getValue();
89 case Stmt::IntegerLiteralClass: {
90 llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
91 llvm::APInt RightLit = cast<IntegerLiteral>(Right)->getValue();
Etienne Bergeronc87599f2016-05-12 04:32:47 +000092 return LeftLit.getBitWidth() == RightLit.getBitWidth() &&
93 LeftLit == RightLit;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000094 }
95 case Stmt::FloatingLiteralClass:
96 return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
97 cast<FloatingLiteral>(Right)->getValue());
98 case Stmt::StringLiteralClass:
99 return cast<StringLiteral>(Left)->getBytes() ==
100 cast<StringLiteral>(Right)->getBytes();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000101 case Stmt::DependentScopeDeclRefExprClass:
102 if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
103 cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
104 return false;
105 return areEquivalentNameSpecifier(
106 cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
107 cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000108 case Stmt::DeclRefExprClass:
109 return cast<DeclRefExpr>(Left)->getDecl() ==
110 cast<DeclRefExpr>(Right)->getDecl();
111 case Stmt::MemberExprClass:
112 return cast<MemberExpr>(Left)->getMemberDecl() ==
113 cast<MemberExpr>(Right)->getMemberDecl();
Gabor Horvathec87e172017-11-07 13:17:58 +0000114 case Stmt::CXXFunctionalCastExprClass:
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000115 case Stmt::CStyleCastExprClass:
Gabor Horvathec87e172017-11-07 13:17:58 +0000116 return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
117 cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000118 case Stmt::CallExprClass:
119 case Stmt::ImplicitCastExprClass:
120 case Stmt::ArraySubscriptExprClass:
121 return true;
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000122 case Stmt::UnaryOperatorClass:
123 if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
124 return false;
125 return cast<UnaryOperator>(Left)->getOpcode() ==
126 cast<UnaryOperator>(Right)->getOpcode();
127 case Stmt::BinaryOperatorClass:
128 return cast<BinaryOperator>(Left)->getOpcode() ==
129 cast<BinaryOperator>(Right)->getOpcode();
130 }
131}
132
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000133// For a given expression 'x', returns whether the ranges covered by the
134// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
135static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
136 const APSInt &ValueLHS,
137 BinaryOperatorKind OpcodeRHS,
138 const APSInt &ValueRHS) {
139 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
140 "Values must be ordered");
141 // Handle the case where constants are the same: x <= 4 <==> x <= 4.
142 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
143 return OpcodeLHS == OpcodeRHS;
144
145 // Handle the case where constants are off by one: x <= 4 <==> x < 5.
146 APSInt ValueLHS_plus1;
147 return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
148 (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
149 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
150 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0;
151}
152
153// For a given expression 'x', returns whether the ranges covered by the
154// relational operators are fully disjoint (i.e. x < 4 and x > 7).
155static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
156 const APSInt &ValueLHS,
157 BinaryOperatorKind OpcodeRHS,
158 const APSInt &ValueRHS) {
159 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
160 "Values must be ordered");
161
162 // Handle cases where the constants are the same.
163 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
164 switch (OpcodeLHS) {
165 case BO_EQ:
166 return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
167 case BO_NE:
168 return OpcodeRHS == BO_EQ;
169 case BO_LE:
170 return OpcodeRHS == BO_GT;
171 case BO_GE:
172 return OpcodeRHS == BO_LT;
173 case BO_LT:
174 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
175 case BO_GT:
176 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
177 default:
178 return false;
179 }
180 }
181
182 // Handle cases where the constants are different.
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000183 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LT || OpcodeLHS == BO_LE) &&
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000184 (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
185 return true;
186
187 // Handle the case where constants are off by one: x > 5 && x < 6.
188 APSInt ValueLHS_plus1;
189 if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
190 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
191 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
192 return true;
193
194 return false;
195}
196
197// Returns whether the ranges covered by the union of both relational
198// expressions covers the whole domain (i.e. x < 10 and x > 0).
199static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
200 const APSInt &ValueLHS,
201 BinaryOperatorKind OpcodeRHS,
202 const APSInt &ValueRHS) {
203 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
204 "Values must be ordered");
205
206 // Handle cases where the constants are the same: x < 5 || x >= 5.
207 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
208 switch (OpcodeLHS) {
209 case BO_EQ:
210 return OpcodeRHS == BO_NE;
211 case BO_NE:
212 return OpcodeRHS == BO_EQ;
213 case BO_LE:
214 return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
215 case BO_LT:
216 return OpcodeRHS == BO_GE;
217 case BO_GE:
218 return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
219 case BO_GT:
220 return OpcodeRHS == BO_LE;
221 default:
222 return false;
223 }
224 }
225
226 // Handle the case where constants are off by one: x <= 4 || x >= 5.
227 APSInt ValueLHS_plus1;
228 if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
229 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
230 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
231 return true;
232
233 // Handle cases where the constants are different: x > 4 || x <= 7.
234 if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
235 (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
236 return true;
237
Alexander Kornienkoc3acd2e2017-03-23 15:13:54 +0000238 // Handle cases where constants are different but both ops are !=, like:
239 // x != 5 || x != 10
240 if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE)
241 return true;
242
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000243 return false;
244}
245
246static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
247 const APSInt &ValueLHS,
248 BinaryOperatorKind OpcodeRHS,
249 const APSInt &ValueRHS) {
250 int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
251 switch (OpcodeLHS) {
252 case BO_EQ:
253 return OpcodeRHS == BO_EQ && Comparison == 0;
254 case BO_NE:
255 return (OpcodeRHS == BO_NE && Comparison == 0) ||
256 (OpcodeRHS == BO_EQ && Comparison != 0) ||
257 (OpcodeRHS == BO_LT && Comparison >= 0) ||
258 (OpcodeRHS == BO_LE && Comparison > 0) ||
259 (OpcodeRHS == BO_GT && Comparison <= 0) ||
260 (OpcodeRHS == BO_GE && Comparison < 0);
261
262 case BO_LT:
263 return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
264 (OpcodeRHS == BO_LE && Comparison > 0) ||
265 (OpcodeRHS == BO_EQ && Comparison > 0));
266 case BO_GT:
267 return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
268 (OpcodeRHS == BO_GE && Comparison < 0) ||
269 (OpcodeRHS == BO_EQ && Comparison < 0));
270 case BO_LE:
271 return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
272 Comparison >= 0;
273 case BO_GE:
274 return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
275 Comparison <= 0;
276 default:
277 return false;
278 }
279}
280
Gabor Horvathec87e172017-11-07 13:17:58 +0000281static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
282 APSInt &Value) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000283 if (Opcode == BO_Sub) {
284 Opcode = BO_Add;
285 Value = -Value;
286 }
287}
288
289AST_MATCHER(Expr, isIntegerConstantExpr) {
290 if (Node.isInstantiationDependent())
291 return false;
292 return Node.isIntegerConstantExpr(Finder->getASTContext());
293}
294
Gabor Horvathec87e172017-11-07 13:17:58 +0000295AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
296 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
297}
298
299AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
300 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
301}
302
303AST_MATCHER(CallExpr, parametersAreEquivalent) {
304 return Node.getNumArgs() == 2 &&
305 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
306}
307
308AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
309 return Node.getOperatorLoc().isMacroID();
310}
311
312AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
313 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
314}
315
316AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
317
318AST_MATCHER_P(Expr, expandedByMacro, llvm::StringSet<>, Names) {
319 const SourceManager &SM = Finder->getASTContext().getSourceManager();
320 const LangOptions &LO = Finder->getASTContext().getLangOpts();
321 SourceLocation Loc = Node.getExprLoc();
322 while (Loc.isMacroID()) {
323 StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
324 if (Names.count(MacroName))
325 return true;
326 Loc = SM.getImmediateMacroCallerLoc(Loc);
327 }
328 return false;
329}
330
331// Returns a matcher for integer constant expressions.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000332static ast_matchers::internal::Matcher<Expr>
333matchIntegerConstantExpr(StringRef Id) {
334 std::string CstId = (Id + "-const").str();
335 return expr(isIntegerConstantExpr()).bind(CstId);
336}
337
Gabor Horvathec87e172017-11-07 13:17:58 +0000338// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
339// name 'Id' and stores it into 'ConstExpr', the value of the expression is
340// stored into `Value`.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000341static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
Gabor Horvathec87e172017-11-07 13:17:58 +0000342 StringRef Id, APSInt &Value,
343 const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000344 std::string CstId = (Id + "-const").str();
Gabor Horvathec87e172017-11-07 13:17:58 +0000345 ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
346 return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000347}
348
Gabor Horvathec87e172017-11-07 13:17:58 +0000349// Overloaded `retrieveIntegerConstantExpr` for compatibility.
350static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
351 StringRef Id, APSInt &Value) {
352 const Expr *ConstExpr = nullptr;
353 return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
354}
355
356// Returns a matcher for symbolic expressions (matches every expression except
357// ingeter constant expressions).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000358static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
359 std::string SymId = (Id + "-sym").str();
360 return ignoringParenImpCasts(
361 expr(unless(isIntegerConstantExpr())).bind(SymId));
362}
363
Gabor Horvathec87e172017-11-07 13:17:58 +0000364// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
365// stores it into 'SymExpr'.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000366static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
367 StringRef Id, const Expr *&SymExpr) {
368 std::string SymId = (Id + "-sym").str();
369 if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
370 SymExpr = Node;
371 return true;
372 }
373 return false;
374}
375
376// Match a binary operator between a symbolic expression and an integer constant
377// expression.
378static ast_matchers::internal::Matcher<Expr>
379matchBinOpIntegerConstantExpr(StringRef Id) {
380 const auto BinOpCstExpr =
381 expr(
382 anyOf(binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("|"),
383 hasOperatorName("&")),
384 hasEitherOperand(matchSymbolicExpr(Id)),
385 hasEitherOperand(matchIntegerConstantExpr(Id))),
386 binaryOperator(hasOperatorName("-"),
387 hasLHS(matchSymbolicExpr(Id)),
388 hasRHS(matchIntegerConstantExpr(Id)))))
389 .bind(Id);
390 return ignoringParenImpCasts(BinOpCstExpr);
391}
392
Gabor Horvathec87e172017-11-07 13:17:58 +0000393// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000394// name 'Id'.
395static bool
396retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
397 StringRef Id, BinaryOperatorKind &Opcode,
398 const Expr *&Symbol, APSInt &Value) {
399 if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
400 Opcode = BinExpr->getOpcode();
401 return retrieveSymbolicExpr(Result, Id, Symbol) &&
402 retrieveIntegerConstantExpr(Result, Id, Value);
403 }
404 return false;
405}
406
Gabor Horvathec87e172017-11-07 13:17:58 +0000407// Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000408static ast_matchers::internal::Matcher<Expr>
409matchRelationalIntegerConstantExpr(StringRef Id) {
410 std::string CastId = (Id + "-cast").str();
411 std::string SwapId = (Id + "-swap").str();
412 std::string NegateId = (Id + "-negate").str();
413
414 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
415 isComparisonOperator(), expr().bind(Id),
416 anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
417 hasRHS(matchIntegerConstantExpr(Id))),
418 allOf(hasLHS(matchIntegerConstantExpr(Id)),
419 hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
420
421 // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
422 // to if (x != 0)).
423 const auto CastExpr =
424 implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
425 hasSourceExpression(matchSymbolicExpr(Id)))
426 .bind(CastId);
427
428 const auto NegateRelationalExpr =
429 unaryOperator(hasOperatorName("!"),
430 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
431 .bind(NegateId);
432
Gabor Horvathec87e172017-11-07 13:17:58 +0000433 // Do not bind to double negation.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000434 const auto NegateNegateRelationalExpr =
435 unaryOperator(hasOperatorName("!"),
436 hasUnaryOperand(unaryOperator(
437 hasOperatorName("!"),
438 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
439
440 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
441 NegateNegateRelationalExpr);
442}
443
Gabor Horvathec87e172017-11-07 13:17:58 +0000444// Retrieves sub-expressions matched by 'matchRelationalIntegerConstantExpr' with
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000445// name 'Id'.
Gabor Horvathec87e172017-11-07 13:17:58 +0000446static bool retrieveRelationalIntegerConstantExpr(
447 const MatchFinder::MatchResult &Result, StringRef Id,
448 const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
449 APSInt &Value, const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000450 std::string CastId = (Id + "-cast").str();
451 std::string SwapId = (Id + "-swap").str();
452 std::string NegateId = (Id + "-negate").str();
453
454 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
455 // Operand received with explicit comparator.
456 Opcode = Bin->getOpcode();
457 OperandExpr = Bin;
Gabor Horvathec87e172017-11-07 13:17:58 +0000458
459 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000460 return false;
Gabor Horvathec87e172017-11-07 13:17:58 +0000461
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000462 } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
463 // Operand received with implicit comparator (cast).
464 Opcode = BO_NE;
465 OperandExpr = Cast;
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000466 Value = APSInt(32, false);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000467 } else {
468 return false;
469 }
470
471 if (!retrieveSymbolicExpr(Result, Id, Symbol))
472 return false;
473
474 if (Result.Nodes.getNodeAs<Expr>(SwapId))
475 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
476 if (Result.Nodes.getNodeAs<Expr>(NegateId))
477 Opcode = BinaryOperator::negateComparisonOp(Opcode);
Gabor Horvathec87e172017-11-07 13:17:58 +0000478 return true;
479}
480
481// Checks for expressions like (X == 4) && (Y != 9)
482static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
483 const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
484 const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
485
486 if (!LhsBinOp || !RhsBinOp)
487 return false;
488
489 if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
490 LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
491 (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
492 RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
493 return true;
494 return false;
495}
496
497// Retrieves integer constant subexpressions from binary operator expressions
498// that have two equivalent sides
499// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
500static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
501 BinaryOperatorKind &MainOpcode,
502 BinaryOperatorKind &SideOpcode,
503 const Expr *&LhsConst,
504 const Expr *&RhsConst,
505 const ASTContext *AstCtx) {
506 assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
507 "Both sides of binary operator must be constant expressions!");
508
509 MainOpcode = BinOp->getOpcode();
510
511 const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
512 const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
513
514 LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
515 ? BinOpLhs->getLHS()
516 : BinOpLhs->getRHS();
517 RhsConst = BinOpRhs->getLHS()->isIntegerConstantExpr(*AstCtx)
518 ? BinOpRhs->getLHS()
519 : BinOpRhs->getRHS();
520
521 if (!LhsConst || !RhsConst)
522 return false;
523
524 assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
525 "Sides of the binary operator must be equivalent expressions!");
526
527 SideOpcode = BinOpLhs->getOpcode();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000528
529 return true;
530}
531
Gabor Horvathec87e172017-11-07 13:17:58 +0000532static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
533 const Expr *RhsExpr,
534 const ASTContext *AstCtx) {
535 if (!LhsExpr || !RhsExpr)
536 return false;
537
538 SourceLocation LhsLoc = LhsExpr->getExprLoc();
539 SourceLocation RhsLoc = RhsExpr->getExprLoc();
540
541 if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
542 return false;
543
544 const SourceManager &SM = AstCtx->getSourceManager();
545 const LangOptions &LO = AstCtx->getLangOpts();
546
547 return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
548 Lexer::getImmediateMacroName(RhsLoc, SM, LO));
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000549}
550
Gabor Horvathec87e172017-11-07 13:17:58 +0000551static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr, const Expr *&RhsExpr) {
552 if (!LhsExpr || !RhsExpr)
553 return false;
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000554
Gabor Horvathec87e172017-11-07 13:17:58 +0000555 SourceLocation LhsLoc = LhsExpr->getExprLoc();
556 SourceLocation RhsLoc = RhsExpr->getExprLoc();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000557
Gabor Horvathec87e172017-11-07 13:17:58 +0000558 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000559}
560
561void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
562 const auto AnyLiteralExpr = ignoringParenImpCasts(
563 anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
564
Gabor Horvathec87e172017-11-07 13:17:58 +0000565 const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(KnownBannedMacroNames));
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000566
Gabor Horvathec87e172017-11-07 13:17:58 +0000567 // Binary with equivalent operands, like (X != 2 && X != 2).
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000568 Finder->addMatcher(
569 binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
570 hasOperatorName("%"), hasOperatorName("|"),
571 hasOperatorName("&"), hasOperatorName("^"),
572 matchers::isComparisonOperator(),
573 hasOperatorName("&&"), hasOperatorName("||"),
574 hasOperatorName("=")),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000575 operandsAreEquivalent(),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000576 // Filter noisy false positives.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000577 unless(isInTemplateInstantiation()),
578 unless(binaryOperatorIsInMacro()),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000579 unless(hasType(realFloatingPointType())),
580 unless(hasEitherOperand(hasType(realFloatingPointType()))),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000581 unless(hasLHS(AnyLiteralExpr)),
582 unless(hasDescendant(BannedIntegerLiteral)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000583 .bind("binary"),
584 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000585
Gabor Horvathec87e172017-11-07 13:17:58 +0000586 // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000587 Finder->addMatcher(
588 conditionalOperator(expressionsAreEquivalent(),
589 // Filter noisy false positives.
590 unless(conditionalOperatorIsInMacro()),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000591 unless(isInTemplateInstantiation()))
592 .bind("cond"),
593 this);
594
Gabor Horvathec87e172017-11-07 13:17:58 +0000595 // Overloaded operators with equivalent operands.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000596 Finder->addMatcher(
597 cxxOperatorCallExpr(
598 anyOf(
599 hasOverloadedOperatorName("-"), hasOverloadedOperatorName("/"),
600 hasOverloadedOperatorName("%"), hasOverloadedOperatorName("|"),
601 hasOverloadedOperatorName("&"), hasOverloadedOperatorName("^"),
602 hasOverloadedOperatorName("=="), hasOverloadedOperatorName("!="),
603 hasOverloadedOperatorName("<"), hasOverloadedOperatorName("<="),
604 hasOverloadedOperatorName(">"), hasOverloadedOperatorName(">="),
605 hasOverloadedOperatorName("&&"), hasOverloadedOperatorName("||"),
606 hasOverloadedOperatorName("=")),
607 parametersAreEquivalent(),
608 // Filter noisy false positives.
609 unless(isMacro()), unless(isInTemplateInstantiation()))
610 .bind("call"),
611 this);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000612
613 // Match common expressions and apply more checks to find redundant
614 // sub-expressions.
615 // a) Expr <op> K1 == K2
616 // b) Expr <op> K1 == Expr
617 // c) Expr <op> K1 == Expr <op> K2
618 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
619 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
620 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
621 const auto CstRight = matchIntegerConstantExpr("rhs");
622 const auto SymRight = matchSymbolicExpr("rhs");
623
624 // Match expressions like: x <op> 0xFF == 0xF00.
625 Finder->addMatcher(binaryOperator(isComparisonOperator(),
626 hasEitherOperand(BinOpCstLeft),
627 hasEitherOperand(CstRight))
628 .bind("binop-const-compare-to-const"),
629 this);
630
631 // Match expressions like: x <op> 0xFF == x.
632 Finder->addMatcher(
633 binaryOperator(isComparisonOperator(),
634 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
635 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
636 .bind("binop-const-compare-to-sym"),
637 this);
638
639 // Match expressions like: x <op> 10 == x <op> 12.
640 Finder->addMatcher(binaryOperator(isComparisonOperator(),
641 hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
642 // Already reported as redundant.
643 unless(operandsAreEquivalent()))
644 .bind("binop-const-compare-to-binop-const"),
645 this);
646
647 // Match relational expressions combined with logical operators and find
648 // redundant sub-expressions.
649 // see: 'checkRelationalExpr'
650
651 // Match expressions like: x < 2 && x > 2.
652 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
653 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
654 Finder->addMatcher(
655 binaryOperator(anyOf(hasOperatorName("||"), hasOperatorName("&&")),
656 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
657 // Already reported as redundant.
658 unless(operandsAreEquivalent()))
659 .bind("comparisons-of-symbol-and-const"),
660 this);
661}
662
663void RedundantExpressionCheck::checkArithmeticExpr(
664 const MatchFinder::MatchResult &Result) {
665 APSInt LhsValue, RhsValue;
666 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
667 BinaryOperatorKind LhsOpcode, RhsOpcode;
668
669 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
670 "binop-const-compare-to-sym")) {
671 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
672 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
673 LhsValue) ||
674 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
675 !areEquivalentExpr(LhsSymbol, RhsSymbol))
676 return;
677
678 // Check expressions: x + k == x or x - k == x.
679 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
680 if ((LhsValue != 0 && Opcode == BO_EQ) ||
681 (LhsValue == 0 && Opcode == BO_NE))
682 diag(ComparisonOperator->getOperatorLoc(),
683 "logical expression is always false");
684 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
685 (LhsValue != 0 && Opcode == BO_NE))
686 diag(ComparisonOperator->getOperatorLoc(),
687 "logical expression is always true");
688 }
689 } else if (const auto *ComparisonOperator =
690 Result.Nodes.getNodeAs<BinaryOperator>(
691 "binop-const-compare-to-binop-const")) {
692 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
693
694 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
695 LhsValue) ||
696 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
697 RhsValue) ||
698 !areEquivalentExpr(LhsSymbol, RhsSymbol))
699 return;
700
Gabor Horvathec87e172017-11-07 13:17:58 +0000701 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
702 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000703
704 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
705 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
706 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
707 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
708 diag(ComparisonOperator->getOperatorLoc(),
709 "logical expression is always true");
710 } else if ((Opcode == BO_EQ &&
711 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
712 (Opcode == BO_NE &&
713 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
714 diag(ComparisonOperator->getOperatorLoc(),
715 "logical expression is always false");
716 }
717 }
718 }
719}
720
721void RedundantExpressionCheck::checkBitwiseExpr(
722 const MatchFinder::MatchResult &Result) {
723 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
724 "binop-const-compare-to-const")) {
725 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
726
727 APSInt LhsValue, RhsValue;
728 const Expr *LhsSymbol = nullptr;
729 BinaryOperatorKind LhsOpcode;
730 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
731 LhsValue) ||
732 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
733 return;
734
735 uint64_t LhsConstant = LhsValue.getZExtValue();
736 uint64_t RhsConstant = RhsValue.getZExtValue();
737 SourceLocation Loc = ComparisonOperator->getOperatorLoc();
738
739 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
740 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
741 if (Opcode == BO_EQ)
742 diag(Loc, "logical expression is always false");
743 else if (Opcode == BO_NE)
744 diag(Loc, "logical expression is always true");
745 }
746
747 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
748 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
749 if (Opcode == BO_EQ)
750 diag(Loc, "logical expression is always false");
751 else if (Opcode == BO_NE)
752 diag(Loc, "logical expression is always true");
753 }
754 }
755}
756
757void RedundantExpressionCheck::checkRelationalExpr(
758 const MatchFinder::MatchResult &Result) {
759 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
760 "comparisons-of-symbol-and-const")) {
761 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
Gabor Horvathec87e172017-11-07 13:17:58 +0000762 // E.g.: (X < 2) && (X > 4)
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000763 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
764
765 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000766 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
Gabor Horvathec87e172017-11-07 13:17:58 +0000767 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000768 BinaryOperatorKind LhsOpcode, RhsOpcode;
Gabor Horvathec87e172017-11-07 13:17:58 +0000769 APSInt LhsValue, RhsValue;
770
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000771 if (!retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000772 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000773 !retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000774 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000775 !areEquivalentExpr(LhsSymbol, RhsSymbol))
776 return;
777
Gabor Horvathec87e172017-11-07 13:17:58 +0000778 // Bring expr to a canonical form: smallest constant must be on the left.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000779 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
780 std::swap(LhsExpr, RhsExpr);
781 std::swap(LhsValue, RhsValue);
782 std::swap(LhsSymbol, RhsSymbol);
783 std::swap(LhsOpcode, RhsOpcode);
784 }
785
Gabor Horvathec87e172017-11-07 13:17:58 +0000786 // Constants come from two different macros, or one of them is a macro.
787 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
788 areExprsMacroAndNonMacro(LhsConst, RhsConst))
789 return;
790
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000791 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
792 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
793 diag(ComparisonOperator->getOperatorLoc(),
Gabor Horvathec87e172017-11-07 13:17:58 +0000794 "equivalent expression on both sides of logical operator");
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000795 return;
796 }
797
798 if (Opcode == BO_LAnd) {
799 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
800 diag(ComparisonOperator->getOperatorLoc(),
801 "logical expression is always false");
802 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
803 diag(LhsExpr->getExprLoc(), "expression is redundant");
804 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
805 diag(RhsExpr->getExprLoc(), "expression is redundant");
806 }
807 }
808
809 if (Opcode == BO_LOr) {
810 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
811 diag(ComparisonOperator->getOperatorLoc(),
812 "logical expression is always true");
813 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
814 diag(RhsExpr->getExprLoc(), "expression is redundant");
815 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
816 diag(LhsExpr->getExprLoc(), "expression is redundant");
817 }
818 }
819 }
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000820}
821
822void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000823 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000824
Gabor Horvathec87e172017-11-07 13:17:58 +0000825 // If the expression's constants are macros, check whether they are
826 // intentional.
827 if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
828 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
829 BinaryOperatorKind MainOpcode, SideOpcode;
830
831 if(!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode, LhsConst,
832 RhsConst, Result.Context))
833 return;
834
835 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
836 areExprsMacroAndNonMacro(LhsConst, RhsConst))
837 return;
838 }
839
840 diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
841 }
842
843 if (const auto *CondOp =
844 Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
845 const Expr *TrueExpr = CondOp->getTrueExpr();
846 const Expr *FalseExpr = CondOp->getFalseExpr();
847
848 if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
849 areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
850 return;
851 diag(CondOp->getColonLoc(),
852 "'true' and 'false' expressions are equivalent");
853 }
854
855 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
856 diag(Call->getOperatorLoc(),
857 "both sides of overloaded operator are equivalent");
858 }
859
860 // Check for the following bound expressions:
861 // - "binop-const-compare-to-sym",
862 // - "binop-const-compare-to-binop-const",
863 // Produced message:
864 // -> "logical expression is always false/true"
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000865 checkArithmeticExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +0000866
867 // Check for the following bound expression:
868 // - "binop-const-compare-to-const",
869 // Produced message:
870 // -> "logical expression is always false/true"
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000871 checkBitwiseExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +0000872
873 // Check for te following bound expression:
874 // - "comparisons-of-symbol-and-const",
875 // Produced messages:
876 // -> "equivalent expression on both sides of logical operator",
877 // -> "logical expression is always false/true"
878 // -> "expression is redundant"
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000879 checkRelationalExpr(Result);
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000880}
881
882} // namespace misc
883} // namespace tidy
884} // namespace clang