blob: 635c29ffe7d4784dd749659e125dbc80d2a976cc [file] [log] [blame]
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001//===--- RedundantExpressionCheck.cpp - clang-tidy-------------------------===//
2//
3// The LLVM Compiler Infrastructure
4//
5// This file is distributed under the University of Illinois Open Source
6// License. See LICENSE.TXT for details.
7//
8//===----------------------------------------------------------------------===//
9
10#include "RedundantExpressionCheck.h"
11#include "../utils/Matchers.h"
Etienne Bergeronc87599f2016-05-12 04:32:47 +000012#include "../utils/OptionsUtils.h"
Etienne Bergeronbda187d2016-04-26 17:30:30 +000013#include "clang/AST/ASTContext.h"
14#include "clang/ASTMatchers/ASTMatchFinder.h"
Eugene Zelenko90c117a2016-11-01 18:33:50 +000015#include "clang/Basic/LLVM.h"
16#include "clang/Basic/SourceLocation.h"
17#include "clang/Basic/SourceManager.h"
Etienne Bergeronc87599f2016-05-12 04:32:47 +000018#include "clang/Lex/Lexer.h"
Eugene Zelenko90c117a2016-11-01 18:33:50 +000019#include "llvm/ADT/APInt.h"
20#include "llvm/ADT/APSInt.h"
21#include "llvm/ADT/FoldingSet.h"
22#include "llvm/Support/Casting.h"
23#include <algorithm>
24#include <cassert>
25#include <cstdint>
Eugene Zelenko90c117a2016-11-01 18:33:50 +000026#include <string>
27#include <vector>
Etienne Bergeronbda187d2016-04-26 17:30:30 +000028
29using namespace clang::ast_matchers;
Etienne Bergeron00639bc2016-07-07 04:03:05 +000030using namespace clang::tidy::matchers;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000031
32namespace clang {
33namespace tidy {
34namespace misc {
35
Etienne Bergeron00639bc2016-07-07 04:03:05 +000036namespace {
37using llvm::APSInt;
38} // namespace
39
Gabor Horvathec87e172017-11-07 13:17:58 +000040static const llvm::StringSet<> KnownBannedMacroNames = {"EAGAIN", "EWOULDBLOCK",
41 "SIGCLD", "SIGCHLD"};
Etienne Bergeronc87599f2016-05-12 04:32:47 +000042
Etienne Bergeron00639bc2016-07-07 04:03:05 +000043static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
44 Result = Value;
45 ++Result;
46 return Value < Result;
47}
48
Etienne Bergeronc87599f2016-05-12 04:32:47 +000049static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
50 const NestedNameSpecifier *Right) {
51 llvm::FoldingSetNodeID LeftID, RightID;
52 Left->Profile(LeftID);
53 Right->Profile(RightID);
54 return LeftID == RightID;
55}
56
57static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
Etienne Bergeronbda187d2016-04-26 17:30:30 +000058 if (!Left || !Right)
59 return !Left && !Right;
60
61 Left = Left->IgnoreParens();
62 Right = Right->IgnoreParens();
63
64 // Compare classes.
65 if (Left->getStmtClass() != Right->getStmtClass())
66 return false;
67
68 // Compare children.
69 Expr::const_child_iterator LeftIter = Left->child_begin();
70 Expr::const_child_iterator RightIter = Right->child_begin();
71 while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
Etienne Bergeronc87599f2016-05-12 04:32:47 +000072 if (!areEquivalentExpr(dyn_cast<Expr>(*LeftIter),
73 dyn_cast<Expr>(*RightIter)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +000074 return false;
75 ++LeftIter;
76 ++RightIter;
77 }
78 if (LeftIter != Left->child_end() || RightIter != Right->child_end())
79 return false;
80
81 // Perform extra checks.
82 switch (Left->getStmtClass()) {
83 default:
84 return false;
85
86 case Stmt::CharacterLiteralClass:
87 return cast<CharacterLiteral>(Left)->getValue() ==
88 cast<CharacterLiteral>(Right)->getValue();
89 case Stmt::IntegerLiteralClass: {
90 llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
91 llvm::APInt RightLit = cast<IntegerLiteral>(Right)->getValue();
Etienne Bergeronc87599f2016-05-12 04:32:47 +000092 return LeftLit.getBitWidth() == RightLit.getBitWidth() &&
93 LeftLit == RightLit;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000094 }
95 case Stmt::FloatingLiteralClass:
96 return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
97 cast<FloatingLiteral>(Right)->getValue());
98 case Stmt::StringLiteralClass:
99 return cast<StringLiteral>(Left)->getBytes() ==
100 cast<StringLiteral>(Right)->getBytes();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000101 case Stmt::CXXOperatorCallExprClass:
102 return cast<CXXOperatorCallExpr>(Left)->getOperator() ==
103 cast<CXXOperatorCallExpr>(Right)->getOperator();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000104 case Stmt::DependentScopeDeclRefExprClass:
105 if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
106 cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
107 return false;
108 return areEquivalentNameSpecifier(
109 cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
110 cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000111 case Stmt::DeclRefExprClass:
112 return cast<DeclRefExpr>(Left)->getDecl() ==
113 cast<DeclRefExpr>(Right)->getDecl();
114 case Stmt::MemberExprClass:
115 return cast<MemberExpr>(Left)->getMemberDecl() ==
116 cast<MemberExpr>(Right)->getMemberDecl();
Gabor Horvathec87e172017-11-07 13:17:58 +0000117 case Stmt::CXXFunctionalCastExprClass:
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000118 case Stmt::CStyleCastExprClass:
Gabor Horvathec87e172017-11-07 13:17:58 +0000119 return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
120 cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000121 case Stmt::CallExprClass:
122 case Stmt::ImplicitCastExprClass:
123 case Stmt::ArraySubscriptExprClass:
124 return true;
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000125 case Stmt::UnaryOperatorClass:
126 if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
127 return false;
128 return cast<UnaryOperator>(Left)->getOpcode() ==
129 cast<UnaryOperator>(Right)->getOpcode();
130 case Stmt::BinaryOperatorClass:
131 return cast<BinaryOperator>(Left)->getOpcode() ==
132 cast<BinaryOperator>(Right)->getOpcode();
133 }
134}
135
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000136// For a given expression 'x', returns whether the ranges covered by the
137// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
138static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
139 const APSInt &ValueLHS,
140 BinaryOperatorKind OpcodeRHS,
141 const APSInt &ValueRHS) {
142 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
143 "Values must be ordered");
144 // Handle the case where constants are the same: x <= 4 <==> x <= 4.
145 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
146 return OpcodeLHS == OpcodeRHS;
147
148 // Handle the case where constants are off by one: x <= 4 <==> x < 5.
149 APSInt ValueLHS_plus1;
150 return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
151 (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
152 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
153 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0;
154}
155
156// For a given expression 'x', returns whether the ranges covered by the
157// relational operators are fully disjoint (i.e. x < 4 and x > 7).
158static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
159 const APSInt &ValueLHS,
160 BinaryOperatorKind OpcodeRHS,
161 const APSInt &ValueRHS) {
162 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
163 "Values must be ordered");
164
165 // Handle cases where the constants are the same.
166 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
167 switch (OpcodeLHS) {
168 case BO_EQ:
169 return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
170 case BO_NE:
171 return OpcodeRHS == BO_EQ;
172 case BO_LE:
173 return OpcodeRHS == BO_GT;
174 case BO_GE:
175 return OpcodeRHS == BO_LT;
176 case BO_LT:
177 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
178 case BO_GT:
179 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
180 default:
181 return false;
182 }
183 }
184
185 // Handle cases where the constants are different.
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000186 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LT || OpcodeLHS == BO_LE) &&
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000187 (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
188 return true;
189
190 // Handle the case where constants are off by one: x > 5 && x < 6.
191 APSInt ValueLHS_plus1;
192 if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
193 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
194 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
195 return true;
196
197 return false;
198}
199
200// Returns whether the ranges covered by the union of both relational
Gabor Horvath91c66712017-12-20 12:22:16 +0000201// expressions cover the whole domain (i.e. x < 10 and x > 0).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000202static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
203 const APSInt &ValueLHS,
204 BinaryOperatorKind OpcodeRHS,
205 const APSInt &ValueRHS) {
206 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
207 "Values must be ordered");
208
209 // Handle cases where the constants are the same: x < 5 || x >= 5.
210 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
211 switch (OpcodeLHS) {
212 case BO_EQ:
213 return OpcodeRHS == BO_NE;
214 case BO_NE:
215 return OpcodeRHS == BO_EQ;
216 case BO_LE:
217 return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
218 case BO_LT:
219 return OpcodeRHS == BO_GE;
220 case BO_GE:
221 return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
222 case BO_GT:
223 return OpcodeRHS == BO_LE;
224 default:
225 return false;
226 }
227 }
228
229 // Handle the case where constants are off by one: x <= 4 || x >= 5.
230 APSInt ValueLHS_plus1;
231 if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
232 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
233 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
234 return true;
235
236 // Handle cases where the constants are different: x > 4 || x <= 7.
237 if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
238 (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
239 return true;
240
Alexander Kornienkoc3acd2e2017-03-23 15:13:54 +0000241 // Handle cases where constants are different but both ops are !=, like:
242 // x != 5 || x != 10
243 if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE)
244 return true;
245
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000246 return false;
247}
248
249static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
250 const APSInt &ValueLHS,
251 BinaryOperatorKind OpcodeRHS,
252 const APSInt &ValueRHS) {
253 int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
254 switch (OpcodeLHS) {
255 case BO_EQ:
256 return OpcodeRHS == BO_EQ && Comparison == 0;
257 case BO_NE:
258 return (OpcodeRHS == BO_NE && Comparison == 0) ||
259 (OpcodeRHS == BO_EQ && Comparison != 0) ||
260 (OpcodeRHS == BO_LT && Comparison >= 0) ||
261 (OpcodeRHS == BO_LE && Comparison > 0) ||
262 (OpcodeRHS == BO_GT && Comparison <= 0) ||
263 (OpcodeRHS == BO_GE && Comparison < 0);
264
265 case BO_LT:
266 return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
267 (OpcodeRHS == BO_LE && Comparison > 0) ||
268 (OpcodeRHS == BO_EQ && Comparison > 0));
269 case BO_GT:
270 return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
271 (OpcodeRHS == BO_GE && Comparison < 0) ||
272 (OpcodeRHS == BO_EQ && Comparison < 0));
273 case BO_LE:
274 return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
275 Comparison >= 0;
276 case BO_GE:
277 return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
278 Comparison <= 0;
279 default:
280 return false;
281 }
282}
283
Gabor Horvathec87e172017-11-07 13:17:58 +0000284static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
285 APSInt &Value) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000286 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
Gabor Horvathec87e172017-11-07 13:17:58 +0000298AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
299 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
300}
301
302AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
303 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
304}
305
306AST_MATCHER(CallExpr, parametersAreEquivalent) {
307 return Node.getNumArgs() == 2 &&
308 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
309}
310
311AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
312 return Node.getOperatorLoc().isMacroID();
313}
314
315AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
316 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
317}
318
319AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
320
321AST_MATCHER_P(Expr, expandedByMacro, llvm::StringSet<>, Names) {
322 const SourceManager &SM = Finder->getASTContext().getSourceManager();
323 const LangOptions &LO = Finder->getASTContext().getLangOpts();
324 SourceLocation Loc = Node.getExprLoc();
325 while (Loc.isMacroID()) {
326 StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
327 if (Names.count(MacroName))
328 return true;
329 Loc = SM.getImmediateMacroCallerLoc(Loc);
330 }
331 return false;
332}
333
334// Returns a matcher for integer constant expressions.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000335static ast_matchers::internal::Matcher<Expr>
336matchIntegerConstantExpr(StringRef Id) {
337 std::string CstId = (Id + "-const").str();
338 return expr(isIntegerConstantExpr()).bind(CstId);
339}
340
Gabor Horvathec87e172017-11-07 13:17:58 +0000341// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
342// name 'Id' and stores it into 'ConstExpr', the value of the expression is
343// stored into `Value`.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000344static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
Gabor Horvathec87e172017-11-07 13:17:58 +0000345 StringRef Id, APSInt &Value,
346 const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000347 std::string CstId = (Id + "-const").str();
Gabor Horvathec87e172017-11-07 13:17:58 +0000348 ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
349 return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000350}
351
Gabor Horvathec87e172017-11-07 13:17:58 +0000352// Overloaded `retrieveIntegerConstantExpr` for compatibility.
353static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
354 StringRef Id, APSInt &Value) {
355 const Expr *ConstExpr = nullptr;
356 return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
357}
358
359// Returns a matcher for symbolic expressions (matches every expression except
360// ingeter constant expressions).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000361static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
362 std::string SymId = (Id + "-sym").str();
363 return ignoringParenImpCasts(
364 expr(unless(isIntegerConstantExpr())).bind(SymId));
365}
366
Gabor Horvathec87e172017-11-07 13:17:58 +0000367// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
368// stores it into 'SymExpr'.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000369static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
370 StringRef Id, const Expr *&SymExpr) {
371 std::string SymId = (Id + "-sym").str();
372 if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
373 SymExpr = Node;
374 return true;
375 }
376 return false;
377}
378
379// Match a binary operator between a symbolic expression and an integer constant
380// expression.
381static ast_matchers::internal::Matcher<Expr>
382matchBinOpIntegerConstantExpr(StringRef Id) {
383 const auto BinOpCstExpr =
384 expr(
385 anyOf(binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("|"),
386 hasOperatorName("&")),
387 hasEitherOperand(matchSymbolicExpr(Id)),
388 hasEitherOperand(matchIntegerConstantExpr(Id))),
389 binaryOperator(hasOperatorName("-"),
390 hasLHS(matchSymbolicExpr(Id)),
391 hasRHS(matchIntegerConstantExpr(Id)))))
392 .bind(Id);
393 return ignoringParenImpCasts(BinOpCstExpr);
394}
395
Gabor Horvathec87e172017-11-07 13:17:58 +0000396// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000397// name 'Id'.
398static bool
399retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
400 StringRef Id, BinaryOperatorKind &Opcode,
401 const Expr *&Symbol, APSInt &Value) {
402 if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
403 Opcode = BinExpr->getOpcode();
404 return retrieveSymbolicExpr(Result, Id, Symbol) &&
405 retrieveIntegerConstantExpr(Result, Id, Value);
406 }
407 return false;
408}
409
Gabor Horvathec87e172017-11-07 13:17:58 +0000410// Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000411static ast_matchers::internal::Matcher<Expr>
412matchRelationalIntegerConstantExpr(StringRef Id) {
413 std::string CastId = (Id + "-cast").str();
414 std::string SwapId = (Id + "-swap").str();
415 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000416 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000417
418 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
419 isComparisonOperator(), expr().bind(Id),
420 anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
421 hasRHS(matchIntegerConstantExpr(Id))),
422 allOf(hasLHS(matchIntegerConstantExpr(Id)),
423 hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
424
425 // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
426 // to if (x != 0)).
427 const auto CastExpr =
428 implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
429 hasSourceExpression(matchSymbolicExpr(Id)))
430 .bind(CastId);
431
432 const auto NegateRelationalExpr =
433 unaryOperator(hasOperatorName("!"),
434 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
435 .bind(NegateId);
436
Gabor Horvathec87e172017-11-07 13:17:58 +0000437 // Do not bind to double negation.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000438 const auto NegateNegateRelationalExpr =
439 unaryOperator(hasOperatorName("!"),
440 hasUnaryOperand(unaryOperator(
441 hasOperatorName("!"),
442 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
443
Gabor Horvath250c40d2017-11-27 15:05:24 +0000444 const auto OverloadedOperatorExpr =
445 cxxOperatorCallExpr(
446 anyOf(hasOverloadedOperatorName("=="),
447 hasOverloadedOperatorName("!="), hasOverloadedOperatorName("<"),
448 hasOverloadedOperatorName("<="), hasOverloadedOperatorName(">"),
449 hasOverloadedOperatorName(">=")),
450 // Filter noisy false positives.
451 unless(isMacro()), unless(isInTemplateInstantiation()))
452 .bind(OverloadId);
453
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000454 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
Gabor Horvath250c40d2017-11-27 15:05:24 +0000455 NegateNegateRelationalExpr, OverloadedOperatorExpr);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000456}
457
Gabor Horvath250c40d2017-11-27 15:05:24 +0000458// Checks whether a function param is non constant reference type, and may
459// be modified in the function.
460static bool isNonConstReferenceType(QualType ParamType) {
461 return ParamType->isReferenceType() &&
462 !ParamType.getNonReferenceType().isConstQualified();
463}
464
465// Checks whether the arguments of an overloaded operator can be modified in the
466// function.
467// For operators that take an instance and a constant as arguments, only the
468// first argument (the instance) needs to be checked, since the constant itself
469// is a temporary expression. Whether the second parameter is checked is
470// controlled by the parameter `ParamsToCheckCount`.
471static bool
472canOverloadedOperatorArgsBeModified(const FunctionDecl *OperatorDecl,
473 bool checkSecondParam) {
474 unsigned ParamCount = OperatorDecl->getNumParams();
475
476 // Overloaded operators declared inside a class have only one param.
477 // These functions must be declared const in order to not be able to modify
478 // the instance of the class they are called through.
479 if (ParamCount == 1 &&
480 !OperatorDecl->getType()->getAs<FunctionType>()->isConst())
481 return true;
482
483 if (isNonConstReferenceType(OperatorDecl->getParamDecl(0)->getType()))
484 return true;
485
486 return checkSecondParam && ParamCount == 2 &&
487 isNonConstReferenceType(OperatorDecl->getParamDecl(1)->getType());
488}
489
490// Retrieves sub-expressions matched by 'matchRelationalIntegerConstantExpr'
491// with name 'Id'.
Gabor Horvathec87e172017-11-07 13:17:58 +0000492static bool retrieveRelationalIntegerConstantExpr(
493 const MatchFinder::MatchResult &Result, StringRef Id,
494 const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
495 APSInt &Value, const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000496 std::string CastId = (Id + "-cast").str();
497 std::string SwapId = (Id + "-swap").str();
498 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000499 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000500
501 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
502 // Operand received with explicit comparator.
503 Opcode = Bin->getOpcode();
504 OperandExpr = Bin;
Gabor Horvathec87e172017-11-07 13:17:58 +0000505
506 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000507 return false;
508 } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
509 // Operand received with implicit comparator (cast).
510 Opcode = BO_NE;
511 OperandExpr = Cast;
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000512 Value = APSInt(32, false);
Gabor Horvath250c40d2017-11-27 15:05:24 +0000513 } else if (const auto *OverloadedOperatorExpr =
514 Result.Nodes.getNodeAs<CXXOperatorCallExpr>(OverloadId)) {
515 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
516 if (!OverloadedFunctionDecl)
517 return false;
518
519 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
520 return false;
521
Gabor Horvath91c66712017-12-20 12:22:16 +0000522 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
523 return false;
524
Gabor Horvath250c40d2017-11-27 15:05:24 +0000525 if (!OverloadedOperatorExpr->getArg(1)->isIntegerConstantExpr(
526 Value, *Result.Context))
527 return false;
528
529 Symbol = OverloadedOperatorExpr->getArg(0);
530 OperandExpr = OverloadedOperatorExpr;
531 Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
532
533 return BinaryOperator::isComparisonOp(Opcode);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000534 } else {
535 return false;
536 }
537
538 if (!retrieveSymbolicExpr(Result, Id, Symbol))
539 return false;
540
541 if (Result.Nodes.getNodeAs<Expr>(SwapId))
542 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
543 if (Result.Nodes.getNodeAs<Expr>(NegateId))
544 Opcode = BinaryOperator::negateComparisonOp(Opcode);
Gabor Horvathec87e172017-11-07 13:17:58 +0000545 return true;
546}
547
548// Checks for expressions like (X == 4) && (Y != 9)
549static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
550 const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
551 const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
552
553 if (!LhsBinOp || !RhsBinOp)
554 return false;
555
556 if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
557 LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
558 (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
559 RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
560 return true;
561 return false;
562}
563
564// Retrieves integer constant subexpressions from binary operator expressions
Gabor Horvath91c66712017-12-20 12:22:16 +0000565// that have two equivalent sides.
Gabor Horvathec87e172017-11-07 13:17:58 +0000566// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
567static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
568 BinaryOperatorKind &MainOpcode,
569 BinaryOperatorKind &SideOpcode,
570 const Expr *&LhsConst,
571 const Expr *&RhsConst,
572 const ASTContext *AstCtx) {
573 assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
574 "Both sides of binary operator must be constant expressions!");
575
576 MainOpcode = BinOp->getOpcode();
577
578 const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
579 const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
580
581 LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
582 ? BinOpLhs->getLHS()
583 : BinOpLhs->getRHS();
584 RhsConst = BinOpRhs->getLHS()->isIntegerConstantExpr(*AstCtx)
585 ? BinOpRhs->getLHS()
586 : BinOpRhs->getRHS();
587
588 if (!LhsConst || !RhsConst)
589 return false;
590
591 assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
592 "Sides of the binary operator must be equivalent expressions!");
593
594 SideOpcode = BinOpLhs->getOpcode();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000595
596 return true;
597}
598
Gabor Horvathec87e172017-11-07 13:17:58 +0000599static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
600 const Expr *RhsExpr,
601 const ASTContext *AstCtx) {
602 if (!LhsExpr || !RhsExpr)
603 return false;
604
605 SourceLocation LhsLoc = LhsExpr->getExprLoc();
606 SourceLocation RhsLoc = RhsExpr->getExprLoc();
607
608 if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
609 return false;
610
611 const SourceManager &SM = AstCtx->getSourceManager();
612 const LangOptions &LO = AstCtx->getLangOpts();
613
614 return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
615 Lexer::getImmediateMacroName(RhsLoc, SM, LO));
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000616}
617
Gabor Horvath250c40d2017-11-27 15:05:24 +0000618static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
619 const Expr *&RhsExpr) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000620 if (!LhsExpr || !RhsExpr)
621 return false;
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000622
Gabor Horvathec87e172017-11-07 13:17:58 +0000623 SourceLocation LhsLoc = LhsExpr->getExprLoc();
624 SourceLocation RhsLoc = RhsExpr->getExprLoc();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000625
Gabor Horvathec87e172017-11-07 13:17:58 +0000626 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000627}
628
629void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
630 const auto AnyLiteralExpr = ignoringParenImpCasts(
631 anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
632
Gabor Horvath250c40d2017-11-27 15:05:24 +0000633 const auto BannedIntegerLiteral =
634 integerLiteral(expandedByMacro(KnownBannedMacroNames));
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000635
Gabor Horvathec87e172017-11-07 13:17:58 +0000636 // Binary with equivalent operands, like (X != 2 && X != 2).
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000637 Finder->addMatcher(
638 binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
639 hasOperatorName("%"), hasOperatorName("|"),
640 hasOperatorName("&"), hasOperatorName("^"),
641 matchers::isComparisonOperator(),
642 hasOperatorName("&&"), hasOperatorName("||"),
643 hasOperatorName("=")),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000644 operandsAreEquivalent(),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000645 // Filter noisy false positives.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000646 unless(isInTemplateInstantiation()),
647 unless(binaryOperatorIsInMacro()),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000648 unless(hasType(realFloatingPointType())),
649 unless(hasEitherOperand(hasType(realFloatingPointType()))),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000650 unless(hasLHS(AnyLiteralExpr)),
651 unless(hasDescendant(BannedIntegerLiteral)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000652 .bind("binary"),
653 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000654
Gabor Horvathec87e172017-11-07 13:17:58 +0000655 // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
Gabor Horvath250c40d2017-11-27 15:05:24 +0000656 Finder->addMatcher(conditionalOperator(expressionsAreEquivalent(),
657 // Filter noisy false positives.
658 unless(conditionalOperatorIsInMacro()),
659 unless(isInTemplateInstantiation()))
660 .bind("cond"),
661 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000662
Gabor Horvathec87e172017-11-07 13:17:58 +0000663 // Overloaded operators with equivalent operands.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000664 Finder->addMatcher(
665 cxxOperatorCallExpr(
666 anyOf(
667 hasOverloadedOperatorName("-"), hasOverloadedOperatorName("/"),
668 hasOverloadedOperatorName("%"), hasOverloadedOperatorName("|"),
669 hasOverloadedOperatorName("&"), hasOverloadedOperatorName("^"),
670 hasOverloadedOperatorName("=="), hasOverloadedOperatorName("!="),
671 hasOverloadedOperatorName("<"), hasOverloadedOperatorName("<="),
672 hasOverloadedOperatorName(">"), hasOverloadedOperatorName(">="),
673 hasOverloadedOperatorName("&&"), hasOverloadedOperatorName("||"),
674 hasOverloadedOperatorName("=")),
675 parametersAreEquivalent(),
676 // Filter noisy false positives.
677 unless(isMacro()), unless(isInTemplateInstantiation()))
678 .bind("call"),
679 this);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000680
Gabor Horvath91c66712017-12-20 12:22:16 +0000681 // Match expressions like: !(1 | 2 | 3)
682 Finder->addMatcher(
683 implicitCastExpr(
684 hasImplicitDestinationType(isInteger()),
685 has(unaryOperator(
686 hasOperatorName("!"),
687 hasUnaryOperand(ignoringParenImpCasts(binaryOperator(
688 anyOf(hasOperatorName("|"), hasOperatorName("&")),
689 hasLHS(anyOf(binaryOperator(anyOf(hasOperatorName("|"),
690 hasOperatorName("&"))),
691 integerLiteral())),
692 hasRHS(integerLiteral())))))
693 .bind("logical-bitwise-confusion"))),
694 this);
695
696 // Match expressions like: (X << 8) & 0xFF
697 Finder->addMatcher(
698 binaryOperator(hasOperatorName("&"),
699 hasEitherOperand(ignoringParenImpCasts(binaryOperator(
700 hasOperatorName("<<"),
701 hasRHS(ignoringParenImpCasts(
702 integerLiteral().bind("shift-const")))))),
703 hasEitherOperand(ignoringParenImpCasts(
704 integerLiteral().bind("and-const"))))
705 .bind("left-right-shift-confusion"),
706 this);
707
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000708 // Match common expressions and apply more checks to find redundant
709 // sub-expressions.
710 // a) Expr <op> K1 == K2
711 // b) Expr <op> K1 == Expr
712 // c) Expr <op> K1 == Expr <op> K2
713 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
714 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
715 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
716 const auto CstRight = matchIntegerConstantExpr("rhs");
717 const auto SymRight = matchSymbolicExpr("rhs");
718
719 // Match expressions like: x <op> 0xFF == 0xF00.
720 Finder->addMatcher(binaryOperator(isComparisonOperator(),
721 hasEitherOperand(BinOpCstLeft),
722 hasEitherOperand(CstRight))
723 .bind("binop-const-compare-to-const"),
724 this);
725
726 // Match expressions like: x <op> 0xFF == x.
727 Finder->addMatcher(
728 binaryOperator(isComparisonOperator(),
729 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
730 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
731 .bind("binop-const-compare-to-sym"),
732 this);
733
734 // Match expressions like: x <op> 10 == x <op> 12.
735 Finder->addMatcher(binaryOperator(isComparisonOperator(),
736 hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
737 // Already reported as redundant.
738 unless(operandsAreEquivalent()))
739 .bind("binop-const-compare-to-binop-const"),
740 this);
741
742 // Match relational expressions combined with logical operators and find
743 // redundant sub-expressions.
744 // see: 'checkRelationalExpr'
745
746 // Match expressions like: x < 2 && x > 2.
747 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
748 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
749 Finder->addMatcher(
750 binaryOperator(anyOf(hasOperatorName("||"), hasOperatorName("&&")),
751 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
752 // Already reported as redundant.
753 unless(operandsAreEquivalent()))
754 .bind("comparisons-of-symbol-and-const"),
755 this);
756}
757
758void RedundantExpressionCheck::checkArithmeticExpr(
759 const MatchFinder::MatchResult &Result) {
760 APSInt LhsValue, RhsValue;
761 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
762 BinaryOperatorKind LhsOpcode, RhsOpcode;
763
764 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
765 "binop-const-compare-to-sym")) {
766 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
767 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
768 LhsValue) ||
769 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
770 !areEquivalentExpr(LhsSymbol, RhsSymbol))
771 return;
772
773 // Check expressions: x + k == x or x - k == x.
774 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
775 if ((LhsValue != 0 && Opcode == BO_EQ) ||
776 (LhsValue == 0 && Opcode == BO_NE))
777 diag(ComparisonOperator->getOperatorLoc(),
778 "logical expression is always false");
779 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
780 (LhsValue != 0 && Opcode == BO_NE))
781 diag(ComparisonOperator->getOperatorLoc(),
782 "logical expression is always true");
783 }
784 } else if (const auto *ComparisonOperator =
785 Result.Nodes.getNodeAs<BinaryOperator>(
786 "binop-const-compare-to-binop-const")) {
787 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
788
789 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
790 LhsValue) ||
791 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
792 RhsValue) ||
793 !areEquivalentExpr(LhsSymbol, RhsSymbol))
794 return;
795
Gabor Horvathec87e172017-11-07 13:17:58 +0000796 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
797 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000798
799 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
800 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
801 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
802 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
803 diag(ComparisonOperator->getOperatorLoc(),
804 "logical expression is always true");
805 } else if ((Opcode == BO_EQ &&
806 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
807 (Opcode == BO_NE &&
808 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
809 diag(ComparisonOperator->getOperatorLoc(),
810 "logical expression is always false");
811 }
812 }
813 }
814}
815
Gabor Horvath91c66712017-12-20 12:22:16 +0000816static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value) {
817 return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
818}
819
820static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
821 APSInt Value) {
822 return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
823}
824
825static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value) {
826 return ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) ||
827 ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
828}
829
830
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000831void RedundantExpressionCheck::checkBitwiseExpr(
832 const MatchFinder::MatchResult &Result) {
833 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
834 "binop-const-compare-to-const")) {
835 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
836
837 APSInt LhsValue, RhsValue;
838 const Expr *LhsSymbol = nullptr;
839 BinaryOperatorKind LhsOpcode;
840 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
841 LhsValue) ||
842 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
843 return;
844
845 uint64_t LhsConstant = LhsValue.getZExtValue();
846 uint64_t RhsConstant = RhsValue.getZExtValue();
847 SourceLocation Loc = ComparisonOperator->getOperatorLoc();
848
849 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
850 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
851 if (Opcode == BO_EQ)
852 diag(Loc, "logical expression is always false");
853 else if (Opcode == BO_NE)
854 diag(Loc, "logical expression is always true");
855 }
856
857 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
858 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
859 if (Opcode == BO_EQ)
860 diag(Loc, "logical expression is always false");
861 else if (Opcode == BO_NE)
862 diag(Loc, "logical expression is always true");
863 }
Gabor Horvath91c66712017-12-20 12:22:16 +0000864 } else if (const auto *IneffectiveOperator =
865 Result.Nodes.getNodeAs<BinaryOperator>(
866 "ineffective-bitwise")) {
867 APSInt Value;
868 const Expr *Sym = nullptr, *ConstExpr = nullptr;
869
870 if (!retrieveSymbolicExpr(Result, "ineffective-bitwise", Sym) ||
871 !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value,
872 ConstExpr))
873 return;
874
875 if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
876 return;
877
878 SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
879
880 BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode();
881 if (exprEvaluatesToZero(Opcode, Value)) {
882 diag(Loc, "expression always evaluates to 0");
883 } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) {
884 SourceRange ConstExprRange(ConstExpr->getLocStart(),
885 ConstExpr->getLocEnd());
886 StringRef ConstExprText = Lexer::getSourceText(
887 CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager,
888 Result.Context->getLangOpts());
889
890 diag(Loc, "expression always evaluates to '%0'") << ConstExprText;
891
892 } else if (exprEvaluatesToSymbolic(Opcode, Value)) {
893 SourceRange SymExprRange(Sym->getLocStart(), Sym->getLocEnd());
894
895 StringRef ExprText = Lexer::getSourceText(
896 CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager,
897 Result.Context->getLangOpts());
898
899 diag(Loc, "expression always evaluates to '%0'") << ExprText;
900 }
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000901 }
902}
903
904void RedundantExpressionCheck::checkRelationalExpr(
905 const MatchFinder::MatchResult &Result) {
906 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
907 "comparisons-of-symbol-and-const")) {
908 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
Gabor Horvathec87e172017-11-07 13:17:58 +0000909 // E.g.: (X < 2) && (X > 4)
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000910 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
911
912 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000913 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
Gabor Horvathec87e172017-11-07 13:17:58 +0000914 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000915 BinaryOperatorKind LhsOpcode, RhsOpcode;
Gabor Horvathec87e172017-11-07 13:17:58 +0000916 APSInt LhsValue, RhsValue;
917
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000918 if (!retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000919 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000920 !retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000921 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000922 !areEquivalentExpr(LhsSymbol, RhsSymbol))
923 return;
924
Gabor Horvathec87e172017-11-07 13:17:58 +0000925 // Bring expr to a canonical form: smallest constant must be on the left.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000926 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
927 std::swap(LhsExpr, RhsExpr);
928 std::swap(LhsValue, RhsValue);
929 std::swap(LhsSymbol, RhsSymbol);
930 std::swap(LhsOpcode, RhsOpcode);
931 }
932
Gabor Horvathec87e172017-11-07 13:17:58 +0000933 // Constants come from two different macros, or one of them is a macro.
934 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
935 areExprsMacroAndNonMacro(LhsConst, RhsConst))
936 return;
937
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000938 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
939 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
940 diag(ComparisonOperator->getOperatorLoc(),
Gabor Horvathec87e172017-11-07 13:17:58 +0000941 "equivalent expression on both sides of logical operator");
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000942 return;
943 }
944
945 if (Opcode == BO_LAnd) {
946 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
947 diag(ComparisonOperator->getOperatorLoc(),
948 "logical expression is always false");
949 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
950 diag(LhsExpr->getExprLoc(), "expression is redundant");
951 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
952 diag(RhsExpr->getExprLoc(), "expression is redundant");
953 }
954 }
955
956 if (Opcode == BO_LOr) {
957 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
958 diag(ComparisonOperator->getOperatorLoc(),
959 "logical expression is always true");
960 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
961 diag(RhsExpr->getExprLoc(), "expression is redundant");
962 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
963 diag(LhsExpr->getExprLoc(), "expression is redundant");
964 }
965 }
966 }
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000967}
968
969void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000970 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000971 // If the expression's constants are macros, check whether they are
972 // intentional.
973 if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
974 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
975 BinaryOperatorKind MainOpcode, SideOpcode;
976
Gabor Horvath250c40d2017-11-27 15:05:24 +0000977 if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
978 LhsConst, RhsConst, Result.Context))
979 return;
Gabor Horvathec87e172017-11-07 13:17:58 +0000980
981 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
982 areExprsMacroAndNonMacro(LhsConst, RhsConst))
983 return;
984 }
985
986 diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
987 }
988
989 if (const auto *CondOp =
990 Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
991 const Expr *TrueExpr = CondOp->getTrueExpr();
992 const Expr *FalseExpr = CondOp->getFalseExpr();
993
994 if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
995 areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
996 return;
997 diag(CondOp->getColonLoc(),
998 "'true' and 'false' expressions are equivalent");
999 }
1000
1001 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
Gabor Horvath250c40d2017-11-27 15:05:24 +00001002 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
1003 if (!OverloadedFunctionDecl)
1004 return;
1005
1006 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, true))
1007 return;
1008
Gabor Horvathec87e172017-11-07 13:17:58 +00001009 diag(Call->getOperatorLoc(),
1010 "both sides of overloaded operator are equivalent");
1011 }
1012
Gabor Horvath91c66712017-12-20 12:22:16 +00001013 if (const auto *NegateOperator =
1014 Result.Nodes.getNodeAs<UnaryOperator>("logical-bitwise-confusion")) {
1015 SourceLocation OperatorLoc = NegateOperator->getOperatorLoc();
1016
1017 auto Diag =
1018 diag(OperatorLoc,
1019 "ineffective logical negation operator used; did you mean '~'?");
1020 SourceLocation LogicalNotLocation = OperatorLoc.getLocWithOffset(1);
1021
1022 if (!LogicalNotLocation.isMacroID())
1023 Diag << FixItHint::CreateReplacement(
1024 CharSourceRange::getCharRange(OperatorLoc, LogicalNotLocation), "~");
1025 }
1026
1027 if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs<BinaryOperator>(
1028 "left-right-shift-confusion")) {
1029 const auto *ShiftingConst = Result.Nodes.getNodeAs<Expr>("shift-const");
1030 assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!");
1031 APSInt ShiftingValue;
1032
1033 if (!ShiftingConst->isIntegerConstantExpr(ShiftingValue, *Result.Context))
1034 return;
1035
1036 const auto *AndConst = Result.Nodes.getNodeAs<Expr>("and-const");
1037 assert(AndConst && "Expr* 'AndCont' is nullptr!");
1038 APSInt AndValue;
1039 if (!AndConst->isIntegerConstantExpr(AndValue, *Result.Context))
1040 return;
1041
1042 // If ShiftingConst is shifted left with more bits than the position of the
1043 // leftmost 1 in the bit representation of AndValue, AndConstant is
1044 // ineffective.
Benjamin Kramer10db8d62018-02-02 13:23:21 +00001045 if (AndValue.getActiveBits() > ShiftingValue)
Gabor Horvath91c66712017-12-20 12:22:16 +00001046 return;
1047
1048 auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
Alexander Kornienko396fc872018-02-01 16:39:12 +00001049 "ineffective bitwise and operation");
Gabor Horvath91c66712017-12-20 12:22:16 +00001050 }
1051
Gabor Horvathec87e172017-11-07 13:17:58 +00001052 // Check for the following bound expressions:
1053 // - "binop-const-compare-to-sym",
1054 // - "binop-const-compare-to-binop-const",
1055 // Produced message:
1056 // -> "logical expression is always false/true"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001057 checkArithmeticExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001058
1059 // Check for the following bound expression:
1060 // - "binop-const-compare-to-const",
Gabor Horvath91c66712017-12-20 12:22:16 +00001061 // - "ineffective-bitwise"
Gabor Horvathec87e172017-11-07 13:17:58 +00001062 // Produced message:
1063 // -> "logical expression is always false/true"
Gabor Horvath91c66712017-12-20 12:22:16 +00001064 // -> "expression always evaluates to ..."
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001065 checkBitwiseExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001066
1067 // Check for te following bound expression:
1068 // - "comparisons-of-symbol-and-const",
1069 // Produced messages:
1070 // -> "equivalent expression on both sides of logical operator",
1071 // -> "logical expression is always false/true"
1072 // -> "expression is redundant"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001073 checkRelationalExpr(Result);
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001074}
1075
1076} // namespace misc
1077} // namespace tidy
1078} // namespace clang