1 //===--- RedundantExpressionCheck.cpp - clang-tidy-------------------------===//
2 //
3 // 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
6 //
7 //===----------------------------------------------------------------------===//
8
9 #include "RedundantExpressionCheck.h"
10 #include "../utils/Matchers.h"
11 #include "../utils/OptionsUtils.h"
12 #include "clang/AST/ASTContext.h"
13 #include "clang/ASTMatchers/ASTMatchFinder.h"
14 #include "clang/Basic/LLVM.h"
15 #include "clang/Basic/SourceLocation.h"
16 #include "clang/Basic/SourceManager.h"
17 #include "clang/Lex/Lexer.h"
18 #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>
25 #include <string>
26 #include <vector>
27
28 using namespace clang::ast_matchers;
29 using namespace clang::tidy::matchers;
30
31 namespace clang {
32 namespace tidy {
33 namespace misc {
34 namespace {
35 using llvm::APSInt;
36
37 static constexpr llvm::StringLiteral KnownBannedMacroNames[] = {
38 "EAGAIN",
39 "EWOULDBLOCK",
40 "SIGCLD",
41 "SIGCHLD",
42 };
43
incrementWithoutOverflow(const APSInt & Value,APSInt & Result)44 static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
45 Result = Value;
46 ++Result;
47 return Value < Result;
48 }
49
areEquivalentNameSpecifier(const NestedNameSpecifier * Left,const NestedNameSpecifier * Right)50 static 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
areEquivalentExpr(const Expr * Left,const Expr * Right)58 static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
59 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()) {
73 if (!areEquivalentExpr(dyn_cast<Expr>(*LeftIter),
74 dyn_cast<Expr>(*RightIter)))
75 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();
93 return LeftLit.getBitWidth() == RightLit.getBitWidth() &&
94 LeftLit == RightLit;
95 }
96 case Stmt::FloatingLiteralClass:
97 return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
98 cast<FloatingLiteral>(Right)->getValue());
99 case Stmt::StringLiteralClass:
100 return cast<StringLiteral>(Left)->getBytes() ==
101 cast<StringLiteral>(Right)->getBytes();
102 case Stmt::CXXOperatorCallExprClass:
103 return cast<CXXOperatorCallExpr>(Left)->getOperator() ==
104 cast<CXXOperatorCallExpr>(Right)->getOperator();
105 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());
112 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();
118 case Stmt::CXXFunctionalCastExprClass:
119 case Stmt::CStyleCastExprClass:
120 return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
121 cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
122 case Stmt::CallExprClass:
123 case Stmt::ImplicitCastExprClass:
124 case Stmt::ArraySubscriptExprClass:
125 return true;
126 case Stmt::UnaryOperatorClass:
127 if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
128 return false;
129 return cast<UnaryOperator>(Left)->getOpcode() ==
130 cast<UnaryOperator>(Right)->getOpcode();
131 case Stmt::BinaryOperatorClass:
132 return cast<BinaryOperator>(Left)->getOpcode() ==
133 cast<BinaryOperator>(Right)->getOpcode();
134 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;
148 }
149 }
150
151 // 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).
areEquivalentRanges(BinaryOperatorKind OpcodeLHS,const APSInt & ValueLHS,BinaryOperatorKind OpcodeRHS,const APSInt & ValueRHS)153 static 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).
areExclusiveRanges(BinaryOperatorKind OpcodeLHS,const APSInt & ValueLHS,BinaryOperatorKind OpcodeRHS,const APSInt & ValueRHS)173 static 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.
201 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LT || OpcodeLHS == BO_LE) &&
202 (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
216 // expressions cover the whole domain (i.e. x < 10 and x > 0).
rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,const APSInt & ValueLHS,BinaryOperatorKind OpcodeRHS,const APSInt & ValueRHS)217 static 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
256 // 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
261 return false;
262 }
263
rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,const APSInt & ValueLHS,BinaryOperatorKind OpcodeRHS,const APSInt & ValueRHS)264 static 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
transformSubToCanonicalAddExpr(BinaryOperatorKind & Opcode,APSInt & Value)299 static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
300 APSInt &Value) {
301 if (Opcode == BO_Sub) {
302 Opcode = BO_Add;
303 Value = -Value;
304 }
305 }
306
AST_MATCHER(Expr,isIntegerConstantExpr)307 AST_MATCHER(Expr, isIntegerConstantExpr) {
308 if (Node.isInstantiationDependent())
309 return false;
310 return Node.isIntegerConstantExpr(Finder->getASTContext());
311 }
312
AST_MATCHER(BinaryOperator,operandsAreEquivalent)313 AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
314 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
315 }
316
AST_MATCHER(ConditionalOperator,expressionsAreEquivalent)317 AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
318 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
319 }
320
AST_MATCHER(CallExpr,parametersAreEquivalent)321 AST_MATCHER(CallExpr, parametersAreEquivalent) {
322 return Node.getNumArgs() == 2 &&
323 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
324 }
325
AST_MATCHER(BinaryOperator,binaryOperatorIsInMacro)326 AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
327 return Node.getOperatorLoc().isMacroID();
328 }
329
AST_MATCHER(ConditionalOperator,conditionalOperatorIsInMacro)330 AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
331 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
332 }
333
AST_MATCHER(Expr,isMacro)334 AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
335
AST_MATCHER_P(Expr,expandedByMacro,ArrayRef<llvm::StringLiteral>,Names)336 AST_MATCHER_P(Expr, expandedByMacro, ArrayRef<llvm::StringLiteral>, Names) {
337 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);
342 if (llvm::is_contained(Names, MacroName))
343 return true;
344 Loc = SM.getImmediateMacroCallerLoc(Loc);
345 }
346 return false;
347 }
348
349 // Returns a matcher for integer constant expressions.
350 static ast_matchers::internal::Matcher<Expr>
matchIntegerConstantExpr(StringRef Id)351 matchIntegerConstantExpr(StringRef Id) {
352 std::string CstId = (Id + "-const").str();
353 return expr(isIntegerConstantExpr()).bind(CstId);
354 }
355
356 // 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`.
retrieveIntegerConstantExpr(const MatchFinder::MatchResult & Result,StringRef Id,APSInt & Value,const Expr * & ConstExpr)359 static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
360 StringRef Id, APSInt &Value,
361 const Expr *&ConstExpr) {
362 std::string CstId = (Id + "-const").str();
363 ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
364 return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
365 }
366
367 // Overloaded `retrieveIntegerConstantExpr` for compatibility.
retrieveIntegerConstantExpr(const MatchFinder::MatchResult & Result,StringRef Id,APSInt & Value)368 static 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).
matchSymbolicExpr(StringRef Id)376 static 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
382 // Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
383 // stores it into 'SymExpr'.
retrieveSymbolicExpr(const MatchFinder::MatchResult & Result,StringRef Id,const Expr * & SymExpr)384 static 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.
396 static ast_matchers::internal::Matcher<Expr>
matchBinOpIntegerConstantExpr(StringRef Id)397 matchBinOpIntegerConstantExpr(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
411 // Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
412 // name 'Id'.
413 static bool
retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult & Result,StringRef Id,BinaryOperatorKind & Opcode,const Expr * & Symbol,APSInt & Value)414 retrieveBinOpIntegerConstantExpr(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
425 // Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
426 static ast_matchers::internal::Matcher<Expr>
matchRelationalIntegerConstantExpr(StringRef Id)427 matchRelationalIntegerConstantExpr(StringRef Id) {
428 std::string CastId = (Id + "-cast").str();
429 std::string SwapId = (Id + "-swap").str();
430 std::string NegateId = (Id + "-negate").str();
431 std::string OverloadId = (Id + "-overload").str();
432
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
452 // Do not bind to double negation.
453 const auto NegateNegateRelationalExpr =
454 unaryOperator(hasOperatorName("!"),
455 hasUnaryOperand(unaryOperator(
456 hasOperatorName("!"),
457 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
458
459 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
469 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
470 NegateNegateRelationalExpr, OverloadedOperatorExpr);
471 }
472
473 // Checks whether a function param is non constant reference type, and may
474 // be modified in the function.
isNonConstReferenceType(QualType ParamType)475 static 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`.
486 static bool
canOverloadedOperatorArgsBeModified(const FunctionDecl * OperatorDecl,bool checkSecondParam)487 canOverloadedOperatorArgsBeModified(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 &&
495 !OperatorDecl->getType()->castAs<FunctionType>()->isConst())
496 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'.
retrieveRelationalIntegerConstantExpr(const MatchFinder::MatchResult & Result,StringRef Id,const Expr * & OperandExpr,BinaryOperatorKind & Opcode,const Expr * & Symbol,APSInt & Value,const Expr * & ConstExpr)507 static bool retrieveRelationalIntegerConstantExpr(
508 const MatchFinder::MatchResult &Result, StringRef Id,
509 const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
510 APSInt &Value, const Expr *&ConstExpr) {
511 std::string CastId = (Id + "-cast").str();
512 std::string SwapId = (Id + "-swap").str();
513 std::string NegateId = (Id + "-negate").str();
514 std::string OverloadId = (Id + "-overload").str();
515
516 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
517 // Operand received with explicit comparator.
518 Opcode = Bin->getOpcode();
519 OperandExpr = Bin;
520
521 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
522 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;
527 Value = APSInt(32, false);
528 } 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
537 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
538 return false;
539
540 if (const auto *Arg = OverloadedOperatorExpr->getArg(1)) {
541 if (!Arg->isValueDependent() &&
542 !Arg->isIntegerConstantExpr(Value, *Result.Context))
543 return false;
544 }
545 Symbol = OverloadedOperatorExpr->getArg(0);
546 OperandExpr = OverloadedOperatorExpr;
547 Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
548
549 return BinaryOperator::isComparisonOp(Opcode);
550 } 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);
561 return true;
562 }
563
564 // Checks for expressions like (X == 4) && (Y != 9)
areSidesBinaryConstExpressions(const BinaryOperator * & BinOp,const ASTContext * AstCtx)565 static 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
572 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())))
580 return true;
581 return false;
582 }
583
584 // Retrieves integer constant subexpressions from binary operator expressions
585 // that have two equivalent sides.
586 // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
retrieveConstExprFromBothSides(const BinaryOperator * & BinOp,BinaryOperatorKind & MainOpcode,BinaryOperatorKind & SideOpcode,const Expr * & LhsConst,const Expr * & RhsConst,const ASTContext * AstCtx)587 static 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
601 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();
609
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();
617
618 return true;
619 }
620
isSameRawIdentifierToken(const Token & T1,const Token & T2,const SourceManager & SM)621 static 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
isTokAtEndOfExpr(SourceRange ExprSR,Token T,const SourceManager & SM)633 bool 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.
areExprsFromDifferentMacros(const Expr * LhsExpr,const Expr * RhsExpr,const ASTContext * AstCtx)640 static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
641 const Expr *RhsExpr,
642 const ASTContext *AstCtx) {
643 if (!LhsExpr || !RhsExpr)
644 return false;
645 SourceRange Lsr = LhsExpr->getSourceRange();
646 SourceRange Rsr = RhsExpr->getSourceRange();
647 if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
648 return false;
649
650 const SourceManager &SM = AstCtx->getSourceManager();
651 const LangOptions &LO = AstCtx->getLangOpts();
652
653 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);
677 }
678
areExprsMacroAndNonMacro(const Expr * & LhsExpr,const Expr * & RhsExpr)679 static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
680 const Expr *&RhsExpr) {
681 if (!LhsExpr || !RhsExpr)
682 return false;
683
684 SourceLocation LhsLoc = LhsExpr->getExprLoc();
685 SourceLocation RhsLoc = RhsExpr->getExprLoc();
686
687 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
688 }
689 } // namespace
690
registerMatchers(MatchFinder * Finder)691 void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
692 const auto AnyLiteralExpr = ignoringParenImpCasts(
693 anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
694
695 const auto BannedIntegerLiteral =
696 integerLiteral(expandedByMacro(KnownBannedMacroNames));
697
698 // Binary with equivalent operands, like (X != 2 && X != 2).
699 Finder->addMatcher(
700 binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
701 hasOperatorName("%"), hasOperatorName("|"),
702 hasOperatorName("&"), hasOperatorName("^"),
703 matchers::isComparisonOperator(),
704 hasOperatorName("&&"), hasOperatorName("||"),
705 hasOperatorName("=")),
706 operandsAreEquivalent(),
707 // Filter noisy false positives.
708 unless(isInTemplateInstantiation()),
709 unless(binaryOperatorIsInMacro()),
710 unless(hasType(realFloatingPointType())),
711 unless(hasEitherOperand(hasType(realFloatingPointType()))),
712 unless(hasLHS(AnyLiteralExpr)),
713 unless(hasDescendant(BannedIntegerLiteral)))
714 .bind("binary"),
715 this);
716
717 // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
718 Finder->addMatcher(conditionalOperator(expressionsAreEquivalent(),
719 // Filter noisy false positives.
720 unless(conditionalOperatorIsInMacro()),
721 unless(isInTemplateInstantiation()))
722 .bind("cond"),
723 this);
724
725 // Overloaded operators with equivalent operands.
726 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);
742
743 // 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
770 // 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
checkArithmeticExpr(const MatchFinder::MatchResult & Result)820 void 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
858 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
859 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
860
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
exprEvaluatesToZero(BinaryOperatorKind Opcode,APSInt Value)878 static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value) {
879 return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
880 }
881
exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,APSInt Value)882 static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
883 APSInt Value) {
884 return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
885 }
886
exprEvaluatesToSymbolic(BinaryOperatorKind Opcode,APSInt Value)887 static 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
checkBitwiseExpr(const MatchFinder::MatchResult & Result)893 void 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 }
926 } 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)) {
946 SourceRange ConstExprRange(ConstExpr->getBeginLoc(),
947 ConstExpr->getEndLoc());
948 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)) {
955 SourceRange SymExprRange(Sym->getBeginLoc(), Sym->getEndLoc());
956
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 }
963 }
964 }
965
checkRelationalExpr(const MatchFinder::MatchResult & Result)966 void 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).
971 // E.g.: (X < 2) && (X > 4)
972 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
973
974 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
975 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
976 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
977 BinaryOperatorKind LhsOpcode, RhsOpcode;
978 APSInt LhsValue, RhsValue;
979
980 if (!retrieveRelationalIntegerConstantExpr(
981 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
982 !retrieveRelationalIntegerConstantExpr(
983 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
984 !areEquivalentExpr(LhsSymbol, RhsSymbol))
985 return;
986
987 // Bring expr to a canonical form: smallest constant must be on the left.
988 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
995 // 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
1000 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
1001 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1002 diag(ComparisonOperator->getOperatorLoc(),
1003 "equivalent expression on both sides of logical operator");
1004 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 }
1029 }
1030
check(const MatchFinder::MatchResult & Result)1031 void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
1032 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
1033 // 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
1039 if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
1040 LhsConst, RhsConst, Result.Context))
1041 return;
1042
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")) {
1064 const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
1065 if (!OverloadedFunctionDecl)
1066 return;
1067
1068 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, true))
1069 return;
1070
1071 diag(Call->getOperatorLoc(),
1072 "both sides of overloaded operator are equivalent");
1073 }
1074
1075 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.
1107 if (AndValue.getActiveBits() > ShiftingValue)
1108 return;
1109
1110 auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
1111 "ineffective bitwise and operation");
1112 }
1113
1114 // 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"
1119 checkArithmeticExpr(Result);
1120
1121 // Check for the following bound expression:
1122 // - "binop-const-compare-to-const",
1123 // - "ineffective-bitwise"
1124 // Produced message:
1125 // -> "logical expression is always false/true"
1126 // -> "expression always evaluates to ..."
1127 checkBitwiseExpr(Result);
1128
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"
1135 checkRelationalExpr(Result);
1136 }
1137
1138 } // namespace misc
1139 } // namespace tidy
1140 } // namespace clang
1141