blob: 535918f4f6f495428893bcd25a26ebcf9919c1f5 [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"
Etienne Bergeronc87599f2016-05-12 04:32:47 +000015#include "clang/Lex/Lexer.h"
Etienne Bergeronbda187d2016-04-26 17:30:30 +000016
17using namespace clang::ast_matchers;
Etienne Bergeron00639bc2016-07-07 04:03:05 +000018using namespace clang::tidy::matchers;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000019
20namespace clang {
21namespace tidy {
22namespace misc {
23
Etienne Bergeron00639bc2016-07-07 04:03:05 +000024namespace {
25using llvm::APSInt;
26} // namespace
27
Etienne Bergeronc87599f2016-05-12 04:32:47 +000028static const char KnownBannedMacroNames[] =
29 "EAGAIN;EWOULDBLOCK;SIGCLD;SIGCHLD;";
30
Etienne Bergeron00639bc2016-07-07 04:03:05 +000031static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
32 Result = Value;
33 ++Result;
34 return Value < Result;
35}
36
Etienne Bergeronc87599f2016-05-12 04:32:47 +000037static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
38 const NestedNameSpecifier *Right) {
39 llvm::FoldingSetNodeID LeftID, RightID;
40 Left->Profile(LeftID);
41 Right->Profile(RightID);
42 return LeftID == RightID;
43}
44
45static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
Etienne Bergeronbda187d2016-04-26 17:30:30 +000046 if (!Left || !Right)
47 return !Left && !Right;
48
49 Left = Left->IgnoreParens();
50 Right = Right->IgnoreParens();
51
52 // Compare classes.
53 if (Left->getStmtClass() != Right->getStmtClass())
54 return false;
55
56 // Compare children.
57 Expr::const_child_iterator LeftIter = Left->child_begin();
58 Expr::const_child_iterator RightIter = Right->child_begin();
59 while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
Etienne Bergeronc87599f2016-05-12 04:32:47 +000060 if (!areEquivalentExpr(dyn_cast<Expr>(*LeftIter),
61 dyn_cast<Expr>(*RightIter)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +000062 return false;
63 ++LeftIter;
64 ++RightIter;
65 }
66 if (LeftIter != Left->child_end() || RightIter != Right->child_end())
67 return false;
68
69 // Perform extra checks.
70 switch (Left->getStmtClass()) {
71 default:
72 return false;
73
74 case Stmt::CharacterLiteralClass:
75 return cast<CharacterLiteral>(Left)->getValue() ==
76 cast<CharacterLiteral>(Right)->getValue();
77 case Stmt::IntegerLiteralClass: {
78 llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
79 llvm::APInt RightLit = cast<IntegerLiteral>(Right)->getValue();
Etienne Bergeronc87599f2016-05-12 04:32:47 +000080 return LeftLit.getBitWidth() == RightLit.getBitWidth() &&
81 LeftLit == RightLit;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000082 }
83 case Stmt::FloatingLiteralClass:
84 return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
85 cast<FloatingLiteral>(Right)->getValue());
86 case Stmt::StringLiteralClass:
87 return cast<StringLiteral>(Left)->getBytes() ==
88 cast<StringLiteral>(Right)->getBytes();
89
Etienne Bergeronc87599f2016-05-12 04:32:47 +000090 case Stmt::DependentScopeDeclRefExprClass:
91 if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
92 cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
93 return false;
94 return areEquivalentNameSpecifier(
95 cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
96 cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
Etienne Bergeronbda187d2016-04-26 17:30:30 +000097 case Stmt::DeclRefExprClass:
98 return cast<DeclRefExpr>(Left)->getDecl() ==
99 cast<DeclRefExpr>(Right)->getDecl();
100 case Stmt::MemberExprClass:
101 return cast<MemberExpr>(Left)->getMemberDecl() ==
102 cast<MemberExpr>(Right)->getMemberDecl();
103
104 case Stmt::CStyleCastExprClass:
105 return cast<CStyleCastExpr>(Left)->getTypeAsWritten() ==
106 cast<CStyleCastExpr>(Right)->getTypeAsWritten();
107
108 case Stmt::CallExprClass:
109 case Stmt::ImplicitCastExprClass:
110 case Stmt::ArraySubscriptExprClass:
111 return true;
112
113 case Stmt::UnaryOperatorClass:
114 if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
115 return false;
116 return cast<UnaryOperator>(Left)->getOpcode() ==
117 cast<UnaryOperator>(Right)->getOpcode();
118 case Stmt::BinaryOperatorClass:
119 return cast<BinaryOperator>(Left)->getOpcode() ==
120 cast<BinaryOperator>(Right)->getOpcode();
121 }
122}
123
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000124// For a given expression 'x', returns whether the ranges covered by the
125// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
126static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
127 const APSInt &ValueLHS,
128 BinaryOperatorKind OpcodeRHS,
129 const APSInt &ValueRHS) {
130 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
131 "Values must be ordered");
132 // Handle the case where constants are the same: x <= 4 <==> x <= 4.
133 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
134 return OpcodeLHS == OpcodeRHS;
135
136 // Handle the case where constants are off by one: x <= 4 <==> x < 5.
137 APSInt ValueLHS_plus1;
138 return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
139 (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
140 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
141 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0;
142}
143
144// For a given expression 'x', returns whether the ranges covered by the
145// relational operators are fully disjoint (i.e. x < 4 and x > 7).
146static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
147 const APSInt &ValueLHS,
148 BinaryOperatorKind OpcodeRHS,
149 const APSInt &ValueRHS) {
150 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
151 "Values must be ordered");
152
153 // Handle cases where the constants are the same.
154 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
155 switch (OpcodeLHS) {
156 case BO_EQ:
157 return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
158 case BO_NE:
159 return OpcodeRHS == BO_EQ;
160 case BO_LE:
161 return OpcodeRHS == BO_GT;
162 case BO_GE:
163 return OpcodeRHS == BO_LT;
164 case BO_LT:
165 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
166 case BO_GT:
167 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
168 default:
169 return false;
170 }
171 }
172
173 // Handle cases where the constants are different.
174 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LE || OpcodeLHS == BO_LE) &&
175 (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
176 return true;
177
178 // Handle the case where constants are off by one: x > 5 && x < 6.
179 APSInt ValueLHS_plus1;
180 if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
181 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
182 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
183 return true;
184
185 return false;
186}
187
188// Returns whether the ranges covered by the union of both relational
189// expressions covers the whole domain (i.e. x < 10 and x > 0).
190static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
191 const APSInt &ValueLHS,
192 BinaryOperatorKind OpcodeRHS,
193 const APSInt &ValueRHS) {
194 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
195 "Values must be ordered");
196
197 // Handle cases where the constants are the same: x < 5 || x >= 5.
198 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
199 switch (OpcodeLHS) {
200 case BO_EQ:
201 return OpcodeRHS == BO_NE;
202 case BO_NE:
203 return OpcodeRHS == BO_EQ;
204 case BO_LE:
205 return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
206 case BO_LT:
207 return OpcodeRHS == BO_GE;
208 case BO_GE:
209 return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
210 case BO_GT:
211 return OpcodeRHS == BO_LE;
212 default:
213 return false;
214 }
215 }
216
217 // Handle the case where constants are off by one: x <= 4 || x >= 5.
218 APSInt ValueLHS_plus1;
219 if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
220 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
221 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
222 return true;
223
224 // Handle cases where the constants are different: x > 4 || x <= 7.
225 if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
226 (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
227 return true;
228
229 return false;
230}
231
232static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
233 const APSInt &ValueLHS,
234 BinaryOperatorKind OpcodeRHS,
235 const APSInt &ValueRHS) {
236 int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
237 switch (OpcodeLHS) {
238 case BO_EQ:
239 return OpcodeRHS == BO_EQ && Comparison == 0;
240 case BO_NE:
241 return (OpcodeRHS == BO_NE && Comparison == 0) ||
242 (OpcodeRHS == BO_EQ && Comparison != 0) ||
243 (OpcodeRHS == BO_LT && Comparison >= 0) ||
244 (OpcodeRHS == BO_LE && Comparison > 0) ||
245 (OpcodeRHS == BO_GT && Comparison <= 0) ||
246 (OpcodeRHS == BO_GE && Comparison < 0);
247
248 case BO_LT:
249 return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
250 (OpcodeRHS == BO_LE && Comparison > 0) ||
251 (OpcodeRHS == BO_EQ && Comparison > 0));
252 case BO_GT:
253 return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
254 (OpcodeRHS == BO_GE && Comparison < 0) ||
255 (OpcodeRHS == BO_EQ && Comparison < 0));
256 case BO_LE:
257 return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
258 Comparison >= 0;
259 case BO_GE:
260 return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
261 Comparison <= 0;
262 default:
263 return false;
264 }
265}
266
267static void canonicalNegateExpr(BinaryOperatorKind &Opcode, APSInt &Value) {
268 if (Opcode == BO_Sub) {
269 Opcode = BO_Add;
270 Value = -Value;
271 }
272}
273
274AST_MATCHER(Expr, isIntegerConstantExpr) {
275 if (Node.isInstantiationDependent())
276 return false;
277 return Node.isIntegerConstantExpr(Finder->getASTContext());
278}
279
280// Returns a matcher for integer constant expression.
281static ast_matchers::internal::Matcher<Expr>
282matchIntegerConstantExpr(StringRef Id) {
283 std::string CstId = (Id + "-const").str();
284 return expr(isIntegerConstantExpr()).bind(CstId);
285}
286
287// Retrieve the integer value matched by 'matchIntegerConstantExpr' with name
288// 'Id' and store it into 'Value'.
289static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
290 StringRef Id, APSInt &Value) {
291 std::string CstId = (Id + "-const").str();
292 const auto *CstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
293 return CstExpr && CstExpr->isIntegerConstantExpr(Value, *Result.Context);
294}
295
296// Returns a matcher for a symbolic expression (any expression except ingeter
297// constant expression).
298static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
299 std::string SymId = (Id + "-sym").str();
300 return ignoringParenImpCasts(
301 expr(unless(isIntegerConstantExpr())).bind(SymId));
302}
303
304// Retrieve the expression matched by 'matchSymbolicExpr' with name 'Id' and
305// store it into 'SymExpr'.
306static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
307 StringRef Id, const Expr *&SymExpr) {
308 std::string SymId = (Id + "-sym").str();
309 if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
310 SymExpr = Node;
311 return true;
312 }
313 return false;
314}
315
316// Match a binary operator between a symbolic expression and an integer constant
317// expression.
318static ast_matchers::internal::Matcher<Expr>
319matchBinOpIntegerConstantExpr(StringRef Id) {
320 const auto BinOpCstExpr =
321 expr(
322 anyOf(binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("|"),
323 hasOperatorName("&")),
324 hasEitherOperand(matchSymbolicExpr(Id)),
325 hasEitherOperand(matchIntegerConstantExpr(Id))),
326 binaryOperator(hasOperatorName("-"),
327 hasLHS(matchSymbolicExpr(Id)),
328 hasRHS(matchIntegerConstantExpr(Id)))))
329 .bind(Id);
330 return ignoringParenImpCasts(BinOpCstExpr);
331}
332
333// Retrieve sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
334// name 'Id'.
335static bool
336retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
337 StringRef Id, BinaryOperatorKind &Opcode,
338 const Expr *&Symbol, APSInt &Value) {
339 if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
340 Opcode = BinExpr->getOpcode();
341 return retrieveSymbolicExpr(Result, Id, Symbol) &&
342 retrieveIntegerConstantExpr(Result, Id, Value);
343 }
344 return false;
345}
346
347// Matches relational expression: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
348static ast_matchers::internal::Matcher<Expr>
349matchRelationalIntegerConstantExpr(StringRef Id) {
350 std::string CastId = (Id + "-cast").str();
351 std::string SwapId = (Id + "-swap").str();
352 std::string NegateId = (Id + "-negate").str();
353
354 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
355 isComparisonOperator(), expr().bind(Id),
356 anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
357 hasRHS(matchIntegerConstantExpr(Id))),
358 allOf(hasLHS(matchIntegerConstantExpr(Id)),
359 hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
360
361 // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
362 // to if (x != 0)).
363 const auto CastExpr =
364 implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
365 hasSourceExpression(matchSymbolicExpr(Id)))
366 .bind(CastId);
367
368 const auto NegateRelationalExpr =
369 unaryOperator(hasOperatorName("!"),
370 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
371 .bind(NegateId);
372
373 const auto NegateNegateRelationalExpr =
374 unaryOperator(hasOperatorName("!"),
375 hasUnaryOperand(unaryOperator(
376 hasOperatorName("!"),
377 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
378
379 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
380 NegateNegateRelationalExpr);
381}
382
383// Retrieve sub-expressions matched by 'matchRelationalIntegerConstantExpr' with
384// name 'Id'.
385static bool
386retrieveRelationalIntegerConstantExpr(const MatchFinder::MatchResult &Result,
387 StringRef Id, const Expr *&OperandExpr,
388 BinaryOperatorKind &Opcode,
389 const Expr *&Symbol, APSInt &Value) {
390 std::string CastId = (Id + "-cast").str();
391 std::string SwapId = (Id + "-swap").str();
392 std::string NegateId = (Id + "-negate").str();
393
394 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
395 // Operand received with explicit comparator.
396 Opcode = Bin->getOpcode();
397 OperandExpr = Bin;
398 if (!retrieveIntegerConstantExpr(Result, Id, Value))
399 return false;
400 } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
401 // Operand received with implicit comparator (cast).
402 Opcode = BO_NE;
403 OperandExpr = Cast;
404 Value = APSInt(32, 0);
405 } else {
406 return false;
407 }
408
409 if (!retrieveSymbolicExpr(Result, Id, Symbol))
410 return false;
411
412 if (Result.Nodes.getNodeAs<Expr>(SwapId))
413 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
414 if (Result.Nodes.getNodeAs<Expr>(NegateId))
415 Opcode = BinaryOperator::negateComparisonOp(Opcode);
416
417 return true;
418}
419
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000420AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
421 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000422}
423
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000424AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
425 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
426}
427
428AST_MATCHER(CallExpr, parametersAreEquivalent) {
429 return Node.getNumArgs() == 2 &&
430 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
431}
432
433AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000434 return Node.getOperatorLoc().isMacroID();
435}
436
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000437AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
438 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
439}
440
441AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
442
443AST_MATCHER_P(Expr, expandedByMacro, std::set<std::string>, Names) {
444 const SourceManager &SM = Finder->getASTContext().getSourceManager();
445 const LangOptions &LO = Finder->getASTContext().getLangOpts();
446 SourceLocation Loc = Node.getExprLoc();
447 while (Loc.isMacroID()) {
448 std::string MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
449 if (Names.find(MacroName) != Names.end())
450 return true;
451 Loc = SM.getImmediateMacroCallerLoc(Loc);
452 }
453 return false;
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000454}
455
456void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
457 const auto AnyLiteralExpr = ignoringParenImpCasts(
458 anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
459
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000460 std::vector<std::string> MacroNames =
461 utils::options::parseStringList(KnownBannedMacroNames);
462 std::set<std::string> Names(MacroNames.begin(), MacroNames.end());
463
464 const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(Names));
465
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000466 Finder->addMatcher(
467 binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
468 hasOperatorName("%"), hasOperatorName("|"),
469 hasOperatorName("&"), hasOperatorName("^"),
470 matchers::isComparisonOperator(),
471 hasOperatorName("&&"), hasOperatorName("||"),
472 hasOperatorName("=")),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000473 operandsAreEquivalent(),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000474 // Filter noisy false positives.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000475 unless(isInTemplateInstantiation()),
476 unless(binaryOperatorIsInMacro()),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000477 unless(hasType(realFloatingPointType())),
478 unless(hasEitherOperand(hasType(realFloatingPointType()))),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000479 unless(hasLHS(AnyLiteralExpr)),
480 unless(hasDescendant(BannedIntegerLiteral)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000481 .bind("binary"),
482 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000483
484 Finder->addMatcher(
485 conditionalOperator(expressionsAreEquivalent(),
486 // Filter noisy false positives.
487 unless(conditionalOperatorIsInMacro()),
488 unless(hasTrueExpression(AnyLiteralExpr)),
489 unless(isInTemplateInstantiation()))
490 .bind("cond"),
491 this);
492
493 Finder->addMatcher(
494 cxxOperatorCallExpr(
495 anyOf(
496 hasOverloadedOperatorName("-"), hasOverloadedOperatorName("/"),
497 hasOverloadedOperatorName("%"), hasOverloadedOperatorName("|"),
498 hasOverloadedOperatorName("&"), hasOverloadedOperatorName("^"),
499 hasOverloadedOperatorName("=="), hasOverloadedOperatorName("!="),
500 hasOverloadedOperatorName("<"), hasOverloadedOperatorName("<="),
501 hasOverloadedOperatorName(">"), hasOverloadedOperatorName(">="),
502 hasOverloadedOperatorName("&&"), hasOverloadedOperatorName("||"),
503 hasOverloadedOperatorName("=")),
504 parametersAreEquivalent(),
505 // Filter noisy false positives.
506 unless(isMacro()), unless(isInTemplateInstantiation()))
507 .bind("call"),
508 this);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000509
510 // Match common expressions and apply more checks to find redundant
511 // sub-expressions.
512 // a) Expr <op> K1 == K2
513 // b) Expr <op> K1 == Expr
514 // c) Expr <op> K1 == Expr <op> K2
515 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
516 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
517 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
518 const auto CstRight = matchIntegerConstantExpr("rhs");
519 const auto SymRight = matchSymbolicExpr("rhs");
520
521 // Match expressions like: x <op> 0xFF == 0xF00.
522 Finder->addMatcher(binaryOperator(isComparisonOperator(),
523 hasEitherOperand(BinOpCstLeft),
524 hasEitherOperand(CstRight))
525 .bind("binop-const-compare-to-const"),
526 this);
527
528 // Match expressions like: x <op> 0xFF == x.
529 Finder->addMatcher(
530 binaryOperator(isComparisonOperator(),
531 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
532 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
533 .bind("binop-const-compare-to-sym"),
534 this);
535
536 // Match expressions like: x <op> 10 == x <op> 12.
537 Finder->addMatcher(binaryOperator(isComparisonOperator(),
538 hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
539 // Already reported as redundant.
540 unless(operandsAreEquivalent()))
541 .bind("binop-const-compare-to-binop-const"),
542 this);
543
544 // Match relational expressions combined with logical operators and find
545 // redundant sub-expressions.
546 // see: 'checkRelationalExpr'
547
548 // Match expressions like: x < 2 && x > 2.
549 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
550 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
551 Finder->addMatcher(
552 binaryOperator(anyOf(hasOperatorName("||"), hasOperatorName("&&")),
553 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
554 // Already reported as redundant.
555 unless(operandsAreEquivalent()))
556 .bind("comparisons-of-symbol-and-const"),
557 this);
558}
559
560void RedundantExpressionCheck::checkArithmeticExpr(
561 const MatchFinder::MatchResult &Result) {
562 APSInt LhsValue, RhsValue;
563 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
564 BinaryOperatorKind LhsOpcode, RhsOpcode;
565
566 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
567 "binop-const-compare-to-sym")) {
568 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
569 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
570 LhsValue) ||
571 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
572 !areEquivalentExpr(LhsSymbol, RhsSymbol))
573 return;
574
575 // Check expressions: x + k == x or x - k == x.
576 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
577 if ((LhsValue != 0 && Opcode == BO_EQ) ||
578 (LhsValue == 0 && Opcode == BO_NE))
579 diag(ComparisonOperator->getOperatorLoc(),
580 "logical expression is always false");
581 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
582 (LhsValue != 0 && Opcode == BO_NE))
583 diag(ComparisonOperator->getOperatorLoc(),
584 "logical expression is always true");
585 }
586 } else if (const auto *ComparisonOperator =
587 Result.Nodes.getNodeAs<BinaryOperator>(
588 "binop-const-compare-to-binop-const")) {
589 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
590
591 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
592 LhsValue) ||
593 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
594 RhsValue) ||
595 !areEquivalentExpr(LhsSymbol, RhsSymbol))
596 return;
597
598 canonicalNegateExpr(LhsOpcode, LhsValue);
599 canonicalNegateExpr(RhsOpcode, RhsValue);
600
601 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
602 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
603 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
604 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
605 diag(ComparisonOperator->getOperatorLoc(),
606 "logical expression is always true");
607 } else if ((Opcode == BO_EQ &&
608 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
609 (Opcode == BO_NE &&
610 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
611 diag(ComparisonOperator->getOperatorLoc(),
612 "logical expression is always false");
613 }
614 }
615 }
616}
617
618void RedundantExpressionCheck::checkBitwiseExpr(
619 const MatchFinder::MatchResult &Result) {
620 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
621 "binop-const-compare-to-const")) {
622 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
623
624 APSInt LhsValue, RhsValue;
625 const Expr *LhsSymbol = nullptr;
626 BinaryOperatorKind LhsOpcode;
627 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
628 LhsValue) ||
629 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
630 return;
631
632 uint64_t LhsConstant = LhsValue.getZExtValue();
633 uint64_t RhsConstant = RhsValue.getZExtValue();
634 SourceLocation Loc = ComparisonOperator->getOperatorLoc();
635
636 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
637 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
638 if (Opcode == BO_EQ)
639 diag(Loc, "logical expression is always false");
640 else if (Opcode == BO_NE)
641 diag(Loc, "logical expression is always true");
642 }
643
644 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
645 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
646 if (Opcode == BO_EQ)
647 diag(Loc, "logical expression is always false");
648 else if (Opcode == BO_NE)
649 diag(Loc, "logical expression is always true");
650 }
651 }
652}
653
654void RedundantExpressionCheck::checkRelationalExpr(
655 const MatchFinder::MatchResult &Result) {
656 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
657 "comparisons-of-symbol-and-const")) {
658 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
659 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
660
661 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
662 APSInt LhsValue, RhsValue;
663 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
664 BinaryOperatorKind LhsOpcode, RhsOpcode;
665 if (!retrieveRelationalIntegerConstantExpr(
666 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue) ||
667 !retrieveRelationalIntegerConstantExpr(
668 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue) ||
669 !areEquivalentExpr(LhsSymbol, RhsSymbol))
670 return;
671
672 // Bring to a canonical form: smallest constant must be on the left side.
673 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
674 std::swap(LhsExpr, RhsExpr);
675 std::swap(LhsValue, RhsValue);
676 std::swap(LhsSymbol, RhsSymbol);
677 std::swap(LhsOpcode, RhsOpcode);
678 }
679
680 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
681 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
682 diag(ComparisonOperator->getOperatorLoc(),
683 "equivalent expression on both side of logical operator");
684 return;
685 }
686
687 if (Opcode == BO_LAnd) {
688 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
689 diag(ComparisonOperator->getOperatorLoc(),
690 "logical expression is always false");
691 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
692 diag(LhsExpr->getExprLoc(), "expression is redundant");
693 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
694 diag(RhsExpr->getExprLoc(), "expression is redundant");
695 }
696 }
697
698 if (Opcode == BO_LOr) {
699 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
700 diag(ComparisonOperator->getOperatorLoc(),
701 "logical expression is always true");
702 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
703 diag(RhsExpr->getExprLoc(), "expression is redundant");
704 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
705 diag(LhsExpr->getExprLoc(), "expression is redundant");
706 }
707 }
708 }
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000709}
710
711void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
712 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary"))
713 diag(BinOp->getOperatorLoc(), "both side of operator are equivalent");
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000714 if (const auto *CondOp = Result.Nodes.getNodeAs<ConditionalOperator>("cond"))
715 diag(CondOp->getColonLoc(), "'true' and 'false' expression are equivalent");
716 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call"))
717 diag(Call->getOperatorLoc(),
718 "both side of overloaded operator are equivalent");
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000719
720 checkArithmeticExpr(Result);
721 checkBitwiseExpr(Result);
722 checkRelationalExpr(Result);
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000723}
724
725} // namespace misc
726} // namespace tidy
727} // namespace clang