blob: ea3c33a9270741e8e43273a5f29c4b1fc68bb56f [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
Benjamin Kramer492f1cc2018-02-02 13:23:24 +000040static constexpr llvm::StringLiteral KnownBannedMacroNames[] = {
41 "EAGAIN",
42 "EWOULDBLOCK",
43 "SIGCLD",
44 "SIGCHLD",
45};
Etienne Bergeronc87599f2016-05-12 04:32:47 +000046
Etienne Bergeron00639bc2016-07-07 04:03:05 +000047static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
48 Result = Value;
49 ++Result;
50 return Value < Result;
51}
52
Etienne Bergeronc87599f2016-05-12 04:32:47 +000053static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
54 const NestedNameSpecifier *Right) {
55 llvm::FoldingSetNodeID LeftID, RightID;
56 Left->Profile(LeftID);
57 Right->Profile(RightID);
58 return LeftID == RightID;
59}
60
61static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
Etienne Bergeronbda187d2016-04-26 17:30:30 +000062 if (!Left || !Right)
63 return !Left && !Right;
64
65 Left = Left->IgnoreParens();
66 Right = Right->IgnoreParens();
67
68 // Compare classes.
69 if (Left->getStmtClass() != Right->getStmtClass())
70 return false;
71
72 // Compare children.
73 Expr::const_child_iterator LeftIter = Left->child_begin();
74 Expr::const_child_iterator RightIter = Right->child_begin();
75 while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
Etienne Bergeronc87599f2016-05-12 04:32:47 +000076 if (!areEquivalentExpr(dyn_cast<Expr>(*LeftIter),
77 dyn_cast<Expr>(*RightIter)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +000078 return false;
79 ++LeftIter;
80 ++RightIter;
81 }
82 if (LeftIter != Left->child_end() || RightIter != Right->child_end())
83 return false;
84
85 // Perform extra checks.
86 switch (Left->getStmtClass()) {
87 default:
88 return false;
89
90 case Stmt::CharacterLiteralClass:
91 return cast<CharacterLiteral>(Left)->getValue() ==
92 cast<CharacterLiteral>(Right)->getValue();
93 case Stmt::IntegerLiteralClass: {
94 llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
95 llvm::APInt RightLit = cast<IntegerLiteral>(Right)->getValue();
Etienne Bergeronc87599f2016-05-12 04:32:47 +000096 return LeftLit.getBitWidth() == RightLit.getBitWidth() &&
97 LeftLit == RightLit;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000098 }
99 case Stmt::FloatingLiteralClass:
100 return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
101 cast<FloatingLiteral>(Right)->getValue());
102 case Stmt::StringLiteralClass:
103 return cast<StringLiteral>(Left)->getBytes() ==
104 cast<StringLiteral>(Right)->getBytes();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000105 case Stmt::CXXOperatorCallExprClass:
106 return cast<CXXOperatorCallExpr>(Left)->getOperator() ==
107 cast<CXXOperatorCallExpr>(Right)->getOperator();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000108 case Stmt::DependentScopeDeclRefExprClass:
109 if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
110 cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
111 return false;
112 return areEquivalentNameSpecifier(
113 cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
114 cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000115 case Stmt::DeclRefExprClass:
116 return cast<DeclRefExpr>(Left)->getDecl() ==
117 cast<DeclRefExpr>(Right)->getDecl();
118 case Stmt::MemberExprClass:
119 return cast<MemberExpr>(Left)->getMemberDecl() ==
120 cast<MemberExpr>(Right)->getMemberDecl();
Gabor Horvathec87e172017-11-07 13:17:58 +0000121 case Stmt::CXXFunctionalCastExprClass:
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000122 case Stmt::CStyleCastExprClass:
Gabor Horvathec87e172017-11-07 13:17:58 +0000123 return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
124 cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000125 case Stmt::CallExprClass:
126 case Stmt::ImplicitCastExprClass:
127 case Stmt::ArraySubscriptExprClass:
128 return true;
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000129 case Stmt::UnaryOperatorClass:
130 if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
131 return false;
132 return cast<UnaryOperator>(Left)->getOpcode() ==
133 cast<UnaryOperator>(Right)->getOpcode();
134 case Stmt::BinaryOperatorClass:
135 return cast<BinaryOperator>(Left)->getOpcode() ==
136 cast<BinaryOperator>(Right)->getOpcode();
137 }
138}
139
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000140// For a given expression 'x', returns whether the ranges covered by the
141// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
142static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
143 const APSInt &ValueLHS,
144 BinaryOperatorKind OpcodeRHS,
145 const APSInt &ValueRHS) {
146 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
147 "Values must be ordered");
148 // Handle the case where constants are the same: x <= 4 <==> x <= 4.
149 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
150 return OpcodeLHS == OpcodeRHS;
151
152 // Handle the case where constants are off by one: x <= 4 <==> x < 5.
153 APSInt ValueLHS_plus1;
154 return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
155 (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
156 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
157 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0;
158}
159
160// For a given expression 'x', returns whether the ranges covered by the
161// relational operators are fully disjoint (i.e. x < 4 and x > 7).
162static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
163 const APSInt &ValueLHS,
164 BinaryOperatorKind OpcodeRHS,
165 const APSInt &ValueRHS) {
166 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
167 "Values must be ordered");
168
169 // Handle cases where the constants are the same.
170 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
171 switch (OpcodeLHS) {
172 case BO_EQ:
173 return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
174 case BO_NE:
175 return OpcodeRHS == BO_EQ;
176 case BO_LE:
177 return OpcodeRHS == BO_GT;
178 case BO_GE:
179 return OpcodeRHS == BO_LT;
180 case BO_LT:
181 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
182 case BO_GT:
183 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
184 default:
185 return false;
186 }
187 }
188
189 // Handle cases where the constants are different.
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000190 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LT || OpcodeLHS == BO_LE) &&
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000191 (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
192 return true;
193
194 // Handle the case where constants are off by one: x > 5 && x < 6.
195 APSInt ValueLHS_plus1;
196 if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
197 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
198 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
199 return true;
200
201 return false;
202}
203
204// Returns whether the ranges covered by the union of both relational
Gabor Horvath91c66712017-12-20 12:22:16 +0000205// expressions cover the whole domain (i.e. x < 10 and x > 0).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000206static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
207 const APSInt &ValueLHS,
208 BinaryOperatorKind OpcodeRHS,
209 const APSInt &ValueRHS) {
210 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
211 "Values must be ordered");
212
213 // Handle cases where the constants are the same: x < 5 || x >= 5.
214 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
215 switch (OpcodeLHS) {
216 case BO_EQ:
217 return OpcodeRHS == BO_NE;
218 case BO_NE:
219 return OpcodeRHS == BO_EQ;
220 case BO_LE:
221 return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
222 case BO_LT:
223 return OpcodeRHS == BO_GE;
224 case BO_GE:
225 return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
226 case BO_GT:
227 return OpcodeRHS == BO_LE;
228 default:
229 return false;
230 }
231 }
232
233 // Handle the case where constants are off by one: x <= 4 || x >= 5.
234 APSInt ValueLHS_plus1;
235 if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
236 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
237 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
238 return true;
239
240 // Handle cases where the constants are different: x > 4 || x <= 7.
241 if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
242 (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
243 return true;
244
Alexander Kornienkoc3acd2e2017-03-23 15:13:54 +0000245 // Handle cases where constants are different but both ops are !=, like:
246 // x != 5 || x != 10
247 if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE)
248 return true;
249
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000250 return false;
251}
252
253static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
254 const APSInt &ValueLHS,
255 BinaryOperatorKind OpcodeRHS,
256 const APSInt &ValueRHS) {
257 int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
258 switch (OpcodeLHS) {
259 case BO_EQ:
260 return OpcodeRHS == BO_EQ && Comparison == 0;
261 case BO_NE:
262 return (OpcodeRHS == BO_NE && Comparison == 0) ||
263 (OpcodeRHS == BO_EQ && Comparison != 0) ||
264 (OpcodeRHS == BO_LT && Comparison >= 0) ||
265 (OpcodeRHS == BO_LE && Comparison > 0) ||
266 (OpcodeRHS == BO_GT && Comparison <= 0) ||
267 (OpcodeRHS == BO_GE && Comparison < 0);
268
269 case BO_LT:
270 return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
271 (OpcodeRHS == BO_LE && Comparison > 0) ||
272 (OpcodeRHS == BO_EQ && Comparison > 0));
273 case BO_GT:
274 return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
275 (OpcodeRHS == BO_GE && Comparison < 0) ||
276 (OpcodeRHS == BO_EQ && Comparison < 0));
277 case BO_LE:
278 return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
279 Comparison >= 0;
280 case BO_GE:
281 return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
282 Comparison <= 0;
283 default:
284 return false;
285 }
286}
287
Gabor Horvathec87e172017-11-07 13:17:58 +0000288static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
289 APSInt &Value) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000290 if (Opcode == BO_Sub) {
291 Opcode = BO_Add;
292 Value = -Value;
293 }
294}
295
296AST_MATCHER(Expr, isIntegerConstantExpr) {
297 if (Node.isInstantiationDependent())
298 return false;
299 return Node.isIntegerConstantExpr(Finder->getASTContext());
300}
301
Gabor Horvathec87e172017-11-07 13:17:58 +0000302AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
303 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
304}
305
306AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
307 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
308}
309
310AST_MATCHER(CallExpr, parametersAreEquivalent) {
311 return Node.getNumArgs() == 2 &&
312 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
313}
314
315AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
316 return Node.getOperatorLoc().isMacroID();
317}
318
319AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
320 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
321}
322
323AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
324
Benjamin Kramer492f1cc2018-02-02 13:23:24 +0000325AST_MATCHER_P(Expr, expandedByMacro, ArrayRef<llvm::StringLiteral>, Names) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000326 const SourceManager &SM = Finder->getASTContext().getSourceManager();
327 const LangOptions &LO = Finder->getASTContext().getLangOpts();
328 SourceLocation Loc = Node.getExprLoc();
329 while (Loc.isMacroID()) {
330 StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
Benjamin Kramer492f1cc2018-02-02 13:23:24 +0000331 if (llvm::is_contained(Names, MacroName))
Gabor Horvathec87e172017-11-07 13:17:58 +0000332 return true;
333 Loc = SM.getImmediateMacroCallerLoc(Loc);
334 }
335 return false;
336}
337
338// Returns a matcher for integer constant expressions.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000339static ast_matchers::internal::Matcher<Expr>
340matchIntegerConstantExpr(StringRef Id) {
341 std::string CstId = (Id + "-const").str();
342 return expr(isIntegerConstantExpr()).bind(CstId);
343}
344
Gabor Horvathec87e172017-11-07 13:17:58 +0000345// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
346// name 'Id' and stores it into 'ConstExpr', the value of the expression is
347// stored into `Value`.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000348static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
Gabor Horvathec87e172017-11-07 13:17:58 +0000349 StringRef Id, APSInt &Value,
350 const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000351 std::string CstId = (Id + "-const").str();
Gabor Horvathec87e172017-11-07 13:17:58 +0000352 ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
353 return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000354}
355
Gabor Horvathec87e172017-11-07 13:17:58 +0000356// Overloaded `retrieveIntegerConstantExpr` for compatibility.
357static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
358 StringRef Id, APSInt &Value) {
359 const Expr *ConstExpr = nullptr;
360 return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
361}
362
363// Returns a matcher for symbolic expressions (matches every expression except
364// ingeter constant expressions).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000365static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
366 std::string SymId = (Id + "-sym").str();
367 return ignoringParenImpCasts(
368 expr(unless(isIntegerConstantExpr())).bind(SymId));
369}
370
Gabor Horvathec87e172017-11-07 13:17:58 +0000371// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
372// stores it into 'SymExpr'.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000373static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
374 StringRef Id, const Expr *&SymExpr) {
375 std::string SymId = (Id + "-sym").str();
376 if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
377 SymExpr = Node;
378 return true;
379 }
380 return false;
381}
382
383// Match a binary operator between a symbolic expression and an integer constant
384// expression.
385static ast_matchers::internal::Matcher<Expr>
386matchBinOpIntegerConstantExpr(StringRef Id) {
387 const auto BinOpCstExpr =
388 expr(
389 anyOf(binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("|"),
390 hasOperatorName("&")),
391 hasEitherOperand(matchSymbolicExpr(Id)),
392 hasEitherOperand(matchIntegerConstantExpr(Id))),
393 binaryOperator(hasOperatorName("-"),
394 hasLHS(matchSymbolicExpr(Id)),
395 hasRHS(matchIntegerConstantExpr(Id)))))
396 .bind(Id);
397 return ignoringParenImpCasts(BinOpCstExpr);
398}
399
Gabor Horvathec87e172017-11-07 13:17:58 +0000400// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000401// name 'Id'.
402static bool
403retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
404 StringRef Id, BinaryOperatorKind &Opcode,
405 const Expr *&Symbol, APSInt &Value) {
406 if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
407 Opcode = BinExpr->getOpcode();
408 return retrieveSymbolicExpr(Result, Id, Symbol) &&
409 retrieveIntegerConstantExpr(Result, Id, Value);
410 }
411 return false;
412}
413
Gabor Horvathec87e172017-11-07 13:17:58 +0000414// Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000415static ast_matchers::internal::Matcher<Expr>
416matchRelationalIntegerConstantExpr(StringRef Id) {
417 std::string CastId = (Id + "-cast").str();
418 std::string SwapId = (Id + "-swap").str();
419 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000420 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000421
422 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
423 isComparisonOperator(), expr().bind(Id),
424 anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
425 hasRHS(matchIntegerConstantExpr(Id))),
426 allOf(hasLHS(matchIntegerConstantExpr(Id)),
427 hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
428
429 // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
430 // to if (x != 0)).
431 const auto CastExpr =
432 implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
433 hasSourceExpression(matchSymbolicExpr(Id)))
434 .bind(CastId);
435
436 const auto NegateRelationalExpr =
437 unaryOperator(hasOperatorName("!"),
438 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
439 .bind(NegateId);
440
Gabor Horvathec87e172017-11-07 13:17:58 +0000441 // Do not bind to double negation.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000442 const auto NegateNegateRelationalExpr =
443 unaryOperator(hasOperatorName("!"),
444 hasUnaryOperand(unaryOperator(
445 hasOperatorName("!"),
446 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
447
Gabor Horvath250c40d2017-11-27 15:05:24 +0000448 const auto OverloadedOperatorExpr =
449 cxxOperatorCallExpr(
450 anyOf(hasOverloadedOperatorName("=="),
451 hasOverloadedOperatorName("!="), hasOverloadedOperatorName("<"),
452 hasOverloadedOperatorName("<="), hasOverloadedOperatorName(">"),
453 hasOverloadedOperatorName(">=")),
454 // Filter noisy false positives.
455 unless(isMacro()), unless(isInTemplateInstantiation()))
456 .bind(OverloadId);
457
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000458 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
Gabor Horvath250c40d2017-11-27 15:05:24 +0000459 NegateNegateRelationalExpr, OverloadedOperatorExpr);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000460}
461
Gabor Horvath250c40d2017-11-27 15:05:24 +0000462// Checks whether a function param is non constant reference type, and may
463// be modified in the function.
464static bool isNonConstReferenceType(QualType ParamType) {
465 return ParamType->isReferenceType() &&
466 !ParamType.getNonReferenceType().isConstQualified();
467}
468
469// Checks whether the arguments of an overloaded operator can be modified in the
470// function.
471// For operators that take an instance and a constant as arguments, only the
472// first argument (the instance) needs to be checked, since the constant itself
473// is a temporary expression. Whether the second parameter is checked is
474// controlled by the parameter `ParamsToCheckCount`.
475static bool
476canOverloadedOperatorArgsBeModified(const FunctionDecl *OperatorDecl,
477 bool checkSecondParam) {
478 unsigned ParamCount = OperatorDecl->getNumParams();
479
480 // Overloaded operators declared inside a class have only one param.
481 // These functions must be declared const in order to not be able to modify
482 // the instance of the class they are called through.
483 if (ParamCount == 1 &&
484 !OperatorDecl->getType()->getAs<FunctionType>()->isConst())
485 return true;
486
487 if (isNonConstReferenceType(OperatorDecl->getParamDecl(0)->getType()))
488 return true;
489
490 return checkSecondParam && ParamCount == 2 &&
491 isNonConstReferenceType(OperatorDecl->getParamDecl(1)->getType());
492}
493
494// Retrieves sub-expressions matched by 'matchRelationalIntegerConstantExpr'
495// with name 'Id'.
Gabor Horvathec87e172017-11-07 13:17:58 +0000496static bool retrieveRelationalIntegerConstantExpr(
497 const MatchFinder::MatchResult &Result, StringRef Id,
498 const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
499 APSInt &Value, const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000500 std::string CastId = (Id + "-cast").str();
501 std::string SwapId = (Id + "-swap").str();
502 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000503 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000504
505 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
506 // Operand received with explicit comparator.
507 Opcode = Bin->getOpcode();
508 OperandExpr = Bin;
Gabor Horvathec87e172017-11-07 13:17:58 +0000509
510 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000511 return false;
512 } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
513 // Operand received with implicit comparator (cast).
514 Opcode = BO_NE;
515 OperandExpr = Cast;
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000516 Value = APSInt(32, false);
Gabor Horvath250c40d2017-11-27 15:05:24 +0000517 } else if (const auto *OverloadedOperatorExpr =
518 Result.Nodes.getNodeAs<CXXOperatorCallExpr>(OverloadId)) {
519 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
520 if (!OverloadedFunctionDecl)
521 return false;
522
523 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
524 return false;
525
Gabor Horvath91c66712017-12-20 12:22:16 +0000526 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
527 return false;
528
Gabor Horvath250c40d2017-11-27 15:05:24 +0000529 if (!OverloadedOperatorExpr->getArg(1)->isIntegerConstantExpr(
530 Value, *Result.Context))
531 return false;
532
533 Symbol = OverloadedOperatorExpr->getArg(0);
534 OperandExpr = OverloadedOperatorExpr;
535 Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
536
537 return BinaryOperator::isComparisonOp(Opcode);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000538 } else {
539 return false;
540 }
541
542 if (!retrieveSymbolicExpr(Result, Id, Symbol))
543 return false;
544
545 if (Result.Nodes.getNodeAs<Expr>(SwapId))
546 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
547 if (Result.Nodes.getNodeAs<Expr>(NegateId))
548 Opcode = BinaryOperator::negateComparisonOp(Opcode);
Gabor Horvathec87e172017-11-07 13:17:58 +0000549 return true;
550}
551
552// Checks for expressions like (X == 4) && (Y != 9)
553static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
554 const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
555 const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
556
557 if (!LhsBinOp || !RhsBinOp)
558 return false;
559
560 if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
561 LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
562 (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
563 RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
564 return true;
565 return false;
566}
567
568// Retrieves integer constant subexpressions from binary operator expressions
Gabor Horvath91c66712017-12-20 12:22:16 +0000569// that have two equivalent sides.
Gabor Horvathec87e172017-11-07 13:17:58 +0000570// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
571static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
572 BinaryOperatorKind &MainOpcode,
573 BinaryOperatorKind &SideOpcode,
574 const Expr *&LhsConst,
575 const Expr *&RhsConst,
576 const ASTContext *AstCtx) {
577 assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
578 "Both sides of binary operator must be constant expressions!");
579
580 MainOpcode = BinOp->getOpcode();
581
582 const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
583 const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
584
585 LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
586 ? BinOpLhs->getLHS()
587 : BinOpLhs->getRHS();
588 RhsConst = BinOpRhs->getLHS()->isIntegerConstantExpr(*AstCtx)
589 ? BinOpRhs->getLHS()
590 : BinOpRhs->getRHS();
591
592 if (!LhsConst || !RhsConst)
593 return false;
594
595 assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
596 "Sides of the binary operator must be equivalent expressions!");
597
598 SideOpcode = BinOpLhs->getOpcode();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000599
600 return true;
601}
602
Gabor Horvathec87e172017-11-07 13:17:58 +0000603static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
604 const Expr *RhsExpr,
605 const ASTContext *AstCtx) {
606 if (!LhsExpr || !RhsExpr)
607 return false;
608
609 SourceLocation LhsLoc = LhsExpr->getExprLoc();
610 SourceLocation RhsLoc = RhsExpr->getExprLoc();
611
612 if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
613 return false;
614
615 const SourceManager &SM = AstCtx->getSourceManager();
616 const LangOptions &LO = AstCtx->getLangOpts();
617
618 return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
619 Lexer::getImmediateMacroName(RhsLoc, SM, LO));
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000620}
621
Gabor Horvath250c40d2017-11-27 15:05:24 +0000622static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
623 const Expr *&RhsExpr) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000624 if (!LhsExpr || !RhsExpr)
625 return false;
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000626
Gabor Horvathec87e172017-11-07 13:17:58 +0000627 SourceLocation LhsLoc = LhsExpr->getExprLoc();
628 SourceLocation RhsLoc = RhsExpr->getExprLoc();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000629
Gabor Horvathec87e172017-11-07 13:17:58 +0000630 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000631}
632
633void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
634 const auto AnyLiteralExpr = ignoringParenImpCasts(
635 anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
636
Gabor Horvath250c40d2017-11-27 15:05:24 +0000637 const auto BannedIntegerLiteral =
638 integerLiteral(expandedByMacro(KnownBannedMacroNames));
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000639
Gabor Horvathec87e172017-11-07 13:17:58 +0000640 // Binary with equivalent operands, like (X != 2 && X != 2).
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000641 Finder->addMatcher(
642 binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
643 hasOperatorName("%"), hasOperatorName("|"),
644 hasOperatorName("&"), hasOperatorName("^"),
645 matchers::isComparisonOperator(),
646 hasOperatorName("&&"), hasOperatorName("||"),
647 hasOperatorName("=")),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000648 operandsAreEquivalent(),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000649 // Filter noisy false positives.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000650 unless(isInTemplateInstantiation()),
651 unless(binaryOperatorIsInMacro()),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000652 unless(hasType(realFloatingPointType())),
653 unless(hasEitherOperand(hasType(realFloatingPointType()))),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000654 unless(hasLHS(AnyLiteralExpr)),
655 unless(hasDescendant(BannedIntegerLiteral)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000656 .bind("binary"),
657 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000658
Gabor Horvathec87e172017-11-07 13:17:58 +0000659 // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
Gabor Horvath250c40d2017-11-27 15:05:24 +0000660 Finder->addMatcher(conditionalOperator(expressionsAreEquivalent(),
661 // Filter noisy false positives.
662 unless(conditionalOperatorIsInMacro()),
663 unless(isInTemplateInstantiation()))
664 .bind("cond"),
665 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000666
Gabor Horvathec87e172017-11-07 13:17:58 +0000667 // Overloaded operators with equivalent operands.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000668 Finder->addMatcher(
669 cxxOperatorCallExpr(
670 anyOf(
671 hasOverloadedOperatorName("-"), hasOverloadedOperatorName("/"),
672 hasOverloadedOperatorName("%"), hasOverloadedOperatorName("|"),
673 hasOverloadedOperatorName("&"), hasOverloadedOperatorName("^"),
674 hasOverloadedOperatorName("=="), hasOverloadedOperatorName("!="),
675 hasOverloadedOperatorName("<"), hasOverloadedOperatorName("<="),
676 hasOverloadedOperatorName(">"), hasOverloadedOperatorName(">="),
677 hasOverloadedOperatorName("&&"), hasOverloadedOperatorName("||"),
678 hasOverloadedOperatorName("=")),
679 parametersAreEquivalent(),
680 // Filter noisy false positives.
681 unless(isMacro()), unless(isInTemplateInstantiation()))
682 .bind("call"),
683 this);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000684
Gabor Horvath91c66712017-12-20 12:22:16 +0000685 // Match expressions like: !(1 | 2 | 3)
686 Finder->addMatcher(
687 implicitCastExpr(
688 hasImplicitDestinationType(isInteger()),
689 has(unaryOperator(
690 hasOperatorName("!"),
691 hasUnaryOperand(ignoringParenImpCasts(binaryOperator(
692 anyOf(hasOperatorName("|"), hasOperatorName("&")),
693 hasLHS(anyOf(binaryOperator(anyOf(hasOperatorName("|"),
694 hasOperatorName("&"))),
695 integerLiteral())),
696 hasRHS(integerLiteral())))))
697 .bind("logical-bitwise-confusion"))),
698 this);
699
700 // Match expressions like: (X << 8) & 0xFF
701 Finder->addMatcher(
702 binaryOperator(hasOperatorName("&"),
703 hasEitherOperand(ignoringParenImpCasts(binaryOperator(
704 hasOperatorName("<<"),
705 hasRHS(ignoringParenImpCasts(
706 integerLiteral().bind("shift-const")))))),
707 hasEitherOperand(ignoringParenImpCasts(
708 integerLiteral().bind("and-const"))))
709 .bind("left-right-shift-confusion"),
710 this);
711
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000712 // Match common expressions and apply more checks to find redundant
713 // sub-expressions.
714 // a) Expr <op> K1 == K2
715 // b) Expr <op> K1 == Expr
716 // c) Expr <op> K1 == Expr <op> K2
717 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
718 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
719 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
720 const auto CstRight = matchIntegerConstantExpr("rhs");
721 const auto SymRight = matchSymbolicExpr("rhs");
722
723 // Match expressions like: x <op> 0xFF == 0xF00.
724 Finder->addMatcher(binaryOperator(isComparisonOperator(),
725 hasEitherOperand(BinOpCstLeft),
726 hasEitherOperand(CstRight))
727 .bind("binop-const-compare-to-const"),
728 this);
729
730 // Match expressions like: x <op> 0xFF == x.
731 Finder->addMatcher(
732 binaryOperator(isComparisonOperator(),
733 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
734 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
735 .bind("binop-const-compare-to-sym"),
736 this);
737
738 // Match expressions like: x <op> 10 == x <op> 12.
739 Finder->addMatcher(binaryOperator(isComparisonOperator(),
740 hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
741 // Already reported as redundant.
742 unless(operandsAreEquivalent()))
743 .bind("binop-const-compare-to-binop-const"),
744 this);
745
746 // Match relational expressions combined with logical operators and find
747 // redundant sub-expressions.
748 // see: 'checkRelationalExpr'
749
750 // Match expressions like: x < 2 && x > 2.
751 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
752 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
753 Finder->addMatcher(
754 binaryOperator(anyOf(hasOperatorName("||"), hasOperatorName("&&")),
755 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
756 // Already reported as redundant.
757 unless(operandsAreEquivalent()))
758 .bind("comparisons-of-symbol-and-const"),
759 this);
760}
761
762void RedundantExpressionCheck::checkArithmeticExpr(
763 const MatchFinder::MatchResult &Result) {
764 APSInt LhsValue, RhsValue;
765 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
766 BinaryOperatorKind LhsOpcode, RhsOpcode;
767
768 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
769 "binop-const-compare-to-sym")) {
770 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
771 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
772 LhsValue) ||
773 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
774 !areEquivalentExpr(LhsSymbol, RhsSymbol))
775 return;
776
777 // Check expressions: x + k == x or x - k == x.
778 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
779 if ((LhsValue != 0 && Opcode == BO_EQ) ||
780 (LhsValue == 0 && Opcode == BO_NE))
781 diag(ComparisonOperator->getOperatorLoc(),
782 "logical expression is always false");
783 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
784 (LhsValue != 0 && Opcode == BO_NE))
785 diag(ComparisonOperator->getOperatorLoc(),
786 "logical expression is always true");
787 }
788 } else if (const auto *ComparisonOperator =
789 Result.Nodes.getNodeAs<BinaryOperator>(
790 "binop-const-compare-to-binop-const")) {
791 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
792
793 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
794 LhsValue) ||
795 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
796 RhsValue) ||
797 !areEquivalentExpr(LhsSymbol, RhsSymbol))
798 return;
799
Gabor Horvathec87e172017-11-07 13:17:58 +0000800 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
801 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000802
803 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
804 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
805 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
806 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
807 diag(ComparisonOperator->getOperatorLoc(),
808 "logical expression is always true");
809 } else if ((Opcode == BO_EQ &&
810 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
811 (Opcode == BO_NE &&
812 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
813 diag(ComparisonOperator->getOperatorLoc(),
814 "logical expression is always false");
815 }
816 }
817 }
818}
819
Gabor Horvath91c66712017-12-20 12:22:16 +0000820static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value) {
821 return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
822}
823
824static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
825 APSInt Value) {
826 return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
827}
828
829static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value) {
830 return ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) ||
831 ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
832}
833
834
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000835void RedundantExpressionCheck::checkBitwiseExpr(
836 const MatchFinder::MatchResult &Result) {
837 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
838 "binop-const-compare-to-const")) {
839 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
840
841 APSInt LhsValue, RhsValue;
842 const Expr *LhsSymbol = nullptr;
843 BinaryOperatorKind LhsOpcode;
844 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
845 LhsValue) ||
846 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
847 return;
848
849 uint64_t LhsConstant = LhsValue.getZExtValue();
850 uint64_t RhsConstant = RhsValue.getZExtValue();
851 SourceLocation Loc = ComparisonOperator->getOperatorLoc();
852
853 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
854 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
855 if (Opcode == BO_EQ)
856 diag(Loc, "logical expression is always false");
857 else if (Opcode == BO_NE)
858 diag(Loc, "logical expression is always true");
859 }
860
861 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
862 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
863 if (Opcode == BO_EQ)
864 diag(Loc, "logical expression is always false");
865 else if (Opcode == BO_NE)
866 diag(Loc, "logical expression is always true");
867 }
Gabor Horvath91c66712017-12-20 12:22:16 +0000868 } else if (const auto *IneffectiveOperator =
869 Result.Nodes.getNodeAs<BinaryOperator>(
870 "ineffective-bitwise")) {
871 APSInt Value;
872 const Expr *Sym = nullptr, *ConstExpr = nullptr;
873
874 if (!retrieveSymbolicExpr(Result, "ineffective-bitwise", Sym) ||
875 !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value,
876 ConstExpr))
877 return;
878
879 if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
880 return;
881
882 SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
883
884 BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode();
885 if (exprEvaluatesToZero(Opcode, Value)) {
886 diag(Loc, "expression always evaluates to 0");
887 } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) {
888 SourceRange ConstExprRange(ConstExpr->getLocStart(),
889 ConstExpr->getLocEnd());
890 StringRef ConstExprText = Lexer::getSourceText(
891 CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager,
892 Result.Context->getLangOpts());
893
894 diag(Loc, "expression always evaluates to '%0'") << ConstExprText;
895
896 } else if (exprEvaluatesToSymbolic(Opcode, Value)) {
897 SourceRange SymExprRange(Sym->getLocStart(), Sym->getLocEnd());
898
899 StringRef ExprText = Lexer::getSourceText(
900 CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager,
901 Result.Context->getLangOpts());
902
903 diag(Loc, "expression always evaluates to '%0'") << ExprText;
904 }
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000905 }
906}
907
908void RedundantExpressionCheck::checkRelationalExpr(
909 const MatchFinder::MatchResult &Result) {
910 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
911 "comparisons-of-symbol-and-const")) {
912 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
Gabor Horvathec87e172017-11-07 13:17:58 +0000913 // E.g.: (X < 2) && (X > 4)
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000914 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
915
916 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000917 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
Gabor Horvathec87e172017-11-07 13:17:58 +0000918 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000919 BinaryOperatorKind LhsOpcode, RhsOpcode;
Gabor Horvathec87e172017-11-07 13:17:58 +0000920 APSInt LhsValue, RhsValue;
921
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000922 if (!retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000923 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000924 !retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000925 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000926 !areEquivalentExpr(LhsSymbol, RhsSymbol))
927 return;
928
Gabor Horvathec87e172017-11-07 13:17:58 +0000929 // Bring expr to a canonical form: smallest constant must be on the left.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000930 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
931 std::swap(LhsExpr, RhsExpr);
932 std::swap(LhsValue, RhsValue);
933 std::swap(LhsSymbol, RhsSymbol);
934 std::swap(LhsOpcode, RhsOpcode);
935 }
936
Gabor Horvathec87e172017-11-07 13:17:58 +0000937 // Constants come from two different macros, or one of them is a macro.
938 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
939 areExprsMacroAndNonMacro(LhsConst, RhsConst))
940 return;
941
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000942 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
943 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
944 diag(ComparisonOperator->getOperatorLoc(),
Gabor Horvathec87e172017-11-07 13:17:58 +0000945 "equivalent expression on both sides of logical operator");
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000946 return;
947 }
948
949 if (Opcode == BO_LAnd) {
950 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
951 diag(ComparisonOperator->getOperatorLoc(),
952 "logical expression is always false");
953 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
954 diag(LhsExpr->getExprLoc(), "expression is redundant");
955 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
956 diag(RhsExpr->getExprLoc(), "expression is redundant");
957 }
958 }
959
960 if (Opcode == BO_LOr) {
961 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
962 diag(ComparisonOperator->getOperatorLoc(),
963 "logical expression is always true");
964 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
965 diag(RhsExpr->getExprLoc(), "expression is redundant");
966 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
967 diag(LhsExpr->getExprLoc(), "expression is redundant");
968 }
969 }
970 }
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000971}
972
973void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000974 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000975 // If the expression's constants are macros, check whether they are
976 // intentional.
977 if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
978 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
979 BinaryOperatorKind MainOpcode, SideOpcode;
980
Gabor Horvath250c40d2017-11-27 15:05:24 +0000981 if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
982 LhsConst, RhsConst, Result.Context))
983 return;
Gabor Horvathec87e172017-11-07 13:17:58 +0000984
985 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
986 areExprsMacroAndNonMacro(LhsConst, RhsConst))
987 return;
988 }
989
990 diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
991 }
992
993 if (const auto *CondOp =
994 Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
995 const Expr *TrueExpr = CondOp->getTrueExpr();
996 const Expr *FalseExpr = CondOp->getFalseExpr();
997
998 if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
999 areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
1000 return;
1001 diag(CondOp->getColonLoc(),
1002 "'true' and 'false' expressions are equivalent");
1003 }
1004
1005 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
Gabor Horvath250c40d2017-11-27 15:05:24 +00001006 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
1007 if (!OverloadedFunctionDecl)
1008 return;
1009
1010 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, true))
1011 return;
1012
Gabor Horvathec87e172017-11-07 13:17:58 +00001013 diag(Call->getOperatorLoc(),
1014 "both sides of overloaded operator are equivalent");
1015 }
1016
Gabor Horvath91c66712017-12-20 12:22:16 +00001017 if (const auto *NegateOperator =
1018 Result.Nodes.getNodeAs<UnaryOperator>("logical-bitwise-confusion")) {
1019 SourceLocation OperatorLoc = NegateOperator->getOperatorLoc();
1020
1021 auto Diag =
1022 diag(OperatorLoc,
1023 "ineffective logical negation operator used; did you mean '~'?");
1024 SourceLocation LogicalNotLocation = OperatorLoc.getLocWithOffset(1);
1025
1026 if (!LogicalNotLocation.isMacroID())
1027 Diag << FixItHint::CreateReplacement(
1028 CharSourceRange::getCharRange(OperatorLoc, LogicalNotLocation), "~");
1029 }
1030
1031 if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs<BinaryOperator>(
1032 "left-right-shift-confusion")) {
1033 const auto *ShiftingConst = Result.Nodes.getNodeAs<Expr>("shift-const");
1034 assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!");
1035 APSInt ShiftingValue;
1036
1037 if (!ShiftingConst->isIntegerConstantExpr(ShiftingValue, *Result.Context))
1038 return;
1039
1040 const auto *AndConst = Result.Nodes.getNodeAs<Expr>("and-const");
1041 assert(AndConst && "Expr* 'AndCont' is nullptr!");
1042 APSInt AndValue;
1043 if (!AndConst->isIntegerConstantExpr(AndValue, *Result.Context))
1044 return;
1045
1046 // If ShiftingConst is shifted left with more bits than the position of the
1047 // leftmost 1 in the bit representation of AndValue, AndConstant is
1048 // ineffective.
Benjamin Kramer10db8d62018-02-02 13:23:21 +00001049 if (AndValue.getActiveBits() > ShiftingValue)
Gabor Horvath91c66712017-12-20 12:22:16 +00001050 return;
1051
1052 auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
Alexander Kornienko396fc872018-02-01 16:39:12 +00001053 "ineffective bitwise and operation");
Gabor Horvath91c66712017-12-20 12:22:16 +00001054 }
1055
Gabor Horvathec87e172017-11-07 13:17:58 +00001056 // Check for the following bound expressions:
1057 // - "binop-const-compare-to-sym",
1058 // - "binop-const-compare-to-binop-const",
1059 // Produced message:
1060 // -> "logical expression is always false/true"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001061 checkArithmeticExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001062
1063 // Check for the following bound expression:
1064 // - "binop-const-compare-to-const",
Gabor Horvath91c66712017-12-20 12:22:16 +00001065 // - "ineffective-bitwise"
Gabor Horvathec87e172017-11-07 13:17:58 +00001066 // Produced message:
1067 // -> "logical expression is always false/true"
Gabor Horvath91c66712017-12-20 12:22:16 +00001068 // -> "expression always evaluates to ..."
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001069 checkBitwiseExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001070
1071 // Check for te following bound expression:
1072 // - "comparisons-of-symbol-and-const",
1073 // Produced messages:
1074 // -> "equivalent expression on both sides of logical operator",
1075 // -> "logical expression is always false/true"
1076 // -> "expression is redundant"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001077 checkRelationalExpr(Result);
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001078}
1079
1080} // namespace misc
1081} // namespace tidy
1082} // namespace clang