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