clang-tools 23.0.0git
RedundantExpressionCheck.cpp
Go to the documentation of this file.
1//===----------------------------------------------------------------------===//
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
10#include "../utils/Matchers.h"
11#include "clang/AST/ASTContext.h"
12#include "clang/ASTMatchers/ASTMatchFinder.h"
13#include "clang/Basic/LLVM.h"
14#include "clang/Basic/SourceLocation.h"
15#include "clang/Basic/SourceManager.h"
16#include "clang/Lex/Lexer.h"
17#include "llvm/ADT/APInt.h"
18#include "llvm/ADT/APSInt.h"
19#include "llvm/ADT/FoldingSet.h"
20#include "llvm/ADT/SmallBitVector.h"
21#include "llvm/Support/FormatVariadic.h"
22#include <algorithm>
23#include <cassert>
24#include <cstdint>
25#include <optional>
26#include <string>
27
28using namespace clang::ast_matchers;
29using namespace clang::tidy::matchers;
30
31namespace clang::tidy::misc {
32using llvm::APSInt;
33
34static constexpr StringRef KnownBannedMacroNames[] = {
35 "EAGAIN",
36 "EWOULDBLOCK",
37 "SIGCLD",
38 "SIGCHLD",
39};
40
41static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
42 Result = Value;
43 ++Result;
44 return Value < Result;
45}
46
47static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
48 if (!Left || !Right)
49 return !Left && !Right;
50
51 Left = Left->IgnoreParens();
52 Right = Right->IgnoreParens();
53
54 // Compare classes.
55 if (Left->getStmtClass() != Right->getStmtClass())
56 return false;
57
58 // Compare children.
59 Expr::const_child_iterator LeftIter = Left->child_begin();
60 Expr::const_child_iterator RightIter = Right->child_begin();
61 while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
62 if (!areEquivalentExpr(dyn_cast_or_null<Expr>(*LeftIter),
63 dyn_cast_or_null<Expr>(*RightIter)))
64 return false;
65 ++LeftIter;
66 ++RightIter;
67 }
68 if (LeftIter != Left->child_end() || RightIter != Right->child_end())
69 return false;
70
71 // Perform extra checks.
72 switch (Left->getStmtClass()) {
73 default:
74 return false;
75
76 case Stmt::CharacterLiteralClass:
77 return cast<CharacterLiteral>(Left)->getValue() ==
78 cast<CharacterLiteral>(Right)->getValue();
79 case Stmt::IntegerLiteralClass: {
80 const llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
81 const llvm::APInt RightLit = cast<IntegerLiteral>(Right)->getValue();
82 return LeftLit.getBitWidth() == RightLit.getBitWidth() &&
83 LeftLit == RightLit;
84 }
85 case Stmt::FloatingLiteralClass:
86 return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
87 cast<FloatingLiteral>(Right)->getValue());
88 case Stmt::StringLiteralClass:
89 return cast<StringLiteral>(Left)->getBytes() ==
90 cast<StringLiteral>(Right)->getBytes();
91 case Stmt::CXXOperatorCallExprClass:
92 return cast<CXXOperatorCallExpr>(Left)->getOperator() ==
93 cast<CXXOperatorCallExpr>(Right)->getOperator();
94 case Stmt::DependentScopeDeclRefExprClass:
95 if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
96 cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
97 return false;
98 return cast<DependentScopeDeclRefExpr>(Left)->getQualifier() ==
99 cast<DependentScopeDeclRefExpr>(Right)->getQualifier();
100 case Stmt::DeclRefExprClass:
101 return cast<DeclRefExpr>(Left)->getDecl() ==
102 cast<DeclRefExpr>(Right)->getDecl();
103 case Stmt::MemberExprClass:
104 return cast<MemberExpr>(Left)->getMemberDecl() ==
105 cast<MemberExpr>(Right)->getMemberDecl();
106 case Stmt::CXXFoldExprClass:
107 return cast<CXXFoldExpr>(Left)->getOperator() ==
108 cast<CXXFoldExpr>(Right)->getOperator();
109 case Stmt::CXXFunctionalCastExprClass:
110 case Stmt::CStyleCastExprClass:
111 return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
112 cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
113 case Stmt::CallExprClass:
114 case Stmt::ImplicitCastExprClass:
115 case Stmt::ArraySubscriptExprClass:
116 return true;
117 case Stmt::UnaryOperatorClass:
118 if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
119 return false;
120 return cast<UnaryOperator>(Left)->getOpcode() ==
121 cast<UnaryOperator>(Right)->getOpcode();
122 case Stmt::BinaryOperatorClass:
123 if (cast<BinaryOperator>(Left)->isAssignmentOp())
124 return false;
125 return cast<BinaryOperator>(Left)->getOpcode() ==
126 cast<BinaryOperator>(Right)->getOpcode();
127 case Stmt::UnaryExprOrTypeTraitExprClass:
128 const auto *LeftUnaryExpr = cast<UnaryExprOrTypeTraitExpr>(Left);
129 const auto *RightUnaryExpr = cast<UnaryExprOrTypeTraitExpr>(Right);
130 if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
131 return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() &&
132 LeftUnaryExpr->getArgumentType() ==
133 RightUnaryExpr->getArgumentType();
134 if (!LeftUnaryExpr->isArgumentType() && !RightUnaryExpr->isArgumentType())
135 return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(),
136 RightUnaryExpr->getArgumentExpr());
137
138 return false;
139 }
140}
141
142// For a given expression 'x', returns whether the ranges covered by the
143// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
144static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
145 const APSInt &ValueLHS,
146 BinaryOperatorKind OpcodeRHS,
147 const APSInt &ValueRHS) {
148 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
149 "Values must be ordered");
150 // Handle the case where constants are the same: x <= 4 <==> x <= 4.
151 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
152 return OpcodeLHS == OpcodeRHS;
153
154 // Handle the case where constants are off by one: x <= 4 <==> x < 5.
155 APSInt ValueLhsPlus1;
156 return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
157 (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
158 incrementWithoutOverflow(ValueLHS, ValueLhsPlus1) &&
159 APSInt::compareValues(ValueLhsPlus1, ValueRHS) == 0;
160}
161
162// For a given expression 'x', returns whether the ranges covered by the
163// relational operators are fully disjoint (i.e. x < 4 and x > 7).
164static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
165 const APSInt &ValueLHS,
166 BinaryOperatorKind OpcodeRHS,
167 const APSInt &ValueRHS) {
168 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
169 "Values must be ordered");
170
171 // Handle cases where the constants are the same.
172 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
173 switch (OpcodeLHS) {
174 case BO_EQ:
175 return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
176 case BO_NE:
177 return OpcodeRHS == BO_EQ;
178 case BO_LE:
179 return OpcodeRHS == BO_GT;
180 case BO_GE:
181 return OpcodeRHS == BO_LT;
182 case BO_LT:
183 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
184 case BO_GT:
185 return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
186 default:
187 return false;
188 }
189 }
190
191 // Handle cases where the constants are different.
192 if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LT || OpcodeLHS == BO_LE) &&
193 (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
194 return true;
195
196 // Handle the case where constants are off by one: x > 5 && x < 6.
197 APSInt ValueLhsPlus1;
198 if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
199 incrementWithoutOverflow(ValueLHS, ValueLhsPlus1) &&
200 APSInt::compareValues(ValueLhsPlus1, ValueRHS) == 0)
201 return true;
202
203 return false;
204}
205
206// Returns whether the ranges covered by the union of both relational
207// expressions cover the whole domain (i.e. x < 10 and x > 0).
208static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
209 const APSInt &ValueLHS,
210 BinaryOperatorKind OpcodeRHS,
211 const APSInt &ValueRHS) {
212 assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
213 "Values must be ordered");
214
215 // Handle cases where the constants are the same: x < 5 || x >= 5.
216 if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
217 switch (OpcodeLHS) {
218 case BO_EQ:
219 return OpcodeRHS == BO_NE;
220 case BO_NE:
221 return OpcodeRHS == BO_EQ;
222 case BO_LE:
223 return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
224 case BO_LT:
225 return OpcodeRHS == BO_GE;
226 case BO_GE:
227 return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
228 case BO_GT:
229 return OpcodeRHS == BO_LE;
230 default:
231 return false;
232 }
233 }
234
235 // Handle the case where constants are off by one: x <= 4 || x >= 5.
236 APSInt ValueLhsPlus1;
237 if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
238 incrementWithoutOverflow(ValueLHS, ValueLhsPlus1) &&
239 APSInt::compareValues(ValueLhsPlus1, ValueRHS) == 0)
240 return true;
241
242 // Handle cases where the constants are different: x > 4 || x <= 7.
243 if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
244 (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
245 return true;
246
247 // Handle cases where constants are different but both ops are !=, like:
248 // x != 5 || x != 10
249 if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE)
250 return true;
251
252 return false;
253}
254
255static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
256 const APSInt &ValueLHS,
257 BinaryOperatorKind OpcodeRHS,
258 const APSInt &ValueRHS) {
259 const int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
260 switch (OpcodeLHS) {
261 case BO_EQ:
262 return OpcodeRHS == BO_EQ && Comparison == 0;
263 case BO_NE:
264 return (OpcodeRHS == BO_NE && Comparison == 0) ||
265 (OpcodeRHS == BO_EQ && Comparison != 0) ||
266 (OpcodeRHS == BO_LT && Comparison >= 0) ||
267 (OpcodeRHS == BO_LE && Comparison > 0) ||
268 (OpcodeRHS == BO_GT && Comparison <= 0) ||
269 (OpcodeRHS == BO_GE && Comparison < 0);
270
271 case BO_LT:
272 return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
273 (OpcodeRHS == BO_LE && Comparison > 0) ||
274 (OpcodeRHS == BO_EQ && Comparison > 0));
275 case BO_GT:
276 return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
277 (OpcodeRHS == BO_GE && Comparison < 0) ||
278 (OpcodeRHS == BO_EQ && Comparison < 0));
279 case BO_LE:
280 return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
281 Comparison >= 0;
282 case BO_GE:
283 return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
284 Comparison <= 0;
285 default:
286 return false;
287 }
288}
289
290static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
291 APSInt &Value) {
292 if (Opcode == BO_Sub) {
293 Opcode = BO_Add;
294 Value = -Value;
295 }
296}
297
298// to use in the template below
299static OverloadedOperatorKind getOp(const BinaryOperator *Op) {
300 return BinaryOperator::getOverloadedOperator(Op->getOpcode());
301}
302
303static OverloadedOperatorKind getOp(const CXXOperatorCallExpr *Op) {
304 if (Op->getNumArgs() != 2)
305 return OO_None;
306 return Op->getOperator();
307}
308
309static std::pair<const Expr *, const Expr *>
310getOperands(const BinaryOperator *Op) {
311 return {Op->getLHS()->IgnoreParenImpCasts(),
312 Op->getRHS()->IgnoreParenImpCasts()};
313}
314
315static std::pair<const Expr *, const Expr *>
316getOperands(const CXXOperatorCallExpr *Op) {
317 return {Op->getArg(0)->IgnoreParenImpCasts(),
318 Op->getArg(1)->IgnoreParenImpCasts()};
319}
320
321template <typename TExpr>
322static const TExpr *checkOpKind(const Expr *TheExpr,
323 OverloadedOperatorKind OpKind) {
324 const auto *AsTExpr = dyn_cast_or_null<TExpr>(TheExpr);
325 if (AsTExpr && getOp(AsTExpr) == OpKind)
326 return AsTExpr;
327
328 return nullptr;
329}
330
331// returns true if a subexpression has two directly equivalent operands and
332// is already handled by operands/parametersAreEquivalent
333template <typename TExpr, unsigned N>
334static bool collectOperands(const Expr *Part,
335 SmallVector<const Expr *, N> &AllOperands,
336 OverloadedOperatorKind OpKind) {
337 if (const auto *BinOp = checkOpKind<TExpr>(Part, OpKind)) {
338 const std::pair<const Expr *, const Expr *> Operands = getOperands(BinOp);
339 if (areEquivalentExpr(Operands.first, Operands.second))
340 return true;
341 return collectOperands<TExpr>(Operands.first, AllOperands, OpKind) ||
342 collectOperands<TExpr>(Operands.second, AllOperands, OpKind);
343 }
344
345 AllOperands.push_back(Part);
346 return false;
347}
348
349template <typename TExpr>
350static bool hasSameOperatorParent(const Expr *TheExpr,
351 OverloadedOperatorKind OpKind,
352 ASTContext &Context) {
353 // IgnoreParenImpCasts logic in reverse: skip surrounding uninteresting nodes
354 const DynTypedNodeList Parents = Context.getParents(*TheExpr);
355 for (const DynTypedNode DynParent : Parents) {
356 if (const auto *Parent = DynParent.get<Expr>()) {
357 const bool Skip =
358 isa<ParenExpr>(Parent) || isa<ImplicitCastExpr>(Parent) ||
359 isa<FullExpr>(Parent) || isa<MaterializeTemporaryExpr>(Parent);
360 if (Skip && hasSameOperatorParent<TExpr>(Parent, OpKind, Context))
361 return true;
362 if (checkOpKind<TExpr>(Parent, OpKind))
363 return true;
364 }
365 }
366
367 return false;
368}
369
370template <typename TExpr>
371static bool
372markDuplicateOperands(const TExpr *TheExpr,
373 ast_matchers::internal::BoundNodesTreeBuilder *Builder,
374 ASTContext &Context) {
375 const OverloadedOperatorKind OpKind = getOp(TheExpr);
376 if (OpKind == OO_None)
377 return false;
378 // if there are no nested operators of the same kind, it's handled by
379 // operands/parametersAreEquivalent
380 const std::pair<const Expr *, const Expr *> Operands = getOperands(TheExpr);
381 if (!(checkOpKind<TExpr>(Operands.first, OpKind) ||
382 checkOpKind<TExpr>(Operands.second, OpKind)))
383 return false;
384
385 // if parent is the same kind of operator, it's handled by a previous call to
386 // markDuplicateOperands
387 if (hasSameOperatorParent<TExpr>(TheExpr, OpKind, Context))
388 return false;
389
390 SmallVector<const Expr *, 4> AllOperands;
391 if (collectOperands<TExpr>(Operands.first, AllOperands, OpKind))
392 return false;
393 if (collectOperands<TExpr>(Operands.second, AllOperands, OpKind))
394 return false;
395 const size_t NumOperands = AllOperands.size();
396 llvm::SmallBitVector Duplicates(NumOperands);
397 for (size_t I = 0; I < NumOperands; I++) {
398 if (Duplicates[I])
399 continue;
400 bool FoundDuplicates = false;
401
402 for (size_t J = I + 1; J < NumOperands; J++) {
403 if (AllOperands[J]->HasSideEffects(Context))
404 break;
405
406 if (areEquivalentExpr(AllOperands[I], AllOperands[J])) {
407 FoundDuplicates = true;
408 Duplicates.set(J);
409 Builder->setBinding(SmallString<11>(llvm::formatv("duplicate{0}", J)),
410 DynTypedNode::create(*AllOperands[J]));
411 }
412 }
413
414 if (FoundDuplicates)
415 Builder->setBinding(SmallString<11>(llvm::formatv("duplicate{0}", I)),
416 DynTypedNode::create(*AllOperands[I]));
417 }
418
419 return Duplicates.any();
420}
421
422namespace {
423
424AST_MATCHER(Expr, isIntegerConstantExpr) {
425 if (Node.isInstantiationDependent())
426 return false;
427 return Node.isIntegerConstantExpr(Finder->getASTContext());
428}
429
430AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
431 return areEquivalentExpr(Node.getLHS(), Node.getRHS());
432}
433
434AST_MATCHER(BinaryOperator, nestedOperandsAreEquivalent) {
435 return markDuplicateOperands(&Node, Builder, Finder->getASTContext());
436}
437
438AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
439 return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
440}
441
442AST_MATCHER(CallExpr, parametersAreEquivalent) {
443 return Node.getNumArgs() == 2 &&
444 areEquivalentExpr(Node.getArg(0), Node.getArg(1));
445}
446
447AST_MATCHER(CXXOperatorCallExpr, nestedParametersAreEquivalent) {
448 return markDuplicateOperands(&Node, Builder, Finder->getASTContext());
449}
450
451AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
452 return Node.getOperatorLoc().isMacroID();
453}
454
455AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
456 return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
457}
458
459AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
460
461AST_MATCHER_P(Expr, expandedByMacro, ArrayRef<StringRef>, Names) {
462 const SourceManager &SM = Finder->getASTContext().getSourceManager();
463 const LangOptions &LO = Finder->getASTContext().getLangOpts();
464 SourceLocation Loc = Node.getExprLoc();
465 while (Loc.isMacroID()) {
466 const StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
467 if (llvm::is_contained(Names, MacroName))
468 return true;
469 Loc = SM.getImmediateMacroCallerLoc(Loc);
470 }
471 return false;
472}
473
474} // namespace
475
476// Returns a matcher for integer constant expressions.
477static ast_matchers::internal::Matcher<Expr>
479 const std::string CstId = (Id + "-const").str();
480 return expr(isIntegerConstantExpr()).bind(CstId);
481}
482
483// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
484// name 'Id' and stores it into 'ConstExpr', the value of the expression is
485// stored into `Value`.
486static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
487 StringRef Id, APSInt &Value,
488 const Expr *&ConstExpr) {
489 const std::string CstId = (Id + "-const").str();
490 ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
491 if (!ConstExpr)
492 return false;
493 std::optional<llvm::APSInt> R =
494 ConstExpr->getIntegerConstantExpr(*Result.Context);
495 if (!R)
496 return false;
497 Value = *R;
498 return true;
499}
500
501// Overloaded `retrieveIntegerConstantExpr` for compatibility.
502static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
503 StringRef Id, APSInt &Value) {
504 const Expr *ConstExpr = nullptr;
505 return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
506}
507
508// Returns a matcher for symbolic expressions (matches every expression except
509// ingeter constant expressions).
510static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
511 const std::string SymId = (Id + "-sym").str();
512 return ignoringParenImpCasts(
513 expr(unless(isIntegerConstantExpr())).bind(SymId));
514}
515
516// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
517// stores it into 'SymExpr'.
518static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
519 StringRef Id, const Expr *&SymExpr) {
520 const std::string SymId = (Id + "-sym").str();
521 if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
522 SymExpr = Node;
523 return true;
524 }
525 return false;
526}
527
528// Match a binary operator between a symbolic expression and an integer constant
529// expression.
530static ast_matchers::internal::Matcher<Expr>
532 const auto BinOpCstExpr =
533 expr(anyOf(binaryOperator(hasAnyOperatorName("+", "|", "&"),
534 hasOperands(matchSymbolicExpr(Id),
536 binaryOperator(hasOperatorName("-"),
537 hasLHS(matchSymbolicExpr(Id)),
538 hasRHS(matchIntegerConstantExpr(Id)))))
539 .bind(Id);
540 return ignoringParenImpCasts(BinOpCstExpr);
541}
542
543// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
544// name 'Id'.
545static bool
546retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
547 StringRef Id, BinaryOperatorKind &Opcode,
548 const Expr *&Symbol, APSInt &Value) {
549 if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
550 Opcode = BinExpr->getOpcode();
551 return retrieveSymbolicExpr(Result, Id, Symbol) &&
552 retrieveIntegerConstantExpr(Result, Id, Value);
553 }
554 return false;
555}
556
557// Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
558static ast_matchers::internal::Matcher<Expr>
560 const std::string CastId = (Id + "-cast").str();
561 const std::string SwapId = (Id + "-swap").str();
562 const std::string NegateId = (Id + "-negate").str();
563 const std::string OverloadId = (Id + "-overload").str();
564 const std::string ConstId = (Id + "-const").str();
565
566 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
567 isComparisonOperator(), expr().bind(Id),
568 anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
569 hasRHS(matchIntegerConstantExpr(Id))),
570 allOf(hasLHS(matchIntegerConstantExpr(Id)),
571 hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
572
573 // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
574 // to if (x != 0)).
575 const auto CastExpr =
576 implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
577 hasSourceExpression(matchSymbolicExpr(Id)))
578 .bind(CastId);
579
580 const auto NegateRelationalExpr =
581 unaryOperator(hasOperatorName("!"),
582 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
583 .bind(NegateId);
584
585 // Do not bind to double negation.
586 const auto NegateNegateRelationalExpr =
587 unaryOperator(hasOperatorName("!"),
588 hasUnaryOperand(unaryOperator(
589 hasOperatorName("!"),
590 hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
591
592 const auto OverloadedOperatorExpr =
593 cxxOperatorCallExpr(
594 hasAnyOverloadedOperatorName("==", "!=", "<", "<=", ">", ">="),
595 // Filter noisy false positives.
596 unless(isMacro()), unless(isInTemplateInstantiation()),
597 anyOf(hasLHS(ignoringParenImpCasts(integerLiteral().bind(ConstId))),
598 hasRHS(ignoringParenImpCasts(integerLiteral().bind(ConstId)))))
599 .bind(OverloadId);
600
601 return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
602 NegateNegateRelationalExpr, OverloadedOperatorExpr);
603}
604
605// Checks whether a function param is non constant reference type, and may
606// be modified in the function.
607static bool isNonConstReferenceType(QualType ParamType) {
608 return ParamType->isReferenceType() &&
609 !ParamType.getNonReferenceType().isConstQualified();
610}
611
612// Checks whether the arguments of an overloaded operator can be modified in the
613// function.
614// For operators that take an instance and a constant as arguments, only the
615// first argument (the instance) needs to be checked, since the constant itself
616// is a temporary expression. Whether the second parameter is checked is
617// controlled by the parameter `ParamsToCheckCount`.
618static bool
619canOverloadedOperatorArgsBeModified(const CXXOperatorCallExpr *OperatorCall,
620 bool CheckSecondParam) {
621 const auto *OperatorDecl =
622 dyn_cast_or_null<FunctionDecl>(OperatorCall->getCalleeDecl());
623 // if we can't find the declaration, conservatively assume it can modify
624 // arguments
625 if (!OperatorDecl)
626 return true;
627
628 const unsigned ParamCount = OperatorDecl->getNumParams();
629
630 // Overloaded operators declared inside a class have only one param.
631 // These functions must be declared const in order to not be able to modify
632 // the instance of the class they are called through.
633 if (ParamCount == 1 &&
634 !OperatorDecl->getType()->castAs<FunctionType>()->isConst())
635 return true;
636
637 if (isNonConstReferenceType(OperatorDecl->getParamDecl(0)->getType()))
638 return true;
639
640 return CheckSecondParam && ParamCount == 2 &&
641 isNonConstReferenceType(OperatorDecl->getParamDecl(1)->getType());
642}
643
644// Retrieves sub-expressions matched by 'matchRelationalIntegerConstantExpr'
645// with name 'Id'.
647 const MatchFinder::MatchResult &Result, StringRef Id,
648 const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
649 APSInt &Value, const Expr *&ConstExpr) {
650 const std::string CastId = (Id + "-cast").str();
651 const std::string SwapId = (Id + "-swap").str();
652 const std::string NegateId = (Id + "-negate").str();
653 const std::string OverloadId = (Id + "-overload").str();
654
655 if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
656 // Operand received with explicit comparator.
657 Opcode = Bin->getOpcode();
658 OperandExpr = Bin;
659
660 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
661 return false;
662 } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
663 // Operand received with implicit comparator (cast).
664 Opcode = BO_NE;
665 OperandExpr = Cast;
666 Value = APSInt(32, false);
667 } else if (const auto *OverloadedOperatorExpr =
668 Result.Nodes.getNodeAs<CXXOperatorCallExpr>(OverloadId)) {
669 if (canOverloadedOperatorArgsBeModified(OverloadedOperatorExpr, false))
670 return false;
671
672 bool IntegerConstantIsFirstArg = false;
673
674 if (const auto *Arg = OverloadedOperatorExpr->getArg(1)) {
675 if (!Arg->isValueDependent() &&
676 !Arg->isIntegerConstantExpr(*Result.Context)) {
677 IntegerConstantIsFirstArg = true;
678 if (const auto *Arg = OverloadedOperatorExpr->getArg(0)) {
679 if (!Arg->isValueDependent() &&
680 !Arg->isIntegerConstantExpr(*Result.Context))
681 return false;
682 } else {
683 return false;
684 }
685 }
686 } else {
687 return false;
688 }
689
690 Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
691 OperandExpr = OverloadedOperatorExpr;
692 Opcode = BinaryOperator::getOverloadedOpcode(
693 OverloadedOperatorExpr->getOperator());
694
695 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
696 return false;
697
698 if (!BinaryOperator::isComparisonOp(Opcode))
699 return false;
700
701 // The call site of this function expects the constant on the RHS,
702 // so change the opcode accordingly.
703 if (IntegerConstantIsFirstArg)
704 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
705
706 return true;
707 } else {
708 return false;
709 }
710
711 if (!retrieveSymbolicExpr(Result, Id, Symbol))
712 return false;
713
714 if (Result.Nodes.getNodeAs<Expr>(SwapId))
715 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
716 if (Result.Nodes.getNodeAs<Expr>(NegateId))
717 Opcode = BinaryOperator::negateComparisonOp(Opcode);
718 return true;
719}
720
721// Checks for expressions like (X == 4) && (Y != 9)
722static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
723 const ASTContext *AstCtx) {
724 const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
725 const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
726
727 if (!LhsBinOp || !RhsBinOp)
728 return false;
729
730 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
731 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
732 };
733
734 if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
735 IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
736 (IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
737 IsIntegerConstantExpr(RhsBinOp->getRHS())))
738 return true;
739 return false;
740}
741
743 const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
744 if (areSidesBinaryConstExpressions(BinOp, AstCtx))
745 return true;
746
747 const Expr *Lhs = BinOp->getLHS();
748 const Expr *Rhs = BinOp->getRHS();
749
750 if (!Lhs || !Rhs)
751 return false;
752
753 auto IsDefineExpr = [AstCtx](const Expr *E) {
754 const SourceRange Lsr = E->getSourceRange();
755 if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
756 !E->isIntegerConstantExpr(*AstCtx))
757 return false;
758 return true;
759 };
760
761 return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
762}
763
764// Retrieves integer constant subexpressions from binary operator expressions
765// that have two equivalent sides.
766// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
767static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
768 BinaryOperatorKind &MainOpcode,
769 BinaryOperatorKind &SideOpcode,
770 const Expr *&LhsConst,
771 const Expr *&RhsConst,
772 const ASTContext *AstCtx) {
773 assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
774 "Both sides of binary operator must be constant expressions!");
775
776 MainOpcode = BinOp->getOpcode();
777
778 const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
779 const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
780
781 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
782 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
783 };
784
785 LhsConst = IsIntegerConstantExpr(BinOpLhs->getLHS()) ? BinOpLhs->getLHS()
786 : BinOpLhs->getRHS();
787 RhsConst = IsIntegerConstantExpr(BinOpRhs->getLHS()) ? BinOpRhs->getLHS()
788 : BinOpRhs->getRHS();
789
790 if (!LhsConst || !RhsConst)
791 return false;
792
793 assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
794 "Sides of the binary operator must be equivalent expressions!");
795
796 SideOpcode = BinOpLhs->getOpcode();
797
798 return true;
799}
800
801static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
802 const SourceManager &SM) {
803 if (T1.getKind() != T2.getKind())
804 return false;
805 if (T1.isNot(tok::raw_identifier))
806 return true;
807 if (T1.getLength() != T2.getLength())
808 return false;
809 return StringRef(SM.getCharacterData(T1.getLocation()), T1.getLength()) ==
810 StringRef(SM.getCharacterData(T2.getLocation()), T2.getLength());
811}
812
813static bool isTokAtEndOfExpr(SourceRange ExprSR, Token T,
814 const SourceManager &SM) {
815 return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation();
816}
817
818/// Returns true if both LhsExpr and RhsExpr are
819/// macro expressions and they are expanded
820/// from different macros.
821static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
822 const Expr *RhsExpr,
823 const ASTContext *AstCtx) {
824 if (!LhsExpr || !RhsExpr)
825 return false;
826 const SourceRange Lsr = LhsExpr->getSourceRange();
827 const SourceRange Rsr = RhsExpr->getSourceRange();
828 if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
829 return false;
830
831 const SourceManager &SM = AstCtx->getSourceManager();
832 const LangOptions &LO = AstCtx->getLangOpts();
833
834 const std::pair<FileID, unsigned> LsrLocInfo =
835 SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
836 const std::pair<FileID, unsigned> RsrLocInfo =
837 SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
838 const llvm::MemoryBufferRef MB = SM.getBufferOrFake(LsrLocInfo.first);
839
840 const char *LTokenPos = MB.getBufferStart() + LsrLocInfo.second;
841 const char *RTokenPos = MB.getBufferStart() + RsrLocInfo.second;
842 Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
843 MB.getBufferStart(), LTokenPos, MB.getBufferEnd());
844 Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
845 MB.getBufferStart(), RTokenPos, MB.getBufferEnd());
846
847 Token LTok, RTok;
848 do { // Compare the expressions token-by-token.
849 LRawLex.LexFromRawLexer(LTok);
850 RRawLex.LexFromRawLexer(RTok);
851 } while (!LTok.is(tok::eof) && !RTok.is(tok::eof) &&
852 isSameRawIdentifierToken(LTok, RTok, SM) &&
853 !isTokAtEndOfExpr(Lsr, LTok, SM) &&
854 !isTokAtEndOfExpr(Rsr, RTok, SM));
855 return (!isTokAtEndOfExpr(Lsr, LTok, SM) ||
856 !isTokAtEndOfExpr(Rsr, RTok, SM)) ||
857 !isSameRawIdentifierToken(LTok, RTok, SM);
858}
859
860static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
861 const Expr *&RhsExpr) {
862 if (!LhsExpr || !RhsExpr)
863 return false;
864
865 const SourceLocation LhsLoc = LhsExpr->getExprLoc();
866 const SourceLocation RhsLoc = RhsExpr->getExprLoc();
867
868 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
869}
870
871static bool areStringsSameIgnoreSpaces(const llvm::StringRef Left,
872 const llvm::StringRef Right) {
873 if (Left == Right)
874 return true;
875
876 // Do running comparison ignoring spaces
877 llvm::StringRef L = Left.trim();
878 llvm::StringRef R = Right.trim();
879 while (!L.empty() && !R.empty()) {
880 L = L.ltrim();
881 R = R.ltrim();
882 if (L.empty() && R.empty())
883 return true;
884 // If symbol compared are different ==> strings are not the same
885 if (L.front() != R.front())
886 return false;
887 L = L.drop_front();
888 R = R.drop_front();
889 }
890 return L.empty() && R.empty();
891}
892
893static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
894 const ASTContext *Context) {
895 if (!BinOp)
896 return false;
897
898 const Expr *Lhs = BinOp->getLHS();
899 const Expr *Rhs = BinOp->getRHS();
900 const SourceManager &SM = Context->getSourceManager();
901
902 const SourceRange Lsr = Lhs->getSourceRange();
903 const SourceRange Rsr = Rhs->getSourceRange();
904 if (Lsr.getBegin().isMacroID()) {
905 // Left is macro so right macro too
906 if (Rsr.getBegin().isMacroID()) {
907 // Both sides are macros so they are same macro or literal
908 const llvm::StringRef L = Lexer::getSourceText(
909 CharSourceRange::getTokenRange(Lsr), SM, Context->getLangOpts());
910 const llvm::StringRef R = Lexer::getSourceText(
911 CharSourceRange::getTokenRange(Rsr), SM, Context->getLangOpts());
912 return areStringsSameIgnoreSpaces(L, R);
913 }
914 // Left is macro but right is not so they are not same macro or literal
915 return false;
916 }
917 const auto *Lil = dyn_cast<IntegerLiteral>(Lhs);
918 const auto *Ril = dyn_cast<IntegerLiteral>(Rhs);
919 if (Lil && Ril)
920 return Lil->getValue() == Ril->getValue();
921
922 const auto *Lbl = dyn_cast<CXXBoolLiteralExpr>(Lhs);
923 const auto *Rbl = dyn_cast<CXXBoolLiteralExpr>(Rhs);
924 if (Lbl && Rbl)
925 return Lbl->getValue() == Rbl->getValue();
926
927 return false;
928}
929
931 const auto BannedIntegerLiteral =
932 integerLiteral(expandedByMacro(KnownBannedMacroNames));
933 const auto IsInUnevaluatedContext = expr(anyOf(
934 hasAncestor(expr(hasUnevaluatedContext())), hasAncestor(typeLoc())));
935
936 // Binary with equivalent operands, like (X != 2 && X != 2).
937 Finder->addMatcher(
938 traverse(TK_AsIs,
939 binaryOperator(anyOf(isComparisonOperator(),
940 hasAnyOperatorName("-", "/", "%", "|", "&",
941 "^", "&&", "||", "=")),
942 operandsAreEquivalent(),
943 // Filter noisy false positives.
944 unless(isInTemplateInstantiation()),
945 unless(binaryOperatorIsInMacro()),
946 unless(hasAncestor(arraySubscriptExpr())),
947 unless(hasDescendant(BannedIntegerLiteral)),
948 unless(IsInUnevaluatedContext))
949 .bind("binary")),
950 this);
951
952 // Logical or bitwise operator with equivalent nested operands, like (X && Y
953 // && X) or (X && (Y && X))
954 Finder->addMatcher(
955 binaryOperator(hasAnyOperatorName("|", "&", "||", "&&", "^"),
956 nestedOperandsAreEquivalent(),
957 // Filter noisy false positives.
958 unless(isInTemplateInstantiation()),
959 unless(binaryOperatorIsInMacro()),
960 // TODO: if the banned macros are themselves duplicated
961 unless(hasDescendant(BannedIntegerLiteral)),
962 unless(IsInUnevaluatedContext))
963 .bind("nested-duplicates"),
964 this);
965
966 // Conditional (ternary) operator with equivalent operands, like (Y ? X : X).
967 Finder->addMatcher(
968 traverse(TK_AsIs,
969 conditionalOperator(expressionsAreEquivalent(),
970 // Filter noisy false positives.
971 unless(conditionalOperatorIsInMacro()),
972 unless(isInTemplateInstantiation()),
973 unless(IsInUnevaluatedContext))
974 .bind("cond")),
975 this);
976
977 // Overloaded operators with equivalent operands.
978 Finder->addMatcher(
979 traverse(TK_AsIs,
980 cxxOperatorCallExpr(
981 hasAnyOverloadedOperatorName("-", "/", "%", "|", "&", "^",
982 "==", "!=", "<", "<=", ">",
983 ">=", "&&", "||", "="),
984 parametersAreEquivalent(),
985 // Filter noisy false positives.
986 unless(isMacro()), unless(isInTemplateInstantiation()),
987 unless(IsInUnevaluatedContext))
988 .bind("call")),
989 this);
990
991 // Overloaded operators with equivalent operands.
992 Finder->addMatcher(
993 cxxOperatorCallExpr(
994 hasAnyOverloadedOperatorName("|", "&", "||", "&&", "^"),
995 nestedParametersAreEquivalent(), argumentCountIs(2),
996 // Filter noisy false positives.
997 unless(isMacro()), unless(isInTemplateInstantiation()),
998 unless(IsInUnevaluatedContext))
999 .bind("nested-duplicates"),
1000 this);
1001
1002 // Match expressions like: !(1 | 2 | 3)
1003 Finder->addMatcher(
1004 traverse(TK_AsIs,
1005 implicitCastExpr(
1006 hasImplicitDestinationType(isInteger()),
1007 has(unaryOperator(
1008 hasOperatorName("!"),
1009 hasUnaryOperand(ignoringParenImpCasts(binaryOperator(
1010 hasAnyOperatorName("|", "&"),
1011 hasLHS(anyOf(
1012 binaryOperator(hasAnyOperatorName("|", "&")),
1013 integerLiteral())),
1014 hasRHS(integerLiteral())))))
1015 .bind("logical-bitwise-confusion")),
1016 unless(IsInUnevaluatedContext))),
1017 this);
1018
1019 // Match expressions like: (X << 8) & 0xFF
1020 Finder->addMatcher(
1021 traverse(TK_AsIs,
1022 binaryOperator(
1023 hasOperatorName("&"),
1024 hasOperands(ignoringParenImpCasts(binaryOperator(
1025 hasOperatorName("<<"),
1026 hasRHS(ignoringParenImpCasts(
1027 integerLiteral().bind("shift-const"))))),
1028 ignoringParenImpCasts(
1029 integerLiteral().bind("and-const"))),
1030 unless(IsInUnevaluatedContext))
1031 .bind("left-right-shift-confusion")),
1032 this);
1033
1034 // Match common expressions and apply more checks to find redundant
1035 // sub-expressions.
1036 // a) Expr <op> K1 == K2
1037 // b) Expr <op> K1 == Expr
1038 // c) Expr <op> K1 == Expr <op> K2
1039 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
1040 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
1041 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
1042 const auto CstRight = matchIntegerConstantExpr("rhs");
1043 const auto SymRight = matchSymbolicExpr("rhs");
1044
1045 // Match expressions like: x <op> 0xFF == 0xF00.
1046 Finder->addMatcher(
1047 traverse(TK_AsIs, binaryOperator(isComparisonOperator(),
1048 hasOperands(BinOpCstLeft, CstRight),
1049 unless(IsInUnevaluatedContext))
1050 .bind("binop-const-compare-to-const")),
1051 this);
1052
1053 // Match expressions like: x <op> 0xFF == x.
1054 Finder->addMatcher(
1055 traverse(
1056 TK_AsIs,
1057 binaryOperator(isComparisonOperator(),
1058 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
1059 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))),
1060 unless(IsInUnevaluatedContext))
1061 .bind("binop-const-compare-to-sym")),
1062 this);
1063
1064 // Match expressions like: x <op> 10 == x <op> 12.
1065 Finder->addMatcher(
1066 traverse(TK_AsIs,
1067 binaryOperator(isComparisonOperator(), hasLHS(BinOpCstLeft),
1068 hasRHS(BinOpCstRight),
1069 // Already reported as redundant.
1070 unless(operandsAreEquivalent()),
1071 unless(IsInUnevaluatedContext))
1072 .bind("binop-const-compare-to-binop-const")),
1073 this);
1074
1075 // Match relational expressions combined with logical operators and find
1076 // redundant sub-expressions.
1077 // see: 'checkRelationalExpr'
1078
1079 // Match expressions like: x < 2 && x > 2.
1080 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
1081 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
1082 Finder->addMatcher(
1083 traverse(TK_AsIs,
1084 binaryOperator(hasAnyOperatorName("||", "&&"),
1085 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
1086 // Already reported as redundant.
1087 unless(operandsAreEquivalent()),
1088 unless(IsInUnevaluatedContext))
1089 .bind("comparisons-of-symbol-and-const")),
1090 this);
1091}
1092
1093void RedundantExpressionCheck::checkArithmeticExpr(
1094 const MatchFinder::MatchResult &Result) {
1095 APSInt LhsValue, RhsValue;
1096 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
1097 BinaryOperatorKind LhsOpcode{}, RhsOpcode{};
1098
1099 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
1100 "binop-const-compare-to-sym")) {
1101 const BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1102 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
1103 LhsValue) ||
1104 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
1105 !areEquivalentExpr(LhsSymbol, RhsSymbol))
1106 return;
1107
1108 // Check expressions: x + k == x or x - k == x.
1109 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
1110 if ((LhsValue != 0 && Opcode == BO_EQ) ||
1111 (LhsValue == 0 && Opcode == BO_NE))
1112 diag(ComparisonOperator->getOperatorLoc(),
1113 "logical expression is always false");
1114 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
1115 (LhsValue != 0 && Opcode == BO_NE))
1116 diag(ComparisonOperator->getOperatorLoc(),
1117 "logical expression is always true");
1118 }
1119 } else if (const auto *ComparisonOperator =
1120 Result.Nodes.getNodeAs<BinaryOperator>(
1121 "binop-const-compare-to-binop-const")) {
1122 const BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1123
1124 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
1125 LhsValue) ||
1126 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
1127 RhsValue) ||
1128 !areEquivalentExpr(LhsSymbol, RhsSymbol))
1129 return;
1130
1131 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
1132 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
1133
1134 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
1135 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
1136 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
1137 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
1138 diag(ComparisonOperator->getOperatorLoc(),
1139 "logical expression is always true");
1140 } else if ((Opcode == BO_EQ &&
1141 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
1142 (Opcode == BO_NE &&
1143 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
1144 diag(ComparisonOperator->getOperatorLoc(),
1145 "logical expression is always false");
1146 }
1147 }
1148 }
1149}
1150
1151static bool exprEvaluatesToZero(BinaryOperatorKind Opcode,
1152 const APSInt &Value) {
1153 return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
1154}
1155
1156static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
1157 const APSInt &Value) {
1158 return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
1159}
1160
1161static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode,
1162 const APSInt &Value) {
1163 return ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) ||
1164 ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
1165}
1166
1167void RedundantExpressionCheck::checkBitwiseExpr(
1168 const MatchFinder::MatchResult &Result) {
1169 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
1170 "binop-const-compare-to-const")) {
1171 const BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1172
1173 APSInt LhsValue, RhsValue;
1174 const Expr *LhsSymbol = nullptr;
1175 BinaryOperatorKind LhsOpcode{};
1176 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
1177 LhsValue) ||
1178 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
1179 return;
1180
1181 const uint64_t LhsConstant = LhsValue.getZExtValue();
1182 const uint64_t RhsConstant = RhsValue.getZExtValue();
1183 const SourceLocation Loc = ComparisonOperator->getOperatorLoc();
1184
1185 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
1186 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
1187 if (Opcode == BO_EQ)
1188 diag(Loc, "logical expression is always false");
1189 else if (Opcode == BO_NE)
1190 diag(Loc, "logical expression is always true");
1191 }
1192
1193 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
1194 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
1195 if (Opcode == BO_EQ)
1196 diag(Loc, "logical expression is always false");
1197 else if (Opcode == BO_NE)
1198 diag(Loc, "logical expression is always true");
1199 }
1200 } else if (const auto *IneffectiveOperator =
1201 Result.Nodes.getNodeAs<BinaryOperator>(
1202 "ineffective-bitwise")) {
1203 APSInt Value;
1204 const Expr *Sym = nullptr, *ConstExpr = nullptr;
1205
1206 if (!retrieveSymbolicExpr(Result, "ineffective-bitwise", Sym) ||
1207 !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value,
1208 ConstExpr))
1209 return;
1210
1211 if ((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
1212 return;
1213
1214 const SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
1215
1216 const BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode();
1217 if (exprEvaluatesToZero(Opcode, Value)) {
1218 diag(Loc, "expression always evaluates to 0");
1219 } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) {
1220 const SourceRange ConstExprRange(ConstExpr->getBeginLoc(),
1221 ConstExpr->getEndLoc());
1222 const StringRef ConstExprText = Lexer::getSourceText(
1223 CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager,
1224 Result.Context->getLangOpts());
1225
1226 diag(Loc, "expression always evaluates to '%0'") << ConstExprText;
1227
1228 } else if (exprEvaluatesToSymbolic(Opcode, Value)) {
1229 const SourceRange SymExprRange(Sym->getBeginLoc(), Sym->getEndLoc());
1230
1231 const StringRef ExprText = Lexer::getSourceText(
1232 CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager,
1233 Result.Context->getLangOpts());
1234
1235 diag(Loc, "expression always evaluates to '%0'") << ExprText;
1236 }
1237 }
1238}
1239
1240void RedundantExpressionCheck::checkRelationalExpr(
1241 const MatchFinder::MatchResult &Result) {
1242 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
1243 "comparisons-of-symbol-and-const")) {
1244 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
1245 // E.g.: (X < 2) && (X > 4)
1246 const BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1247
1248 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
1249 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
1250 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
1251 BinaryOperatorKind LhsOpcode{}, RhsOpcode{};
1252 APSInt LhsValue, RhsValue;
1253
1255 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
1257 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
1258 !areEquivalentExpr(LhsSymbol, RhsSymbol))
1259 return;
1260
1261 // Bring expr to a canonical form: smallest constant must be on the left.
1262 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
1263 std::swap(LhsExpr, RhsExpr);
1264 std::swap(LhsValue, RhsValue);
1265 std::swap(LhsSymbol, RhsSymbol);
1266 std::swap(LhsOpcode, RhsOpcode);
1267 }
1268
1269 // Constants come from two different macros, or one of them is a macro.
1270 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
1271 areExprsMacroAndNonMacro(LhsConst, RhsConst))
1272 return;
1273
1274 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
1275 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1276 diag(ComparisonOperator->getOperatorLoc(),
1277 "equivalent expression on both sides of logical operator");
1278 return;
1279 }
1280
1281 if (Opcode == BO_LAnd) {
1282 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1283 diag(ComparisonOperator->getOperatorLoc(),
1284 "logical expression is always false");
1285 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1286 diag(LhsExpr->getExprLoc(), "expression is redundant");
1287 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
1288 diag(RhsExpr->getExprLoc(), "expression is redundant");
1289 }
1290 }
1291
1292 if (Opcode == BO_LOr) {
1293 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1294 diag(ComparisonOperator->getOperatorLoc(),
1295 "logical expression is always true");
1296 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1297 diag(RhsExpr->getExprLoc(), "expression is redundant");
1298 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
1299 diag(LhsExpr->getExprLoc(), "expression is redundant");
1300 }
1301 }
1302 }
1303}
1304
1305void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
1306 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
1307 // If the expression's constants are macros, check whether they are
1308 // intentional.
1309
1310 //
1311 // Special case for floating-point representation.
1312 //
1313 // If expressions on both sides of comparison operator are of type float,
1314 // then for some comparison operators no warning shall be
1315 // reported even if the expressions are identical from a symbolic point of
1316 // view. Comparison between expressions, declared variables and literals
1317 // are treated differently.
1318 //
1319 // != and == between float literals that have the same value should NOT
1320 // warn. < > between float literals that have the same value SHOULD warn.
1321 //
1322 // != and == between the same float declaration should NOT warn.
1323 // < > between the same float declaration SHOULD warn.
1324 //
1325 // != and == between eq. expressions that evaluates into float
1326 // should NOT warn.
1327 // < > between eq. expressions that evaluates into float
1328 // should NOT warn.
1329 //
1330 const Expr *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
1331 const Expr *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
1332 const BinaryOperator::Opcode Op = BinOp->getOpcode();
1333 const bool OpEqualEQorNE = ((Op == BO_EQ) || (Op == BO_NE));
1334
1335 const auto *DeclRef1 = dyn_cast<DeclRefExpr>(LHS);
1336 const auto *DeclRef2 = dyn_cast<DeclRefExpr>(RHS);
1337 const auto *FloatLit1 = dyn_cast<FloatingLiteral>(LHS);
1338 const auto *FloatLit2 = dyn_cast<FloatingLiteral>(RHS);
1339
1340 if (DeclRef1 && DeclRef2 &&
1341 DeclRef1->getType()->hasFloatingRepresentation() &&
1342 DeclRef2->getType()->hasFloatingRepresentation() &&
1343 (DeclRef1->getDecl() == DeclRef2->getDecl()) && OpEqualEQorNE) {
1344 return;
1345 }
1346
1347 if (FloatLit1 && FloatLit2 &&
1348 FloatLit1->getValue().bitwiseIsEqual(FloatLit2->getValue()) &&
1349 OpEqualEQorNE) {
1350 return;
1351 }
1352
1354 BinOp, Result.Context)) {
1355 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
1356 BinaryOperatorKind MainOpcode{}, SideOpcode{};
1357 if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
1358 if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
1359 LhsConst, RhsConst, Result.Context))
1360 return;
1361
1362 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
1363 areExprsMacroAndNonMacro(LhsConst, RhsConst))
1364 return;
1365 } else {
1366 if (!areExprsSameMacroOrLiteral(BinOp, Result.Context))
1367 return;
1368 }
1369 }
1370 diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
1371 }
1372
1373 if (const auto *CondOp =
1374 Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
1375 const Expr *TrueExpr = CondOp->getTrueExpr();
1376 const Expr *FalseExpr = CondOp->getFalseExpr();
1377
1378 if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
1379 areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
1380 return;
1381 diag(CondOp->getColonLoc(),
1382 "'true' and 'false' expressions are equivalent");
1383 }
1384
1385 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
1387 return;
1388
1389 diag(Call->getOperatorLoc(),
1390 "both sides of overloaded operator are equivalent");
1391 }
1392
1393 if (const auto *Op = Result.Nodes.getNodeAs<Expr>("nested-duplicates")) {
1394 const auto *Call = dyn_cast<CXXOperatorCallExpr>(Op);
1395 if (Call && canOverloadedOperatorArgsBeModified(Call, true))
1396 return;
1397
1398 const StringRef Message =
1399 Call ? "overloaded operator has equivalent nested operands"
1400 : "operator has equivalent nested operands";
1401
1402 const auto Diag = diag(Op->getExprLoc(), Message);
1403 for (const auto &KeyValue : Result.Nodes.getMap())
1404 if (StringRef(KeyValue.first).starts_with("duplicate"))
1405 Diag << KeyValue.second.getSourceRange();
1406 }
1407
1408 if (const auto *NegateOperator =
1409 Result.Nodes.getNodeAs<UnaryOperator>("logical-bitwise-confusion")) {
1410 const SourceLocation OperatorLoc = NegateOperator->getOperatorLoc();
1411
1412 auto Diag =
1413 diag(OperatorLoc,
1414 "ineffective logical negation operator used; did you mean '~'?");
1415 const SourceLocation LogicalNotLocation = OperatorLoc.getLocWithOffset(1);
1416
1417 if (!LogicalNotLocation.isMacroID())
1418 Diag << FixItHint::CreateReplacement(
1419 CharSourceRange::getCharRange(OperatorLoc, LogicalNotLocation), "~");
1420 }
1421
1422 if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs<BinaryOperator>(
1423 "left-right-shift-confusion")) {
1424 const auto *ShiftingConst = Result.Nodes.getNodeAs<Expr>("shift-const");
1425 assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!");
1426 std::optional<llvm::APSInt> ShiftingValue =
1427 ShiftingConst->getIntegerConstantExpr(*Result.Context);
1428
1429 if (!ShiftingValue)
1430 return;
1431
1432 const auto *AndConst = Result.Nodes.getNodeAs<Expr>("and-const");
1433 assert(AndConst && "Expr* 'AndCont' is nullptr!");
1434 std::optional<llvm::APSInt> AndValue =
1435 AndConst->getIntegerConstantExpr(*Result.Context);
1436 if (!AndValue)
1437 return;
1438
1439 // If ShiftingConst is shifted left with more bits than the position of the
1440 // leftmost 1 in the bit representation of AndValue, AndConstant is
1441 // ineffective.
1442 if (AndValue->getActiveBits() > *ShiftingValue)
1443 return;
1444
1445 auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
1446 "ineffective bitwise and operation");
1447 }
1448
1449 // Check for the following bound expressions:
1450 // - "binop-const-compare-to-sym",
1451 // - "binop-const-compare-to-binop-const",
1452 // Produced message:
1453 // -> "logical expression is always false/true"
1454 checkArithmeticExpr(Result);
1455
1456 // Check for the following bound expression:
1457 // - "binop-const-compare-to-const",
1458 // - "ineffective-bitwise"
1459 // Produced message:
1460 // -> "logical expression is always false/true"
1461 // -> "expression always evaluates to ..."
1462 checkBitwiseExpr(Result);
1463
1464 // Check for te following bound expression:
1465 // - "comparisons-of-symbol-and-const",
1466 // Produced messages:
1467 // -> "equivalent expression on both sides of logical operator",
1468 // -> "logical expression is always false/true"
1469 // -> "expression is redundant"
1470 checkRelationalExpr(Result);
1471}
1472
1473} // namespace clang::tidy::misc
void registerMatchers(ast_matchers::MatchFinder *Finder) override
void check(const ast_matchers::MatchFinder::MatchResult &Result) override
AST_MATCHER_P(Stmt, isStatementIdenticalToBoundNode, std::string, ID)
AST_MATCHER(BinaryOperator, isRelationalOperator)
static bool areExprsFromDifferentMacros(const Expr *LhsExpr, const Expr *RhsExpr, const ASTContext *AstCtx)
Returns true if both LhsExpr and RhsExpr are macro expressions and they are expanded from different m...
static const TExpr * checkOpKind(const Expr *TheExpr, OverloadedOperatorKind OpKind)
static constexpr StringRef KnownBannedMacroNames[]
static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx)
static bool markDuplicateOperands(const TExpr *TheExpr, ast_matchers::internal::BoundNodesTreeBuilder *Builder, ASTContext &Context)
static bool collectOperands(const Expr *Part, SmallVector< const Expr *, N > &AllOperands, OverloadedOperatorKind OpKind)
static std::pair< const Expr *, const Expr * > getOperands(const BinaryOperator *Op)
static constexpr StringRef Message
static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp, BinaryOperatorKind &MainOpcode, BinaryOperatorKind &SideOpcode, const Expr *&LhsConst, const Expr *&RhsConst, const ASTContext *AstCtx)
static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode, APSInt &Value)
static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result, StringRef Id, APSInt &Value, const Expr *&ConstExpr)
static bool retrieveRelationalIntegerConstantExpr(const MatchFinder::MatchResult &Result, StringRef Id, const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol, APSInt &Value, const Expr *&ConstExpr)
static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS, const APSInt &ValueLHS, BinaryOperatorKind OpcodeRHS, const APSInt &ValueRHS)
static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS, const APSInt &ValueLHS, BinaryOperatorKind OpcodeRHS, const APSInt &ValueRHS)
static ast_matchers::internal::Matcher< Expr > matchSymbolicExpr(StringRef Id)
static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result)
static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr, const Expr *&RhsExpr)
static bool isSameRawIdentifierToken(const Token &T1, const Token &T2, const SourceManager &SM)
static bool areStringsSameIgnoreSpaces(const llvm::StringRef Left, const llvm::StringRef Right)
static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(const BinaryOperator *&BinOp, const ASTContext *AstCtx)
static bool isTokAtEndOfExpr(SourceRange ExprSR, Token T, const SourceManager &SM)
static bool areEquivalentExpr(const Expr *Left, const Expr *Right)
static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS, const APSInt &ValueLHS, BinaryOperatorKind OpcodeRHS, const APSInt &ValueRHS)
static bool isNonConstReferenceType(QualType ParamType)
static ast_matchers::internal::Matcher< Expr > matchIntegerConstantExpr(StringRef Id)
static OverloadedOperatorKind getOp(const BinaryOperator *Op)
static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode, const APSInt &Value)
static ast_matchers::internal::Matcher< Expr > matchBinOpIntegerConstantExpr(StringRef Id)
static bool hasSameOperatorParent(const Expr *TheExpr, OverloadedOperatorKind OpKind, ASTContext &Context)
static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result, StringRef Id, const Expr *&SymExpr)
static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS, const APSInt &ValueLHS, BinaryOperatorKind OpcodeRHS, const APSInt &ValueRHS)
static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp, const ASTContext *Context)
static bool retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result, StringRef Id, BinaryOperatorKind &Opcode, const Expr *&Symbol, APSInt &Value)
static ast_matchers::internal::Matcher< Expr > matchRelationalIntegerConstantExpr(StringRef Id)
static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, const APSInt &Value)
static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, const APSInt &Value)
static bool canOverloadedOperatorArgsBeModified(const CXXOperatorCallExpr *OperatorCall, bool CheckSecondParam)