blob: e3e84b71601b514fbd39067e94a9f9a833247478 [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();
Aaron Ballman1caa66d2019-10-30 13:35:20 -0400134 case Stmt::UnaryExprOrTypeTraitExprClass:
135 const auto *LeftUnaryExpr =
136 cast<UnaryExprOrTypeTraitExpr>(Left);
137 const auto *RightUnaryExpr =
138 cast<UnaryExprOrTypeTraitExpr>(Right);
139 if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
140 return LeftUnaryExpr->getArgumentType() ==
141 RightUnaryExpr->getArgumentType();
142 else if (!LeftUnaryExpr->isArgumentType() &&
143 !RightUnaryExpr->isArgumentType())
144 return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(),
145 RightUnaryExpr->getArgumentExpr());
146
147 return false;
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000148 }
149}
150
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000151// For a given expression 'x', returns whether the ranges covered by the
152// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
153static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
154 const APSInt &ValueLHS,
155 BinaryOperatorKind OpcodeRHS,
156 const APSInt &ValueRHS) {
157 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
158 "Values must be ordered");
159 // Handle the case where constants are the same: x <= 4 <==> x <= 4.
160 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
161 return OpcodeLHS == OpcodeRHS;
162
163 // Handle the case where constants are off by one: x <= 4 <==> x < 5.
164 APSInt ValueLHS_plus1;
165 return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
166 (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
167 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
168 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0;
169}
170
171// For a given expression 'x', returns whether the ranges covered by the
172// relational operators are fully disjoint (i.e. x < 4 and x > 7).
173static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
174 const APSInt &ValueLHS,
175 BinaryOperatorKind OpcodeRHS,
176 const APSInt &ValueRHS) {
177 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
178 "Values must be ordered");
179
180 // Handle cases where the constants are the same.
181 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
182 switch (OpcodeLHS) {
183 case BO_EQ:
184 return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
185 case BO_NE:
186 return OpcodeRHS == BO_EQ;
187 case BO_LE:
188 return OpcodeRHS == BO_GT;
189 case BO_GE:
190 return OpcodeRHS == BO_LT;
191 case BO_LT:
192 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
193 case BO_GT:
194 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
195 default:
196 return false;
197 }
198 }
199
200 // Handle cases where the constants are different.
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000201 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LT || OpcodeLHS == BO_LE) &&
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000202 (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
203 return true;
204
205 // Handle the case where constants are off by one: x > 5 && x < 6.
206 APSInt ValueLHS_plus1;
207 if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
208 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
209 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
210 return true;
211
212 return false;
213}
214
215// Returns whether the ranges covered by the union of both relational
Gabor Horvath91c66712017-12-20 12:22:16 +0000216// expressions cover the whole domain (i.e. x < 10 and x > 0).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000217static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
218 const APSInt &ValueLHS,
219 BinaryOperatorKind OpcodeRHS,
220 const APSInt &ValueRHS) {
221 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
222 "Values must be ordered");
223
224 // Handle cases where the constants are the same: x < 5 || x >= 5.
225 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
226 switch (OpcodeLHS) {
227 case BO_EQ:
228 return OpcodeRHS == BO_NE;
229 case BO_NE:
230 return OpcodeRHS == BO_EQ;
231 case BO_LE:
232 return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
233 case BO_LT:
234 return OpcodeRHS == BO_GE;
235 case BO_GE:
236 return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
237 case BO_GT:
238 return OpcodeRHS == BO_LE;
239 default:
240 return false;
241 }
242 }
243
244 // Handle the case where constants are off by one: x <= 4 || x >= 5.
245 APSInt ValueLHS_plus1;
246 if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
247 incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
248 APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
249 return true;
250
251 // Handle cases where the constants are different: x > 4 || x <= 7.
252 if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
253 (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
254 return true;
255
Alexander Kornienkoc3acd2e2017-03-23 15:13:54 +0000256 // Handle cases where constants are different but both ops are !=, like:
257 // x != 5 || x != 10
258 if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE)
259 return true;
260
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000261 return false;
262}
263
264static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
265 const APSInt &ValueLHS,
266 BinaryOperatorKind OpcodeRHS,
267 const APSInt &ValueRHS) {
268 int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
269 switch (OpcodeLHS) {
270 case BO_EQ:
271 return OpcodeRHS == BO_EQ && Comparison == 0;
272 case BO_NE:
273 return (OpcodeRHS == BO_NE && Comparison == 0) ||
274 (OpcodeRHS == BO_EQ && Comparison != 0) ||
275 (OpcodeRHS == BO_LT && Comparison >= 0) ||
276 (OpcodeRHS == BO_LE && Comparison > 0) ||
277 (OpcodeRHS == BO_GT && Comparison <= 0) ||
278 (OpcodeRHS == BO_GE && Comparison < 0);
279
280 case BO_LT:
281 return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
282 (OpcodeRHS == BO_LE && Comparison > 0) ||
283 (OpcodeRHS == BO_EQ && Comparison > 0));
284 case BO_GT:
285 return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
286 (OpcodeRHS == BO_GE && Comparison < 0) ||
287 (OpcodeRHS == BO_EQ && Comparison < 0));
288 case BO_LE:
289 return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
290 Comparison >= 0;
291 case BO_GE:
292 return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
293 Comparison <= 0;
294 default:
295 return false;
296 }
297}
298
Gabor Horvathec87e172017-11-07 13:17:58 +0000299static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
300 APSInt &Value) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000301 if (Opcode == BO_Sub) {
302 Opcode = BO_Add;
303 Value = -Value;
304 }
305}
306
307AST_MATCHER(Expr, isIntegerConstantExpr) {
Haojian Wua4f5a2a2019-06-06 13:43:38 +0000308 if (Node.isInstantiationDependent())
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000309 return false;
310 return Node.isIntegerConstantExpr(Finder->getASTContext());
311}
312
Gabor Horvathec87e172017-11-07 13:17:58 +0000313AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
314 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
315}
316
317AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
318 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
319}
320
321AST_MATCHER(CallExpr, parametersAreEquivalent) {
322 return Node.getNumArgs() == 2 &&
323 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
324}
325
326AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
327 return Node.getOperatorLoc().isMacroID();
328}
329
330AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
331 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
332}
333
334AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
335
Benjamin Kramer492f1cc2018-02-02 13:23:24 +0000336AST_MATCHER_P(Expr, expandedByMacro, ArrayRef<llvm::StringLiteral>, Names) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000337 const SourceManager &SM = Finder->getASTContext().getSourceManager();
338 const LangOptions &LO = Finder->getASTContext().getLangOpts();
339 SourceLocation Loc = Node.getExprLoc();
340 while (Loc.isMacroID()) {
341 StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
Benjamin Kramer492f1cc2018-02-02 13:23:24 +0000342 if (llvm::is_contained(Names, MacroName))
Gabor Horvathec87e172017-11-07 13:17:58 +0000343 return true;
344 Loc = SM.getImmediateMacroCallerLoc(Loc);
345 }
346 return false;
347}
348
349// Returns a matcher for integer constant expressions.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000350static ast_matchers::internal::Matcher<Expr>
351matchIntegerConstantExpr(StringRef Id) {
352 std::string CstId = (Id + "-const").str();
353 return expr(isIntegerConstantExpr()).bind(CstId);
354}
355
Gabor Horvathec87e172017-11-07 13:17:58 +0000356// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
357// name 'Id' and stores it into 'ConstExpr', the value of the expression is
358// stored into `Value`.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000359static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
Gabor Horvathec87e172017-11-07 13:17:58 +0000360 StringRef Id, APSInt &Value,
361 const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000362 std::string CstId = (Id + "-const").str();
Gabor Horvathec87e172017-11-07 13:17:58 +0000363 ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
364 return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000365}
366
Gabor Horvathec87e172017-11-07 13:17:58 +0000367// Overloaded `retrieveIntegerConstantExpr` for compatibility.
368static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
369 StringRef Id, APSInt &Value) {
370 const Expr *ConstExpr = nullptr;
371 return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
372}
373
374// Returns a matcher for symbolic expressions (matches every expression except
375// ingeter constant expressions).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000376static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
377 std::string SymId = (Id + "-sym").str();
378 return ignoringParenImpCasts(
379 expr(unless(isIntegerConstantExpr())).bind(SymId));
380}
381
Gabor Horvathec87e172017-11-07 13:17:58 +0000382// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
383// stores it into 'SymExpr'.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000384static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
385 StringRef Id, const Expr *&SymExpr) {
386 std::string SymId = (Id + "-sym").str();
387 if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
388 SymExpr = Node;
389 return true;
390 }
391 return false;
392}
393
394// Match a binary operator between a symbolic expression and an integer constant
395// expression.
396static ast_matchers::internal::Matcher<Expr>
397matchBinOpIntegerConstantExpr(StringRef Id) {
398 const auto BinOpCstExpr =
399 expr(
400 anyOf(binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("|"),
401 hasOperatorName("&")),
402 hasEitherOperand(matchSymbolicExpr(Id)),
403 hasEitherOperand(matchIntegerConstantExpr(Id))),
404 binaryOperator(hasOperatorName("-"),
405 hasLHS(matchSymbolicExpr(Id)),
406 hasRHS(matchIntegerConstantExpr(Id)))))
407 .bind(Id);
408 return ignoringParenImpCasts(BinOpCstExpr);
409}
410
Gabor Horvathec87e172017-11-07 13:17:58 +0000411// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000412// name 'Id'.
413static bool
414retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
415 StringRef Id, BinaryOperatorKind &Opcode,
416 const Expr *&Symbol, APSInt &Value) {
417 if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
418 Opcode = BinExpr->getOpcode();
419 return retrieveSymbolicExpr(Result, Id, Symbol) &&
420 retrieveIntegerConstantExpr(Result, Id, Value);
421 }
422 return false;
423}
424
Gabor Horvathec87e172017-11-07 13:17:58 +0000425// Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000426static ast_matchers::internal::Matcher<Expr>
427matchRelationalIntegerConstantExpr(StringRef Id) {
428 std::string CastId = (Id + "-cast").str();
429 std::string SwapId = (Id + "-swap").str();
430 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000431 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000432
433 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
434 isComparisonOperator(), expr().bind(Id),
435 anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
436 hasRHS(matchIntegerConstantExpr(Id))),
437 allOf(hasLHS(matchIntegerConstantExpr(Id)),
438 hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
439
440 // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
441 // to if (x != 0)).
442 const auto CastExpr =
443 implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
444 hasSourceExpression(matchSymbolicExpr(Id)))
445 .bind(CastId);
446
447 const auto NegateRelationalExpr =
448 unaryOperator(hasOperatorName("!"),
449 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
450 .bind(NegateId);
451
Gabor Horvathec87e172017-11-07 13:17:58 +0000452 // Do not bind to double negation.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000453 const auto NegateNegateRelationalExpr =
454 unaryOperator(hasOperatorName("!"),
455 hasUnaryOperand(unaryOperator(
456 hasOperatorName("!"),
457 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
458
Gabor Horvath250c40d2017-11-27 15:05:24 +0000459 const auto OverloadedOperatorExpr =
460 cxxOperatorCallExpr(
461 anyOf(hasOverloadedOperatorName("=="),
462 hasOverloadedOperatorName("!="), hasOverloadedOperatorName("<"),
463 hasOverloadedOperatorName("<="), hasOverloadedOperatorName(">"),
464 hasOverloadedOperatorName(">=")),
465 // Filter noisy false positives.
466 unless(isMacro()), unless(isInTemplateInstantiation()))
467 .bind(OverloadId);
468
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000469 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
Gabor Horvath250c40d2017-11-27 15:05:24 +0000470 NegateNegateRelationalExpr, OverloadedOperatorExpr);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000471}
472
Gabor Horvath250c40d2017-11-27 15:05:24 +0000473// Checks whether a function param is non constant reference type, and may
474// be modified in the function.
475static bool isNonConstReferenceType(QualType ParamType) {
476 return ParamType->isReferenceType() &&
477 !ParamType.getNonReferenceType().isConstQualified();
478}
479
480// Checks whether the arguments of an overloaded operator can be modified in the
481// function.
482// For operators that take an instance and a constant as arguments, only the
483// first argument (the instance) needs to be checked, since the constant itself
484// is a temporary expression. Whether the second parameter is checked is
485// controlled by the parameter `ParamsToCheckCount`.
486static bool
487canOverloadedOperatorArgsBeModified(const FunctionDecl *OperatorDecl,
488 bool checkSecondParam) {
489 unsigned ParamCount = OperatorDecl->getNumParams();
490
491 // Overloaded operators declared inside a class have only one param.
492 // These functions must be declared const in order to not be able to modify
493 // the instance of the class they are called through.
494 if (ParamCount == 1 &&
Simon Pilgrim2ea8b582019-10-17 11:12:53 +0000495 !OperatorDecl->getType()->castAs<FunctionType>()->isConst())
Gabor Horvath250c40d2017-11-27 15:05:24 +0000496 return true;
497
498 if (isNonConstReferenceType(OperatorDecl->getParamDecl(0)->getType()))
499 return true;
500
501 return checkSecondParam && ParamCount == 2 &&
502 isNonConstReferenceType(OperatorDecl->getParamDecl(1)->getType());
503}
504
505// Retrieves sub-expressions matched by 'matchRelationalIntegerConstantExpr'
506// with name 'Id'.
Gabor Horvathec87e172017-11-07 13:17:58 +0000507static bool retrieveRelationalIntegerConstantExpr(
508 const MatchFinder::MatchResult &Result, StringRef Id,
509 const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
510 APSInt &Value, const Expr *&ConstExpr) {
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000511 std::string CastId = (Id + "-cast").str();
512 std::string SwapId = (Id + "-swap").str();
513 std::string NegateId = (Id + "-negate").str();
Gabor Horvath250c40d2017-11-27 15:05:24 +0000514 std::string OverloadId = (Id + "-overload").str();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000515
516 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
517 // Operand received with explicit comparator.
518 Opcode = Bin->getOpcode();
519 OperandExpr = Bin;
Gabor Horvathec87e172017-11-07 13:17:58 +0000520
521 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000522 return false;
523 } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
524 // Operand received with implicit comparator (cast).
525 Opcode = BO_NE;
526 OperandExpr = Cast;
Eugene Zelenko90c117a2016-11-01 18:33:50 +0000527 Value = APSInt(32, false);
Gabor Horvath250c40d2017-11-27 15:05:24 +0000528 } else if (const auto *OverloadedOperatorExpr =
529 Result.Nodes.getNodeAs<CXXOperatorCallExpr>(OverloadId)) {
530 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
531 if (!OverloadedFunctionDecl)
532 return false;
533
534 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
535 return false;
536
Gabor Horvath91c66712017-12-20 12:22:16 +0000537 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
538 return false;
539
Haojian Wua4f5a2a2019-06-06 13:43:38 +0000540 if (const auto *Arg = OverloadedOperatorExpr->getArg(1)) {
541 if (!Arg->isValueDependent() &&
542 !Arg->isIntegerConstantExpr(Value, *Result.Context))
543 return false;
544 }
Gabor Horvath250c40d2017-11-27 15:05:24 +0000545 Symbol = OverloadedOperatorExpr->getArg(0);
546 OperandExpr = OverloadedOperatorExpr;
547 Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
548
549 return BinaryOperator::isComparisonOp(Opcode);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000550 } else {
551 return false;
552 }
553
554 if (!retrieveSymbolicExpr(Result, Id, Symbol))
555 return false;
556
557 if (Result.Nodes.getNodeAs<Expr>(SwapId))
558 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
559 if (Result.Nodes.getNodeAs<Expr>(NegateId))
560 Opcode = BinaryOperator::negateComparisonOp(Opcode);
Gabor Horvathec87e172017-11-07 13:17:58 +0000561 return true;
562}
563
564// Checks for expressions like (X == 4) && (Y != 9)
565static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
566 const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
567 const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
568
569 if (!LhsBinOp || !RhsBinOp)
570 return false;
571
Dmitri Gribenkocf7d7682019-06-12 08:40:53 +0000572 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
573 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
574 };
575
576 if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
577 IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
578 (IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
579 IsIntegerConstantExpr(RhsBinOp->getRHS())))
Gabor Horvathec87e172017-11-07 13:17:58 +0000580 return true;
581 return false;
582}
583
584// Retrieves integer constant subexpressions from binary operator expressions
Gabor Horvath91c66712017-12-20 12:22:16 +0000585// that have two equivalent sides.
Gabor Horvathec87e172017-11-07 13:17:58 +0000586// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
587static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
588 BinaryOperatorKind &MainOpcode,
589 BinaryOperatorKind &SideOpcode,
590 const Expr *&LhsConst,
591 const Expr *&RhsConst,
592 const ASTContext *AstCtx) {
593 assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
594 "Both sides of binary operator must be constant expressions!");
595
596 MainOpcode = BinOp->getOpcode();
597
598 const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
599 const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
600
Dmitri Gribenkocf7d7682019-06-12 08:40:53 +0000601 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
602 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
603 };
604
605 LhsConst = IsIntegerConstantExpr(BinOpLhs->getLHS()) ? BinOpLhs->getLHS()
606 : BinOpLhs->getRHS();
607 RhsConst = IsIntegerConstantExpr(BinOpRhs->getLHS()) ? BinOpRhs->getLHS()
608 : BinOpRhs->getRHS();
Gabor Horvathec87e172017-11-07 13:17:58 +0000609
610 if (!LhsConst || !RhsConst)
611 return false;
612
613 assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
614 "Sides of the binary operator must be equivalent expressions!");
615
616 SideOpcode = BinOpLhs->getOpcode();
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000617
618 return true;
619}
620
Aaron Ballman1caa66d2019-10-30 13:35:20 -0400621static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
622 const SourceManager &SM) {
623 if (T1.getKind() != T2.getKind())
624 return false;
625 if (T1.isNot(tok::raw_identifier))
626 return true;
627 if (T1.getLength() != T2.getLength())
628 return false;
629 return StringRef(SM.getCharacterData(T1.getLocation()), T1.getLength()) ==
630 StringRef(SM.getCharacterData(T2.getLocation()), T2.getLength());
631}
632
633bool isTokAtEndOfExpr(SourceRange ExprSR, Token T, const SourceManager &SM) {
634 return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation();
635}
636
637/// Returns true if both LhsEpxr and RhsExpr are
638/// macro expressions and they are expanded
639/// from different macros.
Gabor Horvathec87e172017-11-07 13:17:58 +0000640static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
641 const Expr *RhsExpr,
642 const ASTContext *AstCtx) {
643 if (!LhsExpr || !RhsExpr)
644 return false;
Aaron Ballman1caa66d2019-10-30 13:35:20 -0400645 SourceRange Lsr = LhsExpr->getSourceRange();
646 SourceRange Rsr = RhsExpr->getSourceRange();
647 if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
Gabor Horvathec87e172017-11-07 13:17:58 +0000648 return false;
649
650 const SourceManager &SM = AstCtx->getSourceManager();
651 const LangOptions &LO = AstCtx->getLangOpts();
652
Aaron Ballman1caa66d2019-10-30 13:35:20 -0400653 std::pair<FileID, unsigned> LsrLocInfo =
654 SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
655 std::pair<FileID, unsigned> RsrLocInfo =
656 SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
657 const llvm::MemoryBuffer *MB = SM.getBuffer(LsrLocInfo.first);
658
659 const char *LTokenPos = MB->getBufferStart() + LsrLocInfo.second;
660 const char *RTokenPos = MB->getBufferStart() + RsrLocInfo.second;
661 Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
662 MB->getBufferStart(), LTokenPos, MB->getBufferEnd());
663 Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
664 MB->getBufferStart(), RTokenPos, MB->getBufferEnd());
665
666 Token LTok, RTok;
667 do { // Compare the expressions token-by-token.
668 LRawLex.LexFromRawLexer(LTok);
669 RRawLex.LexFromRawLexer(RTok);
670 } while (!LTok.is(tok::eof) && !RTok.is(tok::eof) &&
671 isSameRawIdentifierToken(LTok, RTok, SM) &&
672 !isTokAtEndOfExpr(Lsr, LTok, SM) &&
673 !isTokAtEndOfExpr(Rsr, RTok, SM));
674 return (!isTokAtEndOfExpr(Lsr, LTok, SM) ||
675 !isTokAtEndOfExpr(Rsr, RTok, SM)) ||
676 !isSameRawIdentifierToken(LTok, RTok, SM);
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000677}
678
Gabor Horvath250c40d2017-11-27 15:05:24 +0000679static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
680 const Expr *&RhsExpr) {
Gabor Horvathec87e172017-11-07 13:17:58 +0000681 if (!LhsExpr || !RhsExpr)
682 return false;
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000683
Gabor Horvathec87e172017-11-07 13:17:58 +0000684 SourceLocation LhsLoc = LhsExpr->getExprLoc();
685 SourceLocation RhsLoc = RhsExpr->getExprLoc();
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000686
Gabor Horvathec87e172017-11-07 13:17:58 +0000687 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000688}
Benjamin Kramera9bef622018-02-02 13:39:00 +0000689} // namespace
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000690
691void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
692 const auto AnyLiteralExpr = ignoringParenImpCasts(
693 anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
694
Gabor Horvath250c40d2017-11-27 15:05:24 +0000695 const auto BannedIntegerLiteral =
696 integerLiteral(expandedByMacro(KnownBannedMacroNames));
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000697
Gabor Horvathec87e172017-11-07 13:17:58 +0000698 // Binary with equivalent operands, like (X != 2 && X != 2).
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000699 Finder->addMatcher(
700 binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
701 hasOperatorName("%"), hasOperatorName("|"),
702 hasOperatorName("&"), hasOperatorName("^"),
703 matchers::isComparisonOperator(),
704 hasOperatorName("&&"), hasOperatorName("||"),
705 hasOperatorName("=")),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000706 operandsAreEquivalent(),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000707 // Filter noisy false positives.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000708 unless(isInTemplateInstantiation()),
709 unless(binaryOperatorIsInMacro()),
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000710 unless(hasType(realFloatingPointType())),
711 unless(hasEitherOperand(hasType(realFloatingPointType()))),
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000712 unless(hasLHS(AnyLiteralExpr)),
713 unless(hasDescendant(BannedIntegerLiteral)))
Etienne Bergeronbda187d2016-04-26 17:30:30 +0000714 .bind("binary"),
715 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000716
Gabor Horvathec87e172017-11-07 13:17:58 +0000717 // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
Gabor Horvath250c40d2017-11-27 15:05:24 +0000718 Finder->addMatcher(conditionalOperator(expressionsAreEquivalent(),
719 // Filter noisy false positives.
720 unless(conditionalOperatorIsInMacro()),
721 unless(isInTemplateInstantiation()))
722 .bind("cond"),
723 this);
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000724
Gabor Horvathec87e172017-11-07 13:17:58 +0000725 // Overloaded operators with equivalent operands.
Etienne Bergeronc87599f2016-05-12 04:32:47 +0000726 Finder->addMatcher(
727 cxxOperatorCallExpr(
728 anyOf(
729 hasOverloadedOperatorName("-"), hasOverloadedOperatorName("/"),
730 hasOverloadedOperatorName("%"), hasOverloadedOperatorName("|"),
731 hasOverloadedOperatorName("&"), hasOverloadedOperatorName("^"),
732 hasOverloadedOperatorName("=="), hasOverloadedOperatorName("!="),
733 hasOverloadedOperatorName("<"), hasOverloadedOperatorName("<="),
734 hasOverloadedOperatorName(">"), hasOverloadedOperatorName(">="),
735 hasOverloadedOperatorName("&&"), hasOverloadedOperatorName("||"),
736 hasOverloadedOperatorName("=")),
737 parametersAreEquivalent(),
738 // Filter noisy false positives.
739 unless(isMacro()), unless(isInTemplateInstantiation()))
740 .bind("call"),
741 this);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000742
Gabor Horvath91c66712017-12-20 12:22:16 +0000743 // Match expressions like: !(1 | 2 | 3)
744 Finder->addMatcher(
745 implicitCastExpr(
746 hasImplicitDestinationType(isInteger()),
747 has(unaryOperator(
748 hasOperatorName("!"),
749 hasUnaryOperand(ignoringParenImpCasts(binaryOperator(
750 anyOf(hasOperatorName("|"), hasOperatorName("&")),
751 hasLHS(anyOf(binaryOperator(anyOf(hasOperatorName("|"),
752 hasOperatorName("&"))),
753 integerLiteral())),
754 hasRHS(integerLiteral())))))
755 .bind("logical-bitwise-confusion"))),
756 this);
757
758 // Match expressions like: (X << 8) & 0xFF
759 Finder->addMatcher(
760 binaryOperator(hasOperatorName("&"),
761 hasEitherOperand(ignoringParenImpCasts(binaryOperator(
762 hasOperatorName("<<"),
763 hasRHS(ignoringParenImpCasts(
764 integerLiteral().bind("shift-const")))))),
765 hasEitherOperand(ignoringParenImpCasts(
766 integerLiteral().bind("and-const"))))
767 .bind("left-right-shift-confusion"),
768 this);
769
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000770 // Match common expressions and apply more checks to find redundant
771 // sub-expressions.
772 // a) Expr <op> K1 == K2
773 // b) Expr <op> K1 == Expr
774 // c) Expr <op> K1 == Expr <op> K2
775 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
776 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
777 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
778 const auto CstRight = matchIntegerConstantExpr("rhs");
779 const auto SymRight = matchSymbolicExpr("rhs");
780
781 // Match expressions like: x <op> 0xFF == 0xF00.
782 Finder->addMatcher(binaryOperator(isComparisonOperator(),
783 hasEitherOperand(BinOpCstLeft),
784 hasEitherOperand(CstRight))
785 .bind("binop-const-compare-to-const"),
786 this);
787
788 // Match expressions like: x <op> 0xFF == x.
789 Finder->addMatcher(
790 binaryOperator(isComparisonOperator(),
791 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
792 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
793 .bind("binop-const-compare-to-sym"),
794 this);
795
796 // Match expressions like: x <op> 10 == x <op> 12.
797 Finder->addMatcher(binaryOperator(isComparisonOperator(),
798 hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
799 // Already reported as redundant.
800 unless(operandsAreEquivalent()))
801 .bind("binop-const-compare-to-binop-const"),
802 this);
803
804 // Match relational expressions combined with logical operators and find
805 // redundant sub-expressions.
806 // see: 'checkRelationalExpr'
807
808 // Match expressions like: x < 2 && x > 2.
809 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
810 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
811 Finder->addMatcher(
812 binaryOperator(anyOf(hasOperatorName("||"), hasOperatorName("&&")),
813 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
814 // Already reported as redundant.
815 unless(operandsAreEquivalent()))
816 .bind("comparisons-of-symbol-and-const"),
817 this);
818}
819
820void RedundantExpressionCheck::checkArithmeticExpr(
821 const MatchFinder::MatchResult &Result) {
822 APSInt LhsValue, RhsValue;
823 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
824 BinaryOperatorKind LhsOpcode, RhsOpcode;
825
826 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
827 "binop-const-compare-to-sym")) {
828 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
829 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
830 LhsValue) ||
831 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
832 !areEquivalentExpr(LhsSymbol, RhsSymbol))
833 return;
834
835 // Check expressions: x + k == x or x - k == x.
836 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
837 if ((LhsValue != 0 && Opcode == BO_EQ) ||
838 (LhsValue == 0 && Opcode == BO_NE))
839 diag(ComparisonOperator->getOperatorLoc(),
840 "logical expression is always false");
841 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
842 (LhsValue != 0 && Opcode == BO_NE))
843 diag(ComparisonOperator->getOperatorLoc(),
844 "logical expression is always true");
845 }
846 } else if (const auto *ComparisonOperator =
847 Result.Nodes.getNodeAs<BinaryOperator>(
848 "binop-const-compare-to-binop-const")) {
849 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
850
851 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
852 LhsValue) ||
853 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
854 RhsValue) ||
855 !areEquivalentExpr(LhsSymbol, RhsSymbol))
856 return;
857
Gabor Horvathec87e172017-11-07 13:17:58 +0000858 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
859 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000860
861 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
862 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
863 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
864 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
865 diag(ComparisonOperator->getOperatorLoc(),
866 "logical expression is always true");
867 } else if ((Opcode == BO_EQ &&
868 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
869 (Opcode == BO_NE &&
870 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
871 diag(ComparisonOperator->getOperatorLoc(),
872 "logical expression is always false");
873 }
874 }
875 }
876}
877
Gabor Horvath91c66712017-12-20 12:22:16 +0000878static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value) {
879 return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
880}
881
882static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
883 APSInt Value) {
884 return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
885}
886
887static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value) {
888 return ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) ||
889 ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
890}
891
892
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000893void RedundantExpressionCheck::checkBitwiseExpr(
894 const MatchFinder::MatchResult &Result) {
895 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
896 "binop-const-compare-to-const")) {
897 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
898
899 APSInt LhsValue, RhsValue;
900 const Expr *LhsSymbol = nullptr;
901 BinaryOperatorKind LhsOpcode;
902 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
903 LhsValue) ||
904 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
905 return;
906
907 uint64_t LhsConstant = LhsValue.getZExtValue();
908 uint64_t RhsConstant = RhsValue.getZExtValue();
909 SourceLocation Loc = ComparisonOperator->getOperatorLoc();
910
911 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
912 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
913 if (Opcode == BO_EQ)
914 diag(Loc, "logical expression is always false");
915 else if (Opcode == BO_NE)
916 diag(Loc, "logical expression is always true");
917 }
918
919 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
920 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
921 if (Opcode == BO_EQ)
922 diag(Loc, "logical expression is always false");
923 else if (Opcode == BO_NE)
924 diag(Loc, "logical expression is always true");
925 }
Gabor Horvath91c66712017-12-20 12:22:16 +0000926 } else if (const auto *IneffectiveOperator =
927 Result.Nodes.getNodeAs<BinaryOperator>(
928 "ineffective-bitwise")) {
929 APSInt Value;
930 const Expr *Sym = nullptr, *ConstExpr = nullptr;
931
932 if (!retrieveSymbolicExpr(Result, "ineffective-bitwise", Sym) ||
933 !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value,
934 ConstExpr))
935 return;
936
937 if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
938 return;
939
940 SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
941
942 BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode();
943 if (exprEvaluatesToZero(Opcode, Value)) {
944 diag(Loc, "expression always evaluates to 0");
945 } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) {
Stephen Kelly43465bf2018-08-09 22:42:26 +0000946 SourceRange ConstExprRange(ConstExpr->getBeginLoc(),
Stephen Kellyc09197e2018-08-09 22:43:02 +0000947 ConstExpr->getEndLoc());
Gabor Horvath91c66712017-12-20 12:22:16 +0000948 StringRef ConstExprText = Lexer::getSourceText(
949 CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager,
950 Result.Context->getLangOpts());
951
952 diag(Loc, "expression always evaluates to '%0'") << ConstExprText;
953
954 } else if (exprEvaluatesToSymbolic(Opcode, Value)) {
Stephen Kellyc09197e2018-08-09 22:43:02 +0000955 SourceRange SymExprRange(Sym->getBeginLoc(), Sym->getEndLoc());
Gabor Horvath91c66712017-12-20 12:22:16 +0000956
957 StringRef ExprText = Lexer::getSourceText(
958 CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager,
959 Result.Context->getLangOpts());
960
961 diag(Loc, "expression always evaluates to '%0'") << ExprText;
962 }
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000963 }
964}
965
966void RedundantExpressionCheck::checkRelationalExpr(
967 const MatchFinder::MatchResult &Result) {
968 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
969 "comparisons-of-symbol-and-const")) {
970 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
Gabor Horvathec87e172017-11-07 13:17:58 +0000971 // E.g.: (X < 2) && (X > 4)
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000972 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
973
974 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000975 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
Gabor Horvathec87e172017-11-07 13:17:58 +0000976 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000977 BinaryOperatorKind LhsOpcode, RhsOpcode;
Gabor Horvathec87e172017-11-07 13:17:58 +0000978 APSInt LhsValue, RhsValue;
979
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000980 if (!retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000981 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000982 !retrieveRelationalIntegerConstantExpr(
Gabor Horvathec87e172017-11-07 13:17:58 +0000983 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000984 !areEquivalentExpr(LhsSymbol, RhsSymbol))
985 return;
986
Gabor Horvathec87e172017-11-07 13:17:58 +0000987 // Bring expr to a canonical form: smallest constant must be on the left.
Etienne Bergeron00639bc2016-07-07 04:03:05 +0000988 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
989 std::swap(LhsExpr, RhsExpr);
990 std::swap(LhsValue, RhsValue);
991 std::swap(LhsSymbol, RhsSymbol);
992 std::swap(LhsOpcode, RhsOpcode);
993 }
994
Gabor Horvathec87e172017-11-07 13:17:58 +0000995 // Constants come from two different macros, or one of them is a macro.
996 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
997 areExprsMacroAndNonMacro(LhsConst, RhsConst))
998 return;
999
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001000 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
1001 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1002 diag(ComparisonOperator->getOperatorLoc(),
Gabor Horvathec87e172017-11-07 13:17:58 +00001003 "equivalent expression on both sides of logical operator");
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001004 return;
1005 }
1006
1007 if (Opcode == BO_LAnd) {
1008 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1009 diag(ComparisonOperator->getOperatorLoc(),
1010 "logical expression is always false");
1011 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1012 diag(LhsExpr->getExprLoc(), "expression is redundant");
1013 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
1014 diag(RhsExpr->getExprLoc(), "expression is redundant");
1015 }
1016 }
1017
1018 if (Opcode == BO_LOr) {
1019 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1020 diag(ComparisonOperator->getOperatorLoc(),
1021 "logical expression is always true");
1022 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1023 diag(RhsExpr->getExprLoc(), "expression is redundant");
1024 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
1025 diag(LhsExpr->getExprLoc(), "expression is redundant");
1026 }
1027 }
1028 }
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001029}
1030
1031void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
Gabor Horvathec87e172017-11-07 13:17:58 +00001032 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
Gabor Horvathec87e172017-11-07 13:17:58 +00001033 // If the expression's constants are macros, check whether they are
1034 // intentional.
1035 if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
1036 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
1037 BinaryOperatorKind MainOpcode, SideOpcode;
1038
Gabor Horvath250c40d2017-11-27 15:05:24 +00001039 if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
1040 LhsConst, RhsConst, Result.Context))
1041 return;
Gabor Horvathec87e172017-11-07 13:17:58 +00001042
1043 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
1044 areExprsMacroAndNonMacro(LhsConst, RhsConst))
1045 return;
1046 }
1047
1048 diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
1049 }
1050
1051 if (const auto *CondOp =
1052 Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
1053 const Expr *TrueExpr = CondOp->getTrueExpr();
1054 const Expr *FalseExpr = CondOp->getFalseExpr();
1055
1056 if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
1057 areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
1058 return;
1059 diag(CondOp->getColonLoc(),
1060 "'true' and 'false' expressions are equivalent");
1061 }
1062
1063 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
Gabor Horvath250c40d2017-11-27 15:05:24 +00001064 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
1065 if (!OverloadedFunctionDecl)
1066 return;
1067
1068 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, true))
1069 return;
1070
Gabor Horvathec87e172017-11-07 13:17:58 +00001071 diag(Call->getOperatorLoc(),
1072 "both sides of overloaded operator are equivalent");
1073 }
1074
Gabor Horvath91c66712017-12-20 12:22:16 +00001075 if (const auto *NegateOperator =
1076 Result.Nodes.getNodeAs<UnaryOperator>("logical-bitwise-confusion")) {
1077 SourceLocation OperatorLoc = NegateOperator->getOperatorLoc();
1078
1079 auto Diag =
1080 diag(OperatorLoc,
1081 "ineffective logical negation operator used; did you mean '~'?");
1082 SourceLocation LogicalNotLocation = OperatorLoc.getLocWithOffset(1);
1083
1084 if (!LogicalNotLocation.isMacroID())
1085 Diag << FixItHint::CreateReplacement(
1086 CharSourceRange::getCharRange(OperatorLoc, LogicalNotLocation), "~");
1087 }
1088
1089 if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs<BinaryOperator>(
1090 "left-right-shift-confusion")) {
1091 const auto *ShiftingConst = Result.Nodes.getNodeAs<Expr>("shift-const");
1092 assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!");
1093 APSInt ShiftingValue;
1094
1095 if (!ShiftingConst->isIntegerConstantExpr(ShiftingValue, *Result.Context))
1096 return;
1097
1098 const auto *AndConst = Result.Nodes.getNodeAs<Expr>("and-const");
1099 assert(AndConst && "Expr* 'AndCont' is nullptr!");
1100 APSInt AndValue;
1101 if (!AndConst->isIntegerConstantExpr(AndValue, *Result.Context))
1102 return;
1103
1104 // If ShiftingConst is shifted left with more bits than the position of the
1105 // leftmost 1 in the bit representation of AndValue, AndConstant is
1106 // ineffective.
Benjamin Kramer10db8d62018-02-02 13:23:21 +00001107 if (AndValue.getActiveBits() > ShiftingValue)
Gabor Horvath91c66712017-12-20 12:22:16 +00001108 return;
1109
1110 auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
Alexander Kornienko396fc872018-02-01 16:39:12 +00001111 "ineffective bitwise and operation");
Gabor Horvath91c66712017-12-20 12:22:16 +00001112 }
1113
Gabor Horvathec87e172017-11-07 13:17:58 +00001114 // Check for the following bound expressions:
1115 // - "binop-const-compare-to-sym",
1116 // - "binop-const-compare-to-binop-const",
1117 // Produced message:
1118 // -> "logical expression is always false/true"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001119 checkArithmeticExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001120
1121 // Check for the following bound expression:
1122 // - "binop-const-compare-to-const",
Gabor Horvath91c66712017-12-20 12:22:16 +00001123 // - "ineffective-bitwise"
Gabor Horvathec87e172017-11-07 13:17:58 +00001124 // Produced message:
1125 // -> "logical expression is always false/true"
Gabor Horvath91c66712017-12-20 12:22:16 +00001126 // -> "expression always evaluates to ..."
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001127 checkBitwiseExpr(Result);
Gabor Horvathec87e172017-11-07 13:17:58 +00001128
1129 // Check for te following bound expression:
1130 // - "comparisons-of-symbol-and-const",
1131 // Produced messages:
1132 // -> "equivalent expression on both sides of logical operator",
1133 // -> "logical expression is always false/true"
1134 // -> "expression is redundant"
Etienne Bergeron00639bc2016-07-07 04:03:05 +00001135 checkRelationalExpr(Result);
Etienne Bergeronbda187d2016-04-26 17:30:30 +00001136}
1137
1138} // namespace misc
1139} // namespace tidy
1140} // namespace clang