blob: 3cec1fb935d26f5c9365c07c4c651beac680ea97 [file] [log] [blame]
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001//===--- RedundantExpressionCheck.cpp - clang-tidy-------------------------===//
2//
Chandler Carruth2946cd72019-01-19 08:50:56 +00003// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4// See https://llvm.org/LICENSE.txt for license information.
5// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
Etienne Bergeronbda187d2016-04-26 17:30:30 +00006//
7//===----------------------------------------------------------------------===//
8
9#include "RedundantExpressionCheck.h"
10#include "../utils/Matchers.h"
Etienne Bergeronc87599f2016-05-12 04:32:47 +000011#include "../utils/OptionsUtils.h"
Etienne Bergeronbda187d2016-04-26 17:30:30 +000012#include "clang/AST/ASTContext.h"
13#include "clang/ASTMatchers/ASTMatchFinder.h"
Eugene Zelenko90c117a2016-11-01 18:33:50 +000014#include "clang/Basic/LLVM.h"
15#include "clang/Basic/SourceLocation.h"
16#include "clang/Basic/SourceManager.h"
Etienne Bergeronc87599f2016-05-12 04:32:47 +000017#include "clang/Lex/Lexer.h"
Eugene Zelenko90c117a2016-11-01 18:33:50 +000018#include "llvm/ADT/APInt.h"
19#include "llvm/ADT/APSInt.h"
20#include "llvm/ADT/FoldingSet.h"
21#include "llvm/Support/Casting.h"
22#include <algorithm>
23#include <cassert>
24#include <cstdint>
Eugene Zelenko90c117a2016-11-01 18:33:50 +000025#include <string>
26#include <vector>
Etienne Bergeronbda187d2016-04-26 17:30:30 +000027
28using namespace clang::ast_matchers;
Etienne Bergeron00639bc2016-07-07 04:03:05 +000029using namespace clang::tidy::matchers;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000030
31namespace clang {
32namespace tidy {
33namespace misc {
Etienne Bergeron00639bc2016-07-07 04:03:05 +000034namespace {
35using llvm::APSInt;
Etienne Bergeron00639bc2016-07-07 04:03:05 +000036
Benjamin Kramer492f1cc2018-02-02 13:23:24 +000037static constexpr llvm::StringLiteral KnownBannedMacroNames[] = {
38 "EAGAIN",
39 "EWOULDBLOCK",
40 "SIGCLD",
41 "SIGCHLD",
42};
Etienne Bergeronc87599f2016-05-12 04:32:47 +000043
Etienne Bergeron00639bc2016-07-07 04:03:05 +000044static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
45 Result = Value;
46 ++Result;
47 return Value < Result;
48}
49
Etienne Bergeronc87599f2016-05-12 04:32:47 +000050static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
51 const NestedNameSpecifier *Right) {
52 llvm::FoldingSetNodeID LeftID, RightID;
53 Left->Profile(LeftID);
54 Right->Profile(RightID);
55 return LeftID == RightID;
56}
57
58static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
Etienne Bergeronbda187d2016-04-26 17:30:30 +000059 if (!Left || !Right)
60 return !Left && !Right;
61
62 Left = Left->IgnoreParens();
63 Right = Right->IgnoreParens();
64
65 // Compare classes.
66 if (Left->getStmtClass() != Right->getStmtClass())
67 return false;
68
69 // Compare children.
70 Expr::const_child_iterator LeftIter = Left->child_begin();
71 Expr::const_child_iterator RightIter = Right->child_begin();
72 while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
Etienne Bergeronc87599f2016-05-12 04:32:47 +000073 if (!areEquivalentExpr(dyn_cast<Expr>(*LeftIter),
74 dyn_cast<Expr>(*RightIter)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +000075 return false;
76 ++LeftIter;
77 ++RightIter;
78 }
79 if (LeftIter != Left->child_end() || RightIter != Right->child_end())
80 return false;
81
82 // Perform extra checks.
83 switch (Left->getStmtClass()) {
84 default:
85 return false;
86
87 case Stmt::CharacterLiteralClass:
88 return cast<CharacterLiteral>(Left)->getValue() ==
89 cast<CharacterLiteral>(Right)->getValue();
90 case Stmt::IntegerLiteralClass: {
91 llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
92 llvm::APInt RightLit = cast<IntegerLiteral>(Right)->getValue();
Etienne Bergeronc87599f2016-05-12 04:32:47 +000093 return LeftLit.getBitWidth() == RightLit.getBitWidth() &&
94 LeftLit == RightLit;
Etienne Bergeronbda187d2016-04-26 17:30:30 +000095 }
96 case Stmt::FloatingLiteralClass:
97 return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
98 cast<FloatingLiteral>(Right)->getValue());
99 case Stmt::StringLiteralClass:
100 return cast<StringLiteral>(Left)->getBytes() ==
101 cast<StringLiteral>(Right)->getBytes();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000102 case Stmt::CXXOperatorCallExprClass:
103 return cast<CXXOperatorCallExpr>(Left)->getOperator() ==
104 cast<CXXOperatorCallExpr>(Right)->getOperator();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000105 case Stmt::DependentScopeDeclRefExprClass:
106 if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
107 cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
108 return false;
109 return areEquivalentNameSpecifier(
110 cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
111 cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000112 case Stmt::DeclRefExprClass:
113 return cast<DeclRefExpr>(Left)->getDecl() ==
114 cast<DeclRefExpr>(Right)->getDecl();
115 case Stmt::MemberExprClass:
116 return cast<MemberExpr>(Left)->getMemberDecl() ==
117 cast<MemberExpr>(Right)->getMemberDecl();
Gabor Horvathec87e172017-11-07 13:17:58 +0000118 case Stmt::CXXFunctionalCastExprClass:
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000119 case Stmt::CStyleCastExprClass:
Gabor Horvathec87e172017-11-07 13:17:58 +0000120 return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
121 cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000122 case Stmt::CallExprClass:
123 case Stmt::ImplicitCastExprClass:
124 case Stmt::ArraySubscriptExprClass:
125 return true;
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000126 case Stmt::UnaryOperatorClass:
127 if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
128 return false;
129 return cast<UnaryOperator>(Left)->getOpcode() ==
130 cast<UnaryOperator>(Right)->getOpcode();
131 case Stmt::BinaryOperatorClass:
132 return cast<BinaryOperator>(Left)->getOpcode() ==
133 cast<BinaryOperator>(Right)->getOpcode();
134 }
135}
136
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000137// For a given expression 'x', returns whether the ranges covered by the
138// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
139static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
140 const APSInt &ValueLHS,
141 BinaryOperatorKind OpcodeRHS,
142 const APSInt &ValueRHS) {
143 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
144 "Values must be ordered");
145 // Handle the case where constants are the same: x <= 4 <==> x <= 4.
146 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
147 return OpcodeLHS == OpcodeRHS;
148
149 // Handle the case where constants are off by one: x <= 4 <==> x < 5.
150 APSInt ValueLHS_plus1;
151 return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
152 (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
153 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
154 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0;
155}
156
157// For a given expression 'x', returns whether the ranges covered by the
158// relational operators are fully disjoint (i.e. x < 4 and x > 7).
159static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
160 const APSInt &ValueLHS,
161 BinaryOperatorKind OpcodeRHS,
162 const APSInt &ValueRHS) {
163 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
164 "Values must be ordered");
165
166 // Handle cases where the constants are the same.
167 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
168 switch (OpcodeLHS) {
169 case BO_EQ:
170 return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
171 case BO_NE:
172 return OpcodeRHS == BO_EQ;
173 case BO_LE:
174 return OpcodeRHS == BO_GT;
175 case BO_GE:
176 return OpcodeRHS == BO_LT;
177 case BO_LT:
178 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
179 case BO_GT:
180 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
181 default:
182 return false;
183 }
184 }
185
186 // Handle cases where the constants are different.
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000187 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LT || OpcodeLHS == BO_LE) &&
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000188 (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
189 return true;
190
191 // Handle the case where constants are off by one: x > 5 && x < 6.
192 APSInt ValueLHS_plus1;
193 if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
194 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
195 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
196 return true;
197
198 return false;
199}
200
201// Returns whether the ranges covered by the union of both relational
Gabor Horvath91c66712017-12-20 12:22:16 +0000202// expressions cover the whole domain (i.e. x < 10 and x > 0).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000203static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
204 const APSInt &ValueLHS,
205 BinaryOperatorKind OpcodeRHS,
206 const APSInt &ValueRHS) {
207 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
208 "Values must be ordered");
209
210 // Handle cases where the constants are the same: x < 5 || x >= 5.
211 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
212 switch (OpcodeLHS) {
213 case BO_EQ:
214 return OpcodeRHS == BO_NE;
215 case BO_NE:
216 return OpcodeRHS == BO_EQ;
217 case BO_LE:
218 return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
219 case BO_LT:
220 return OpcodeRHS == BO_GE;
221 case BO_GE:
222 return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
223 case BO_GT:
224 return OpcodeRHS == BO_LE;
225 default:
226 return false;
227 }
228 }
229
230 // Handle the case where constants are off by one: x <= 4 || x >= 5.
231 APSInt ValueLHS_plus1;
232 if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
233 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
234 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
235 return true;
236
237 // Handle cases where the constants are different: x > 4 || x <= 7.
238 if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
239 (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
240 return true;
241
Alexander Kornienkoc3acd2e2017-03-23 15:13:54 +0000242 // Handle cases where constants are different but both ops are !=, like:
243 // x != 5 || x != 10
244 if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE)
245 return true;
246
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000247 return false;
248}
249
250static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
251 const APSInt &ValueLHS,
252 BinaryOperatorKind OpcodeRHS,
253 const APSInt &ValueRHS) {
254 int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
255 switch (OpcodeLHS) {
256 case BO_EQ:
257 return OpcodeRHS == BO_EQ && Comparison == 0;
258 case BO_NE:
259 return (OpcodeRHS == BO_NE && Comparison == 0) ||
260 (OpcodeRHS == BO_EQ && Comparison != 0) ||
261 (OpcodeRHS == BO_LT && Comparison >= 0) ||
262 (OpcodeRHS == BO_LE && Comparison > 0) ||
263 (OpcodeRHS == BO_GT && Comparison <= 0) ||
264 (OpcodeRHS == BO_GE && Comparison < 0);
265
266 case BO_LT:
267 return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
268 (OpcodeRHS == BO_LE && Comparison > 0) ||
269 (OpcodeRHS == BO_EQ && Comparison > 0));
270 case BO_GT:
271 return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
272 (OpcodeRHS == BO_GE && Comparison < 0) ||
273 (OpcodeRHS == BO_EQ && Comparison < 0));
274 case BO_LE:
275 return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
276 Comparison >= 0;
277 case BO_GE:
278 return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
279 Comparison <= 0;
280 default:
281 return false;
282 }
283}
284
Gabor Horvathec87e172017-11-07 13:17:58 +0000285static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
286 APSInt &Value) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000287 if (Opcode == BO_Sub) {
288 Opcode = BO_Add;
289 Value = -Value;
290 }
291}
292
293AST_MATCHER(Expr, isIntegerConstantExpr) {
Haojian Wua4f5a2a2019-06-06 13:43:38 +0000294 if (Node.isInstantiationDependent())
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000295 return false;
296 return Node.isIntegerConstantExpr(Finder->getASTContext());
297}
298
Gabor Horvathec87e172017-11-07 13:17:58 +0000299AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
300 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
301}
302
303AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
304 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
305}
306
307AST_MATCHER(CallExpr, parametersAreEquivalent) {
308 return Node.getNumArgs() == 2 &&
309 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
310}
311
312AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
313 return Node.getOperatorLoc().isMacroID();
314}
315
316AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
317 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
318}
319
320AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
321
Benjamin Kramer492f1cc2018-02-02 13:23:24 +0000322AST_MATCHER_P(Expr, expandedByMacro, ArrayRef<llvm::StringLiteral>, Names) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000323 const SourceManager &SM = Finder->getASTContext().getSourceManager();
324 const LangOptions &LO = Finder->getASTContext().getLangOpts();
325 SourceLocation Loc = Node.getExprLoc();
326 while (Loc.isMacroID()) {
327 StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
Benjamin Kramer492f1cc2018-02-02 13:23:24 +0000328 if (llvm::is_contained(Names, MacroName))
Gabor Horvathec87e172017-11-07 13:17:58 +0000329 return true;
330 Loc = SM.getImmediateMacroCallerLoc(Loc);
331 }
332 return false;
333}
334
335// Returns a matcher for integer constant expressions.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000336static ast_matchers::internal::Matcher<Expr>
337matchIntegerConstantExpr(StringRef Id) {
338 std::string CstId = (Id + "-const").str();
339 return expr(isIntegerConstantExpr()).bind(CstId);
340}
341
Gabor Horvathec87e172017-11-07 13:17:58 +0000342// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
343// name 'Id' and stores it into 'ConstExpr', the value of the expression is
344// stored into `Value`.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000345static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
Gabor Horvathec87e172017-11-07 13:17:58 +0000346 StringRef Id, APSInt &Value,
347 const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000348 std::string CstId = (Id + "-const").str();
Gabor Horvathec87e172017-11-07 13:17:58 +0000349 ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
350 return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000351}
352
Gabor Horvathec87e172017-11-07 13:17:58 +0000353// Overloaded `retrieveIntegerConstantExpr` for compatibility.
354static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
355 StringRef Id, APSInt &Value) {
356 const Expr *ConstExpr = nullptr;
357 return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
358}
359
360// Returns a matcher for symbolic expressions (matches every expression except
361// ingeter constant expressions).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000362static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
363 std::string SymId = (Id + "-sym").str();
364 return ignoringParenImpCasts(
365 expr(unless(isIntegerConstantExpr())).bind(SymId));
366}
367
Gabor Horvathec87e172017-11-07 13:17:58 +0000368// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
369// stores it into 'SymExpr'.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000370static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
371 StringRef Id, const Expr *&SymExpr) {
372 std::string SymId = (Id + "-sym").str();
373 if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
374 SymExpr = Node;
375 return true;
376 }
377 return false;
378}
379
380// Match a binary operator between a symbolic expression and an integer constant
381// expression.
382static ast_matchers::internal::Matcher<Expr>
383matchBinOpIntegerConstantExpr(StringRef Id) {
384 const auto BinOpCstExpr =
385 expr(
386 anyOf(binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("|"),
387 hasOperatorName("&")),
388 hasEitherOperand(matchSymbolicExpr(Id)),
389 hasEitherOperand(matchIntegerConstantExpr(Id))),
390 binaryOperator(hasOperatorName("-"),
391 hasLHS(matchSymbolicExpr(Id)),
392 hasRHS(matchIntegerConstantExpr(Id)))))
393 .bind(Id);
394 return ignoringParenImpCasts(BinOpCstExpr);
395}
396
Gabor Horvathec87e172017-11-07 13:17:58 +0000397// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000398// name 'Id'.
399static bool
400retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
401 StringRef Id, BinaryOperatorKind &Opcode,
402 const Expr *&Symbol, APSInt &Value) {
403 if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
404 Opcode = BinExpr->getOpcode();
405 return retrieveSymbolicExpr(Result, Id, Symbol) &&
406 retrieveIntegerConstantExpr(Result, Id, Value);
407 }
408 return false;
409}
410
Gabor Horvathec87e172017-11-07 13:17:58 +0000411// Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000412static ast_matchers::internal::Matcher<Expr>
413matchRelationalIntegerConstantExpr(StringRef Id) {
414 std::string CastId = (Id + "-cast").str();
415 std::string SwapId = (Id + "-swap").str();
416 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000417 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000418
419 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
420 isComparisonOperator(), expr().bind(Id),
421 anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
422 hasRHS(matchIntegerConstantExpr(Id))),
423 allOf(hasLHS(matchIntegerConstantExpr(Id)),
424 hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
425
426 // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
427 // to if (x != 0)).
428 const auto CastExpr =
429 implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
430 hasSourceExpression(matchSymbolicExpr(Id)))
431 .bind(CastId);
432
433 const auto NegateRelationalExpr =
434 unaryOperator(hasOperatorName("!"),
435 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
436 .bind(NegateId);
437
Gabor Horvathec87e172017-11-07 13:17:58 +0000438 // Do not bind to double negation.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000439 const auto NegateNegateRelationalExpr =
440 unaryOperator(hasOperatorName("!"),
441 hasUnaryOperand(unaryOperator(
442 hasOperatorName("!"),
443 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
444
Gabor Horvath250c40d2017-11-27 15:05:24 +0000445 const auto OverloadedOperatorExpr =
446 cxxOperatorCallExpr(
447 anyOf(hasOverloadedOperatorName("=="),
448 hasOverloadedOperatorName("!="), hasOverloadedOperatorName("<"),
449 hasOverloadedOperatorName("<="), hasOverloadedOperatorName(">"),
450 hasOverloadedOperatorName(">=")),
451 // Filter noisy false positives.
452 unless(isMacro()), unless(isInTemplateInstantiation()))
453 .bind(OverloadId);
454
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000455 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
Gabor Horvath250c40d2017-11-27 15:05:24 +0000456 NegateNegateRelationalExpr, OverloadedOperatorExpr);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000457}
458
Gabor Horvath250c40d2017-11-27 15:05:24 +0000459// Checks whether a function param is non constant reference type, and may
460// be modified in the function.
461static bool isNonConstReferenceType(QualType ParamType) {
462 return ParamType->isReferenceType() &&
463 !ParamType.getNonReferenceType().isConstQualified();
464}
465
466// Checks whether the arguments of an overloaded operator can be modified in the
467// function.
468// For operators that take an instance and a constant as arguments, only the
469// first argument (the instance) needs to be checked, since the constant itself
470// is a temporary expression. Whether the second parameter is checked is
471// controlled by the parameter `ParamsToCheckCount`.
472static bool
473canOverloadedOperatorArgsBeModified(const FunctionDecl *OperatorDecl,
474 bool checkSecondParam) {
475 unsigned ParamCount = OperatorDecl->getNumParams();
476
477 // Overloaded operators declared inside a class have only one param.
478 // These functions must be declared const in order to not be able to modify
479 // the instance of the class they are called through.
480 if (ParamCount == 1 &&
481 !OperatorDecl->getType()->getAs<FunctionType>()->isConst())
482 return true;
483
484 if (isNonConstReferenceType(OperatorDecl->getParamDecl(0)->getType()))
485 return true;
486
487 return checkSecondParam && ParamCount == 2 &&
488 isNonConstReferenceType(OperatorDecl->getParamDecl(1)->getType());
489}
490
491// Retrieves sub-expressions matched by 'matchRelationalIntegerConstantExpr'
492// with name 'Id'.
Gabor Horvathec87e172017-11-07 13:17:58 +0000493static bool retrieveRelationalIntegerConstantExpr(
494 const MatchFinder::MatchResult &Result, StringRef Id,
495 const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
496 APSInt &Value, const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000497 std::string CastId = (Id + "-cast").str();
498 std::string SwapId = (Id + "-swap").str();
499 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000500 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000501
502 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
503 // Operand received with explicit comparator.
504 Opcode = Bin->getOpcode();
505 OperandExpr = Bin;
Gabor Horvathec87e172017-11-07 13:17:58 +0000506
507 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000508 return false;
509 } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
510 // Operand received with implicit comparator (cast).
511 Opcode = BO_NE;
512 OperandExpr = Cast;
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000513 Value = APSInt(32, false);
Gabor Horvath250c40d2017-11-27 15:05:24 +0000514 } else if (const auto *OverloadedOperatorExpr =
515 Result.Nodes.getNodeAs<CXXOperatorCallExpr>(OverloadId)) {
516 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
517 if (!OverloadedFunctionDecl)
518 return false;
519
520 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
521 return false;
522
Gabor Horvath91c66712017-12-20 12:22:16 +0000523 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
524 return false;
525
Haojian Wua4f5a2a2019-06-06 13:43:38 +0000526 if (const auto *Arg = OverloadedOperatorExpr->getArg(1)) {
527 if (!Arg->isValueDependent() &&
528 !Arg->isIntegerConstantExpr(Value, *Result.Context))
529 return false;
530 }
Gabor Horvath250c40d2017-11-27 15:05:24 +0000531 Symbol = OverloadedOperatorExpr->getArg(0);
532 OperandExpr = OverloadedOperatorExpr;
533 Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
534
535 return BinaryOperator::isComparisonOp(Opcode);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000536 } else {
537 return false;
538 }
539
540 if (!retrieveSymbolicExpr(Result, Id, Symbol))
541 return false;
542
543 if (Result.Nodes.getNodeAs<Expr>(SwapId))
544 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
545 if (Result.Nodes.getNodeAs<Expr>(NegateId))
546 Opcode = BinaryOperator::negateComparisonOp(Opcode);
Gabor Horvathec87e172017-11-07 13:17:58 +0000547 return true;
548}
549
550// Checks for expressions like (X == 4) && (Y != 9)
551static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
552 const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
553 const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
554
555 if (!LhsBinOp || !RhsBinOp)
556 return false;
557
Dmitri Gribenkocf7d7682019-06-12 08:40:53 +0000558 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
559 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
560 };
561
562 if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
563 IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
564 (IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
565 IsIntegerConstantExpr(RhsBinOp->getRHS())))
Gabor Horvathec87e172017-11-07 13:17:58 +0000566 return true;
567 return false;
568}
569
570// Retrieves integer constant subexpressions from binary operator expressions
Gabor Horvath91c66712017-12-20 12:22:16 +0000571// that have two equivalent sides.
Gabor Horvathec87e172017-11-07 13:17:58 +0000572// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
573static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
574 BinaryOperatorKind &MainOpcode,
575 BinaryOperatorKind &SideOpcode,
576 const Expr *&LhsConst,
577 const Expr *&RhsConst,
578 const ASTContext *AstCtx) {
579 assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
580 "Both sides of binary operator must be constant expressions!");
581
582 MainOpcode = BinOp->getOpcode();
583
584 const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
585 const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
586
Dmitri Gribenkocf7d7682019-06-12 08:40:53 +0000587 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
588 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
589 };
590
591 LhsConst = IsIntegerConstantExpr(BinOpLhs->getLHS()) ? BinOpLhs->getLHS()
592 : BinOpLhs->getRHS();
593 RhsConst = IsIntegerConstantExpr(BinOpRhs->getLHS()) ? BinOpRhs->getLHS()
594 : BinOpRhs->getRHS();
Gabor Horvathec87e172017-11-07 13:17:58 +0000595
596 if (!LhsConst || !RhsConst)
597 return false;
598
599 assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
600 "Sides of the binary operator must be equivalent expressions!");
601
602 SideOpcode = BinOpLhs->getOpcode();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000603
604 return true;
605}
606
Gabor Horvathec87e172017-11-07 13:17:58 +0000607static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
608 const Expr *RhsExpr,
609 const ASTContext *AstCtx) {
610 if (!LhsExpr || !RhsExpr)
611 return false;
612
613 SourceLocation LhsLoc = LhsExpr->getExprLoc();
614 SourceLocation RhsLoc = RhsExpr->getExprLoc();
615
616 if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
617 return false;
618
619 const SourceManager &SM = AstCtx->getSourceManager();
620 const LangOptions &LO = AstCtx->getLangOpts();
621
622 return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
623 Lexer::getImmediateMacroName(RhsLoc, SM, LO));
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000624}
625
Gabor Horvath250c40d2017-11-27 15:05:24 +0000626static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
627 const Expr *&RhsExpr) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000628 if (!LhsExpr || !RhsExpr)
629 return false;
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000630
Gabor Horvathec87e172017-11-07 13:17:58 +0000631 SourceLocation LhsLoc = LhsExpr->getExprLoc();
632 SourceLocation RhsLoc = RhsExpr->getExprLoc();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000633
Gabor Horvathec87e172017-11-07 13:17:58 +0000634 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000635}
Benjamin Kramera9bef622018-02-02 13:39:00 +0000636} // namespace
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000637
638void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
639 const auto AnyLiteralExpr = ignoringParenImpCasts(
640 anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
641
Gabor Horvath250c40d2017-11-27 15:05:24 +0000642 const auto BannedIntegerLiteral =
643 integerLiteral(expandedByMacro(KnownBannedMacroNames));
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000644
Gabor Horvathec87e172017-11-07 13:17:58 +0000645 // Binary with equivalent operands, like (X != 2 && X != 2).
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000646 Finder->addMatcher(
647 binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
648 hasOperatorName("%"), hasOperatorName("|"),
649 hasOperatorName("&"), hasOperatorName("^"),
650 matchers::isComparisonOperator(),
651 hasOperatorName("&&"), hasOperatorName("||"),
652 hasOperatorName("=")),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000653 operandsAreEquivalent(),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000654 // Filter noisy false positives.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000655 unless(isInTemplateInstantiation()),
656 unless(binaryOperatorIsInMacro()),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000657 unless(hasType(realFloatingPointType())),
658 unless(hasEitherOperand(hasType(realFloatingPointType()))),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000659 unless(hasLHS(AnyLiteralExpr)),
660 unless(hasDescendant(BannedIntegerLiteral)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000661 .bind("binary"),
662 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000663
Gabor Horvathec87e172017-11-07 13:17:58 +0000664 // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
Gabor Horvath250c40d2017-11-27 15:05:24 +0000665 Finder->addMatcher(conditionalOperator(expressionsAreEquivalent(),
666 // Filter noisy false positives.
667 unless(conditionalOperatorIsInMacro()),
668 unless(isInTemplateInstantiation()))
669 .bind("cond"),
670 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000671
Gabor Horvathec87e172017-11-07 13:17:58 +0000672 // Overloaded operators with equivalent operands.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000673 Finder->addMatcher(
674 cxxOperatorCallExpr(
675 anyOf(
676 hasOverloadedOperatorName("-"), hasOverloadedOperatorName("/"),
677 hasOverloadedOperatorName("%"), hasOverloadedOperatorName("|"),
678 hasOverloadedOperatorName("&"), hasOverloadedOperatorName("^"),
679 hasOverloadedOperatorName("=="), hasOverloadedOperatorName("!="),
680 hasOverloadedOperatorName("<"), hasOverloadedOperatorName("<="),
681 hasOverloadedOperatorName(">"), hasOverloadedOperatorName(">="),
682 hasOverloadedOperatorName("&&"), hasOverloadedOperatorName("||"),
683 hasOverloadedOperatorName("=")),
684 parametersAreEquivalent(),
685 // Filter noisy false positives.
686 unless(isMacro()), unless(isInTemplateInstantiation()))
687 .bind("call"),
688 this);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000689
Gabor Horvath91c66712017-12-20 12:22:16 +0000690 // Match expressions like: !(1 | 2 | 3)
691 Finder->addMatcher(
692 implicitCastExpr(
693 hasImplicitDestinationType(isInteger()),
694 has(unaryOperator(
695 hasOperatorName("!"),
696 hasUnaryOperand(ignoringParenImpCasts(binaryOperator(
697 anyOf(hasOperatorName("|"), hasOperatorName("&")),
698 hasLHS(anyOf(binaryOperator(anyOf(hasOperatorName("|"),
699 hasOperatorName("&"))),
700 integerLiteral())),
701 hasRHS(integerLiteral())))))
702 .bind("logical-bitwise-confusion"))),
703 this);
704
705 // Match expressions like: (X << 8) & 0xFF
706 Finder->addMatcher(
707 binaryOperator(hasOperatorName("&"),
708 hasEitherOperand(ignoringParenImpCasts(binaryOperator(
709 hasOperatorName("<<"),
710 hasRHS(ignoringParenImpCasts(
711 integerLiteral().bind("shift-const")))))),
712 hasEitherOperand(ignoringParenImpCasts(
713 integerLiteral().bind("and-const"))))
714 .bind("left-right-shift-confusion"),
715 this);
716
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000717 // Match common expressions and apply more checks to find redundant
718 // sub-expressions.
719 // a) Expr <op> K1 == K2
720 // b) Expr <op> K1 == Expr
721 // c) Expr <op> K1 == Expr <op> K2
722 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
723 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
724 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
725 const auto CstRight = matchIntegerConstantExpr("rhs");
726 const auto SymRight = matchSymbolicExpr("rhs");
727
728 // Match expressions like: x <op> 0xFF == 0xF00.
729 Finder->addMatcher(binaryOperator(isComparisonOperator(),
730 hasEitherOperand(BinOpCstLeft),
731 hasEitherOperand(CstRight))
732 .bind("binop-const-compare-to-const"),
733 this);
734
735 // Match expressions like: x <op> 0xFF == x.
736 Finder->addMatcher(
737 binaryOperator(isComparisonOperator(),
738 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
739 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
740 .bind("binop-const-compare-to-sym"),
741 this);
742
743 // Match expressions like: x <op> 10 == x <op> 12.
744 Finder->addMatcher(binaryOperator(isComparisonOperator(),
745 hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
746 // Already reported as redundant.
747 unless(operandsAreEquivalent()))
748 .bind("binop-const-compare-to-binop-const"),
749 this);
750
751 // Match relational expressions combined with logical operators and find
752 // redundant sub-expressions.
753 // see: 'checkRelationalExpr'
754
755 // Match expressions like: x < 2 && x > 2.
756 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
757 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
758 Finder->addMatcher(
759 binaryOperator(anyOf(hasOperatorName("||"), hasOperatorName("&&")),
760 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
761 // Already reported as redundant.
762 unless(operandsAreEquivalent()))
763 .bind("comparisons-of-symbol-and-const"),
764 this);
765}
766
767void RedundantExpressionCheck::checkArithmeticExpr(
768 const MatchFinder::MatchResult &Result) {
769 APSInt LhsValue, RhsValue;
770 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
771 BinaryOperatorKind LhsOpcode, RhsOpcode;
772
773 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
774 "binop-const-compare-to-sym")) {
775 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
776 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
777 LhsValue) ||
778 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
779 !areEquivalentExpr(LhsSymbol, RhsSymbol))
780 return;
781
782 // Check expressions: x + k == x or x - k == x.
783 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
784 if ((LhsValue != 0 && Opcode == BO_EQ) ||
785 (LhsValue == 0 && Opcode == BO_NE))
786 diag(ComparisonOperator->getOperatorLoc(),
787 "logical expression is always false");
788 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
789 (LhsValue != 0 && Opcode == BO_NE))
790 diag(ComparisonOperator->getOperatorLoc(),
791 "logical expression is always true");
792 }
793 } else if (const auto *ComparisonOperator =
794 Result.Nodes.getNodeAs<BinaryOperator>(
795 "binop-const-compare-to-binop-const")) {
796 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
797
798 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
799 LhsValue) ||
800 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
801 RhsValue) ||
802 !areEquivalentExpr(LhsSymbol, RhsSymbol))
803 return;
804
Gabor Horvathec87e172017-11-07 13:17:58 +0000805 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
806 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000807
808 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
809 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
810 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
811 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
812 diag(ComparisonOperator->getOperatorLoc(),
813 "logical expression is always true");
814 } else if ((Opcode == BO_EQ &&
815 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
816 (Opcode == BO_NE &&
817 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
818 diag(ComparisonOperator->getOperatorLoc(),
819 "logical expression is always false");
820 }
821 }
822 }
823}
824
Gabor Horvath91c66712017-12-20 12:22:16 +0000825static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value) {
826 return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
827}
828
829static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
830 APSInt Value) {
831 return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
832}
833
834static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value) {
835 return ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) ||
836 ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
837}
838
839
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000840void RedundantExpressionCheck::checkBitwiseExpr(
841 const MatchFinder::MatchResult &Result) {
842 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
843 "binop-const-compare-to-const")) {
844 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
845
846 APSInt LhsValue, RhsValue;
847 const Expr *LhsSymbol = nullptr;
848 BinaryOperatorKind LhsOpcode;
849 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
850 LhsValue) ||
851 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
852 return;
853
854 uint64_t LhsConstant = LhsValue.getZExtValue();
855 uint64_t RhsConstant = RhsValue.getZExtValue();
856 SourceLocation Loc = ComparisonOperator->getOperatorLoc();
857
858 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
859 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
860 if (Opcode == BO_EQ)
861 diag(Loc, "logical expression is always false");
862 else if (Opcode == BO_NE)
863 diag(Loc, "logical expression is always true");
864 }
865
866 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
867 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
868 if (Opcode == BO_EQ)
869 diag(Loc, "logical expression is always false");
870 else if (Opcode == BO_NE)
871 diag(Loc, "logical expression is always true");
872 }
Gabor Horvath91c66712017-12-20 12:22:16 +0000873 } else if (const auto *IneffectiveOperator =
874 Result.Nodes.getNodeAs<BinaryOperator>(
875 "ineffective-bitwise")) {
876 APSInt Value;
877 const Expr *Sym = nullptr, *ConstExpr = nullptr;
878
879 if (!retrieveSymbolicExpr(Result, "ineffective-bitwise", Sym) ||
880 !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value,
881 ConstExpr))
882 return;
883
884 if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
885 return;
886
887 SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
888
889 BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode();
890 if (exprEvaluatesToZero(Opcode, Value)) {
891 diag(Loc, "expression always evaluates to 0");
892 } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) {
Stephen Kelly43465bf2018-08-09 22:42:26 +0000893 SourceRange ConstExprRange(ConstExpr->getBeginLoc(),
Stephen Kellyc09197e2018-08-09 22:43:02 +0000894 ConstExpr->getEndLoc());
Gabor Horvath91c66712017-12-20 12:22:16 +0000895 StringRef ConstExprText = Lexer::getSourceText(
896 CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager,
897 Result.Context->getLangOpts());
898
899 diag(Loc, "expression always evaluates to '%0'") << ConstExprText;
900
901 } else if (exprEvaluatesToSymbolic(Opcode, Value)) {
Stephen Kellyc09197e2018-08-09 22:43:02 +0000902 SourceRange SymExprRange(Sym->getBeginLoc(), Sym->getEndLoc());
Gabor Horvath91c66712017-12-20 12:22:16 +0000903
904 StringRef ExprText = Lexer::getSourceText(
905 CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager,
906 Result.Context->getLangOpts());
907
908 diag(Loc, "expression always evaluates to '%0'") << ExprText;
909 }
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000910 }
911}
912
913void RedundantExpressionCheck::checkRelationalExpr(
914 const MatchFinder::MatchResult &Result) {
915 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
916 "comparisons-of-symbol-and-const")) {
917 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
Gabor Horvathec87e172017-11-07 13:17:58 +0000918 // E.g.: (X < 2) && (X > 4)
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000919 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
920
921 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000922 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
Gabor Horvathec87e172017-11-07 13:17:58 +0000923 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000924 BinaryOperatorKind LhsOpcode, RhsOpcode;
Gabor Horvathec87e172017-11-07 13:17:58 +0000925 APSInt LhsValue, RhsValue;
926
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000927 if (!retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000928 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000929 !retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000930 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000931 !areEquivalentExpr(LhsSymbol, RhsSymbol))
932 return;
933
Gabor Horvathec87e172017-11-07 13:17:58 +0000934 // Bring expr to a canonical form: smallest constant must be on the left.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000935 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
936 std::swap(LhsExpr, RhsExpr);
937 std::swap(LhsValue, RhsValue);
938 std::swap(LhsSymbol, RhsSymbol);
939 std::swap(LhsOpcode, RhsOpcode);
940 }
941
Gabor Horvathec87e172017-11-07 13:17:58 +0000942 // Constants come from two different macros, or one of them is a macro.
943 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
944 areExprsMacroAndNonMacro(LhsConst, RhsConst))
945 return;
946
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000947 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
948 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
949 diag(ComparisonOperator->getOperatorLoc(),
Gabor Horvathec87e172017-11-07 13:17:58 +0000950 "equivalent expression on both sides of logical operator");
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000951 return;
952 }
953
954 if (Opcode == BO_LAnd) {
955 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
956 diag(ComparisonOperator->getOperatorLoc(),
957 "logical expression is always false");
958 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
959 diag(LhsExpr->getExprLoc(), "expression is redundant");
960 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
961 diag(RhsExpr->getExprLoc(), "expression is redundant");
962 }
963 }
964
965 if (Opcode == BO_LOr) {
966 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
967 diag(ComparisonOperator->getOperatorLoc(),
968 "logical expression is always true");
969 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
970 diag(RhsExpr->getExprLoc(), "expression is redundant");
971 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
972 diag(LhsExpr->getExprLoc(), "expression is redundant");
973 }
974 }
975 }
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000976}
977
978void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000979 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000980 // If the expression's constants are macros, check whether they are
981 // intentional.
982 if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
983 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
984 BinaryOperatorKind MainOpcode, SideOpcode;
985
Gabor Horvath250c40d2017-11-27 15:05:24 +0000986 if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
987 LhsConst, RhsConst, Result.Context))
988 return;
Gabor Horvathec87e172017-11-07 13:17:58 +0000989
990 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
991 areExprsMacroAndNonMacro(LhsConst, RhsConst))
992 return;
993 }
994
995 diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
996 }
997
998 if (const auto *CondOp =
999 Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
1000 const Expr *TrueExpr = CondOp->getTrueExpr();
1001 const Expr *FalseExpr = CondOp->getFalseExpr();
1002
1003 if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
1004 areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
1005 return;
1006 diag(CondOp->getColonLoc(),
1007 "'true' and 'false' expressions are equivalent");
1008 }
1009
1010 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
Gabor Horvath250c40d2017-11-27 15:05:24 +00001011 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
1012 if (!OverloadedFunctionDecl)
1013 return;
1014
1015 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, true))
1016 return;
1017
Gabor Horvathec87e172017-11-07 13:17:58 +00001018 diag(Call->getOperatorLoc(),
1019 "both sides of overloaded operator are equivalent");
1020 }
1021
Gabor Horvath91c66712017-12-20 12:22:16 +00001022 if (const auto *NegateOperator =
1023 Result.Nodes.getNodeAs<UnaryOperator>("logical-bitwise-confusion")) {
1024 SourceLocation OperatorLoc = NegateOperator->getOperatorLoc();
1025
1026 auto Diag =
1027 diag(OperatorLoc,
1028 "ineffective logical negation operator used; did you mean '~'?");
1029 SourceLocation LogicalNotLocation = OperatorLoc.getLocWithOffset(1);
1030
1031 if (!LogicalNotLocation.isMacroID())
1032 Diag << FixItHint::CreateReplacement(
1033 CharSourceRange::getCharRange(OperatorLoc, LogicalNotLocation), "~");
1034 }
1035
1036 if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs<BinaryOperator>(
1037 "left-right-shift-confusion")) {
1038 const auto *ShiftingConst = Result.Nodes.getNodeAs<Expr>("shift-const");
1039 assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!");
1040 APSInt ShiftingValue;
1041
1042 if (!ShiftingConst->isIntegerConstantExpr(ShiftingValue, *Result.Context))
1043 return;
1044
1045 const auto *AndConst = Result.Nodes.getNodeAs<Expr>("and-const");
1046 assert(AndConst && "Expr* 'AndCont' is nullptr!");
1047 APSInt AndValue;
1048 if (!AndConst->isIntegerConstantExpr(AndValue, *Result.Context))
1049 return;
1050
1051 // If ShiftingConst is shifted left with more bits than the position of the
1052 // leftmost 1 in the bit representation of AndValue, AndConstant is
1053 // ineffective.
Benjamin Kramer10db8d62018-02-02 13:23:21 +00001054 if (AndValue.getActiveBits() > ShiftingValue)
Gabor Horvath91c66712017-12-20 12:22:16 +00001055 return;
1056
1057 auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
Alexander Kornienko396fc872018-02-01 16:39:12 +00001058 "ineffective bitwise and operation");
Gabor Horvath91c66712017-12-20 12:22:16 +00001059 }
1060
Gabor Horvathec87e172017-11-07 13:17:58 +00001061 // Check for the following bound expressions:
1062 // - "binop-const-compare-to-sym",
1063 // - "binop-const-compare-to-binop-const",
1064 // Produced message:
1065 // -> "logical expression is always false/true"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001066 checkArithmeticExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001067
1068 // Check for the following bound expression:
1069 // - "binop-const-compare-to-const",
Gabor Horvath91c66712017-12-20 12:22:16 +00001070 // - "ineffective-bitwise"
Gabor Horvathec87e172017-11-07 13:17:58 +00001071 // Produced message:
1072 // -> "logical expression is always false/true"
Gabor Horvath91c66712017-12-20 12:22:16 +00001073 // -> "expression always evaluates to ..."
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001074 checkBitwiseExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001075
1076 // Check for te following bound expression:
1077 // - "comparisons-of-symbol-and-const",
1078 // Produced messages:
1079 // -> "equivalent expression on both sides of logical operator",
1080 // -> "logical expression is always false/true"
1081 // -> "expression is redundant"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001082 checkRelationalExpr(Result);
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001083}
1084
1085} // namespace misc
1086} // namespace tidy
1087} // namespace clang