clang-tools 22.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 llvm::StringLiteral 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 llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
81 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 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 (DynTypedNode DynParent : Parents) {
356 if (const auto *Parent = DynParent.get<Expr>()) {
357 bool Skip = isa<ParenExpr>(Parent) || isa<ImplicitCastExpr>(Parent) ||
358 isa<FullExpr>(Parent) ||
359 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 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<llvm::StringLiteral>, 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 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 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 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 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 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) &&
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 std::string CastId = (Id + "-cast").str();
561 std::string SwapId = (Id + "-swap").str();
562 std::string NegateId = (Id + "-negate").str();
563 std::string OverloadId = (Id + "-overload").str();
564 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 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 std::string CastId = (Id + "-cast").str();
651 std::string SwapId = (Id + "-swap").str();
652 std::string NegateId = (Id + "-negate").str();
653 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 } else
686 return false;
687
688 Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
689 OperandExpr = OverloadedOperatorExpr;
690 Opcode = BinaryOperator::getOverloadedOpcode(
691 OverloadedOperatorExpr->getOperator());
692
693 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
694 return false;
695
696 if (!BinaryOperator::isComparisonOp(Opcode))
697 return false;
698
699 // The call site of this function expects the constant on the RHS,
700 // so change the opcode accordingly.
701 if (IntegerConstantIsFirstArg)
702 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
703
704 return true;
705 } else {
706 return false;
707 }
708
709 if (!retrieveSymbolicExpr(Result, Id, Symbol))
710 return false;
711
712 if (Result.Nodes.getNodeAs<Expr>(SwapId))
713 Opcode = BinaryOperator::reverseComparisonOp(Opcode);
714 if (Result.Nodes.getNodeAs<Expr>(NegateId))
715 Opcode = BinaryOperator::negateComparisonOp(Opcode);
716 return true;
717}
718
719// Checks for expressions like (X == 4) && (Y != 9)
720static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
721 const ASTContext *AstCtx) {
722 const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
723 const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
724
725 if (!LhsBinOp || !RhsBinOp)
726 return false;
727
728 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
729 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
730 };
731
732 if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
733 IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
734 (IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
735 IsIntegerConstantExpr(RhsBinOp->getRHS())))
736 return true;
737 return false;
738}
739
741 const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
742 if (areSidesBinaryConstExpressions(BinOp, AstCtx))
743 return true;
744
745 const Expr *Lhs = BinOp->getLHS();
746 const Expr *Rhs = BinOp->getRHS();
747
748 if (!Lhs || !Rhs)
749 return false;
750
751 auto IsDefineExpr = [AstCtx](const Expr *E) {
752 const SourceRange Lsr = E->getSourceRange();
753 if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
754 !E->isIntegerConstantExpr(*AstCtx))
755 return false;
756 return true;
757 };
758
759 return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
760}
761
762// Retrieves integer constant subexpressions from binary operator expressions
763// that have two equivalent sides.
764// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
765static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
766 BinaryOperatorKind &MainOpcode,
767 BinaryOperatorKind &SideOpcode,
768 const Expr *&LhsConst,
769 const Expr *&RhsConst,
770 const ASTContext *AstCtx) {
771 assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
772 "Both sides of binary operator must be constant expressions!");
773
774 MainOpcode = BinOp->getOpcode();
775
776 const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
777 const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
778
779 auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
780 return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
781 };
782
783 LhsConst = IsIntegerConstantExpr(BinOpLhs->getLHS()) ? BinOpLhs->getLHS()
784 : BinOpLhs->getRHS();
785 RhsConst = IsIntegerConstantExpr(BinOpRhs->getLHS()) ? BinOpRhs->getLHS()
786 : BinOpRhs->getRHS();
787
788 if (!LhsConst || !RhsConst)
789 return false;
790
791 assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
792 "Sides of the binary operator must be equivalent expressions!");
793
794 SideOpcode = BinOpLhs->getOpcode();
795
796 return true;
797}
798
799static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
800 const SourceManager &SM) {
801 if (T1.getKind() != T2.getKind())
802 return false;
803 if (T1.isNot(tok::raw_identifier))
804 return true;
805 if (T1.getLength() != T2.getLength())
806 return false;
807 return StringRef(SM.getCharacterData(T1.getLocation()), T1.getLength()) ==
808 StringRef(SM.getCharacterData(T2.getLocation()), T2.getLength());
809}
810
811static bool isTokAtEndOfExpr(SourceRange ExprSR, Token T,
812 const SourceManager &SM) {
813 return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation();
814}
815
816/// Returns true if both LhsExpr and RhsExpr are
817/// macro expressions and they are expanded
818/// from different macros.
819static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
820 const Expr *RhsExpr,
821 const ASTContext *AstCtx) {
822 if (!LhsExpr || !RhsExpr)
823 return false;
824 const SourceRange Lsr = LhsExpr->getSourceRange();
825 const SourceRange Rsr = RhsExpr->getSourceRange();
826 if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
827 return false;
828
829 const SourceManager &SM = AstCtx->getSourceManager();
830 const LangOptions &LO = AstCtx->getLangOpts();
831
832 std::pair<FileID, unsigned> LsrLocInfo =
833 SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
834 std::pair<FileID, unsigned> RsrLocInfo =
835 SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
836 llvm::MemoryBufferRef MB = SM.getBufferOrFake(LsrLocInfo.first);
837
838 const char *LTokenPos = MB.getBufferStart() + LsrLocInfo.second;
839 const char *RTokenPos = MB.getBufferStart() + RsrLocInfo.second;
840 Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
841 MB.getBufferStart(), LTokenPos, MB.getBufferEnd());
842 Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
843 MB.getBufferStart(), RTokenPos, MB.getBufferEnd());
844
845 Token LTok, RTok;
846 do { // Compare the expressions token-by-token.
847 LRawLex.LexFromRawLexer(LTok);
848 RRawLex.LexFromRawLexer(RTok);
849 } while (!LTok.is(tok::eof) && !RTok.is(tok::eof) &&
850 isSameRawIdentifierToken(LTok, RTok, SM) &&
851 !isTokAtEndOfExpr(Lsr, LTok, SM) &&
852 !isTokAtEndOfExpr(Rsr, RTok, SM));
853 return (!isTokAtEndOfExpr(Lsr, LTok, SM) ||
854 !isTokAtEndOfExpr(Rsr, RTok, SM)) ||
855 !isSameRawIdentifierToken(LTok, RTok, SM);
856}
857
858static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
859 const Expr *&RhsExpr) {
860 if (!LhsExpr || !RhsExpr)
861 return false;
862
863 const SourceLocation LhsLoc = LhsExpr->getExprLoc();
864 const SourceLocation RhsLoc = RhsExpr->getExprLoc();
865
866 return LhsLoc.isMacroID() != RhsLoc.isMacroID();
867}
868
869static bool areStringsSameIgnoreSpaces(const llvm::StringRef Left,
870 const llvm::StringRef Right) {
871 if (Left == Right)
872 return true;
873
874 // Do running comparison ignoring spaces
875 llvm::StringRef L = Left.trim();
876 llvm::StringRef R = Right.trim();
877 while (!L.empty() && !R.empty()) {
878 L = L.ltrim();
879 R = R.ltrim();
880 if (L.empty() && R.empty())
881 return true;
882 // If symbol compared are different ==> strings are not the same
883 if (L.front() != R.front())
884 return false;
885 L = L.drop_front();
886 R = R.drop_front();
887 }
888 return L.empty() && R.empty();
889}
890
891static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
892 const ASTContext *Context) {
893
894 if (!BinOp)
895 return false;
896
897 const Expr *Lhs = BinOp->getLHS();
898 const Expr *Rhs = BinOp->getRHS();
899 const SourceManager &SM = Context->getSourceManager();
900
901 const SourceRange Lsr = Lhs->getSourceRange();
902 const SourceRange Rsr = Rhs->getSourceRange();
903 if (Lsr.getBegin().isMacroID()) {
904 // Left is macro so right macro too
905 if (Rsr.getBegin().isMacroID()) {
906 // Both sides are macros so they are same macro or literal
907 const llvm::StringRef L = Lexer::getSourceText(
908 CharSourceRange::getTokenRange(Lsr), SM, Context->getLangOpts());
909 const llvm::StringRef R = Lexer::getSourceText(
910 CharSourceRange::getTokenRange(Rsr), SM, Context->getLangOpts());
911 return areStringsSameIgnoreSpaces(L, R);
912 }
913 // Left is macro but right is not so they are not same macro or literal
914 return false;
915 }
916 const auto *Lil = dyn_cast<IntegerLiteral>(Lhs);
917 const auto *Ril = dyn_cast<IntegerLiteral>(Rhs);
918 if (Lil && Ril)
919 return Lil->getValue() == Ril->getValue();
920
921 const auto *Lbl = dyn_cast<CXXBoolLiteralExpr>(Lhs);
922 const auto *Rbl = dyn_cast<CXXBoolLiteralExpr>(Rhs);
923 if (Lbl && Rbl)
924 return Lbl->getValue() == Rbl->getValue();
925
926 return false;
927}
928
930 const auto BannedIntegerLiteral =
931 integerLiteral(expandedByMacro(KnownBannedMacroNames));
932 const auto IsInUnevaluatedContext = expr(anyOf(
933 hasAncestor(expr(hasUnevaluatedContext())), hasAncestor(typeLoc())));
934
935 // Binary with equivalent operands, like (X != 2 && X != 2).
936 Finder->addMatcher(
937 traverse(TK_AsIs,
938 binaryOperator(anyOf(isComparisonOperator(),
939 hasAnyOperatorName("-", "/", "%", "|", "&",
940 "^", "&&", "||", "=")),
941 operandsAreEquivalent(),
942 // Filter noisy false positives.
943 unless(isInTemplateInstantiation()),
944 unless(binaryOperatorIsInMacro()),
945 unless(hasAncestor(arraySubscriptExpr())),
946 unless(hasDescendant(BannedIntegerLiteral)),
947 unless(IsInUnevaluatedContext))
948 .bind("binary")),
949 this);
950
951 // Logical or bitwise operator with equivalent nested operands, like (X && Y
952 // && X) or (X && (Y && X))
953 Finder->addMatcher(
954 binaryOperator(hasAnyOperatorName("|", "&", "||", "&&", "^"),
955 nestedOperandsAreEquivalent(),
956 // Filter noisy false positives.
957 unless(isInTemplateInstantiation()),
958 unless(binaryOperatorIsInMacro()),
959 // TODO: if the banned macros are themselves duplicated
960 unless(hasDescendant(BannedIntegerLiteral)),
961 unless(IsInUnevaluatedContext))
962 .bind("nested-duplicates"),
963 this);
964
965 // Conditional (ternary) operator with equivalent operands, like (Y ? X : X).
966 Finder->addMatcher(
967 traverse(TK_AsIs,
968 conditionalOperator(expressionsAreEquivalent(),
969 // Filter noisy false positives.
970 unless(conditionalOperatorIsInMacro()),
971 unless(isInTemplateInstantiation()),
972 unless(IsInUnevaluatedContext))
973 .bind("cond")),
974 this);
975
976 // Overloaded operators with equivalent operands.
977 Finder->addMatcher(
978 traverse(TK_AsIs,
979 cxxOperatorCallExpr(
980 hasAnyOverloadedOperatorName("-", "/", "%", "|", "&", "^",
981 "==", "!=", "<", "<=", ">",
982 ">=", "&&", "||", "="),
983 parametersAreEquivalent(),
984 // Filter noisy false positives.
985 unless(isMacro()), unless(isInTemplateInstantiation()),
986 unless(IsInUnevaluatedContext))
987 .bind("call")),
988 this);
989
990 // Overloaded operators with equivalent operands.
991 Finder->addMatcher(
992 cxxOperatorCallExpr(
993 hasAnyOverloadedOperatorName("|", "&", "||", "&&", "^"),
994 nestedParametersAreEquivalent(), argumentCountIs(2),
995 // Filter noisy false positives.
996 unless(isMacro()), unless(isInTemplateInstantiation()),
997 unless(IsInUnevaluatedContext))
998 .bind("nested-duplicates"),
999 this);
1000
1001 // Match expressions like: !(1 | 2 | 3)
1002 Finder->addMatcher(
1003 traverse(TK_AsIs,
1004 implicitCastExpr(
1005 hasImplicitDestinationType(isInteger()),
1006 has(unaryOperator(
1007 hasOperatorName("!"),
1008 hasUnaryOperand(ignoringParenImpCasts(binaryOperator(
1009 hasAnyOperatorName("|", "&"),
1010 hasLHS(anyOf(
1011 binaryOperator(hasAnyOperatorName("|", "&")),
1012 integerLiteral())),
1013 hasRHS(integerLiteral())))))
1014 .bind("logical-bitwise-confusion")),
1015 unless(IsInUnevaluatedContext))),
1016 this);
1017
1018 // Match expressions like: (X << 8) & 0xFF
1019 Finder->addMatcher(
1020 traverse(TK_AsIs,
1021 binaryOperator(
1022 hasOperatorName("&"),
1023 hasOperands(ignoringParenImpCasts(binaryOperator(
1024 hasOperatorName("<<"),
1025 hasRHS(ignoringParenImpCasts(
1026 integerLiteral().bind("shift-const"))))),
1027 ignoringParenImpCasts(
1028 integerLiteral().bind("and-const"))),
1029 unless(IsInUnevaluatedContext))
1030 .bind("left-right-shift-confusion")),
1031 this);
1032
1033 // Match common expressions and apply more checks to find redundant
1034 // sub-expressions.
1035 // a) Expr <op> K1 == K2
1036 // b) Expr <op> K1 == Expr
1037 // c) Expr <op> K1 == Expr <op> K2
1038 // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
1039 const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
1040 const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
1041 const auto CstRight = matchIntegerConstantExpr("rhs");
1042 const auto SymRight = matchSymbolicExpr("rhs");
1043
1044 // Match expressions like: x <op> 0xFF == 0xF00.
1045 Finder->addMatcher(
1046 traverse(TK_AsIs, binaryOperator(isComparisonOperator(),
1047 hasOperands(BinOpCstLeft, CstRight),
1048 unless(IsInUnevaluatedContext))
1049 .bind("binop-const-compare-to-const")),
1050 this);
1051
1052 // Match expressions like: x <op> 0xFF == x.
1053 Finder->addMatcher(
1054 traverse(
1055 TK_AsIs,
1056 binaryOperator(isComparisonOperator(),
1057 anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
1058 allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))),
1059 unless(IsInUnevaluatedContext))
1060 .bind("binop-const-compare-to-sym")),
1061 this);
1062
1063 // Match expressions like: x <op> 10 == x <op> 12.
1064 Finder->addMatcher(
1065 traverse(TK_AsIs,
1066 binaryOperator(isComparisonOperator(), hasLHS(BinOpCstLeft),
1067 hasRHS(BinOpCstRight),
1068 // Already reported as redundant.
1069 unless(operandsAreEquivalent()),
1070 unless(IsInUnevaluatedContext))
1071 .bind("binop-const-compare-to-binop-const")),
1072 this);
1073
1074 // Match relational expressions combined with logical operators and find
1075 // redundant sub-expressions.
1076 // see: 'checkRelationalExpr'
1077
1078 // Match expressions like: x < 2 && x > 2.
1079 const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
1080 const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
1081 Finder->addMatcher(
1082 traverse(TK_AsIs,
1083 binaryOperator(hasAnyOperatorName("||", "&&"),
1084 hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
1085 // Already reported as redundant.
1086 unless(operandsAreEquivalent()),
1087 unless(IsInUnevaluatedContext))
1088 .bind("comparisons-of-symbol-and-const")),
1089 this);
1090}
1091
1092void RedundantExpressionCheck::checkArithmeticExpr(
1093 const MatchFinder::MatchResult &Result) {
1094 APSInt LhsValue, RhsValue;
1095 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
1096 BinaryOperatorKind LhsOpcode{}, RhsOpcode{};
1097
1098 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
1099 "binop-const-compare-to-sym")) {
1100 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1101 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
1102 LhsValue) ||
1103 !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
1104 !areEquivalentExpr(LhsSymbol, RhsSymbol))
1105 return;
1106
1107 // Check expressions: x + k == x or x - k == x.
1108 if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
1109 if ((LhsValue != 0 && Opcode == BO_EQ) ||
1110 (LhsValue == 0 && Opcode == BO_NE))
1111 diag(ComparisonOperator->getOperatorLoc(),
1112 "logical expression is always false");
1113 else if ((LhsValue == 0 && Opcode == BO_EQ) ||
1114 (LhsValue != 0 && Opcode == BO_NE))
1115 diag(ComparisonOperator->getOperatorLoc(),
1116 "logical expression is always true");
1117 }
1118 } else if (const auto *ComparisonOperator =
1119 Result.Nodes.getNodeAs<BinaryOperator>(
1120 "binop-const-compare-to-binop-const")) {
1121 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1122
1123 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
1124 LhsValue) ||
1125 !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
1126 RhsValue) ||
1127 !areEquivalentExpr(LhsSymbol, RhsSymbol))
1128 return;
1129
1130 transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
1131 transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
1132
1133 // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
1134 if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
1135 if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
1136 (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
1137 diag(ComparisonOperator->getOperatorLoc(),
1138 "logical expression is always true");
1139 } else if ((Opcode == BO_EQ &&
1140 APSInt::compareValues(LhsValue, RhsValue) != 0) ||
1141 (Opcode == BO_NE &&
1142 APSInt::compareValues(LhsValue, RhsValue) == 0)) {
1143 diag(ComparisonOperator->getOperatorLoc(),
1144 "logical expression is always false");
1145 }
1146 }
1147 }
1148}
1149
1150static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value) {
1151 return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
1152}
1153
1154static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
1155 APSInt Value) {
1156 return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
1157}
1158
1159static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value) {
1160 return ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) ||
1161 ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
1162}
1163
1164void RedundantExpressionCheck::checkBitwiseExpr(
1165 const MatchFinder::MatchResult &Result) {
1166 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
1167 "binop-const-compare-to-const")) {
1168 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1169
1170 APSInt LhsValue, RhsValue;
1171 const Expr *LhsSymbol = nullptr;
1172 BinaryOperatorKind LhsOpcode{};
1173 if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
1174 LhsValue) ||
1175 !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
1176 return;
1177
1178 uint64_t LhsConstant = LhsValue.getZExtValue();
1179 uint64_t RhsConstant = RhsValue.getZExtValue();
1180 SourceLocation Loc = ComparisonOperator->getOperatorLoc();
1181
1182 // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
1183 if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
1184 if (Opcode == BO_EQ)
1185 diag(Loc, "logical expression is always false");
1186 else if (Opcode == BO_NE)
1187 diag(Loc, "logical expression is always true");
1188 }
1189
1190 // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
1191 if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
1192 if (Opcode == BO_EQ)
1193 diag(Loc, "logical expression is always false");
1194 else if (Opcode == BO_NE)
1195 diag(Loc, "logical expression is always true");
1196 }
1197 } else if (const auto *IneffectiveOperator =
1198 Result.Nodes.getNodeAs<BinaryOperator>(
1199 "ineffective-bitwise")) {
1200 APSInt Value;
1201 const Expr *Sym = nullptr, *ConstExpr = nullptr;
1202
1203 if (!retrieveSymbolicExpr(Result, "ineffective-bitwise", Sym) ||
1204 !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value,
1205 ConstExpr))
1206 return;
1207
1208 if ((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
1209 return;
1210
1211 SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
1212
1213 BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode();
1214 if (exprEvaluatesToZero(Opcode, Value)) {
1215 diag(Loc, "expression always evaluates to 0");
1216 } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) {
1217 SourceRange ConstExprRange(ConstExpr->getBeginLoc(),
1218 ConstExpr->getEndLoc());
1219 StringRef ConstExprText = Lexer::getSourceText(
1220 CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager,
1221 Result.Context->getLangOpts());
1222
1223 diag(Loc, "expression always evaluates to '%0'") << ConstExprText;
1224
1225 } else if (exprEvaluatesToSymbolic(Opcode, Value)) {
1226 SourceRange SymExprRange(Sym->getBeginLoc(), Sym->getEndLoc());
1227
1228 StringRef ExprText = Lexer::getSourceText(
1229 CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager,
1230 Result.Context->getLangOpts());
1231
1232 diag(Loc, "expression always evaluates to '%0'") << ExprText;
1233 }
1234 }
1235}
1236
1237void RedundantExpressionCheck::checkRelationalExpr(
1238 const MatchFinder::MatchResult &Result) {
1239 if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
1240 "comparisons-of-symbol-and-const")) {
1241 // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
1242 // E.g.: (X < 2) && (X > 4)
1243 BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
1244
1245 const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
1246 const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
1247 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
1248 BinaryOperatorKind LhsOpcode{}, RhsOpcode{};
1249 APSInt LhsValue, RhsValue;
1250
1252 Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
1254 Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
1255 !areEquivalentExpr(LhsSymbol, RhsSymbol))
1256 return;
1257
1258 // Bring expr to a canonical form: smallest constant must be on the left.
1259 if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
1260 std::swap(LhsExpr, RhsExpr);
1261 std::swap(LhsValue, RhsValue);
1262 std::swap(LhsSymbol, RhsSymbol);
1263 std::swap(LhsOpcode, RhsOpcode);
1264 }
1265
1266 // Constants come from two different macros, or one of them is a macro.
1267 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
1268 areExprsMacroAndNonMacro(LhsConst, RhsConst))
1269 return;
1270
1271 if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
1272 areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1273 diag(ComparisonOperator->getOperatorLoc(),
1274 "equivalent expression on both sides of logical operator");
1275 return;
1276 }
1277
1278 if (Opcode == BO_LAnd) {
1279 if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1280 diag(ComparisonOperator->getOperatorLoc(),
1281 "logical expression is always false");
1282 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1283 diag(LhsExpr->getExprLoc(), "expression is redundant");
1284 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
1285 diag(RhsExpr->getExprLoc(), "expression is redundant");
1286 }
1287 }
1288
1289 if (Opcode == BO_LOr) {
1290 if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1291 diag(ComparisonOperator->getOperatorLoc(),
1292 "logical expression is always true");
1293 } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
1294 diag(RhsExpr->getExprLoc(), "expression is redundant");
1295 } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
1296 diag(LhsExpr->getExprLoc(), "expression is redundant");
1297 }
1298 }
1299 }
1300}
1301
1302void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
1303 if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
1304 // If the expression's constants are macros, check whether they are
1305 // intentional.
1306
1307 //
1308 // Special case for floating-point representation.
1309 //
1310 // If expressions on both sides of comparison operator are of type float,
1311 // then for some comparison operators no warning shall be
1312 // reported even if the expressions are identical from a symbolic point of
1313 // view. Comparison between expressions, declared variables and literals
1314 // are treated differently.
1315 //
1316 // != and == between float literals that have the same value should NOT
1317 // warn. < > between float literals that have the same value SHOULD warn.
1318 //
1319 // != and == between the same float declaration should NOT warn.
1320 // < > between the same float declaration SHOULD warn.
1321 //
1322 // != and == between eq. expressions that evaluates into float
1323 // should NOT warn.
1324 // < > between eq. expressions that evaluates into float
1325 // should NOT warn.
1326 //
1327 const Expr *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
1328 const Expr *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
1329 const BinaryOperator::Opcode Op = BinOp->getOpcode();
1330 const bool OpEqualEQorNE = ((Op == BO_EQ) || (Op == BO_NE));
1331
1332 const auto *DeclRef1 = dyn_cast<DeclRefExpr>(LHS);
1333 const auto *DeclRef2 = dyn_cast<DeclRefExpr>(RHS);
1334 const auto *FloatLit1 = dyn_cast<FloatingLiteral>(LHS);
1335 const auto *FloatLit2 = dyn_cast<FloatingLiteral>(RHS);
1336
1337 if (DeclRef1 && DeclRef2 &&
1338 DeclRef1->getType()->hasFloatingRepresentation() &&
1339 DeclRef2->getType()->hasFloatingRepresentation() &&
1340 (DeclRef1->getDecl() == DeclRef2->getDecl()) && OpEqualEQorNE) {
1341 return;
1342 }
1343
1344 if (FloatLit1 && FloatLit2 &&
1345 FloatLit1->getValue().bitwiseIsEqual(FloatLit2->getValue()) &&
1346 OpEqualEQorNE) {
1347 return;
1348 }
1349
1351 BinOp, Result.Context)) {
1352 const Expr *LhsConst = nullptr, *RhsConst = nullptr;
1353 BinaryOperatorKind MainOpcode{}, SideOpcode{};
1354 if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
1355 if (!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode,
1356 LhsConst, RhsConst, Result.Context))
1357 return;
1358
1359 if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
1360 areExprsMacroAndNonMacro(LhsConst, RhsConst))
1361 return;
1362 } else {
1363 if (!areExprsSameMacroOrLiteral(BinOp, Result.Context))
1364 return;
1365 }
1366 }
1367 diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
1368 }
1369
1370 if (const auto *CondOp =
1371 Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
1372 const Expr *TrueExpr = CondOp->getTrueExpr();
1373 const Expr *FalseExpr = CondOp->getFalseExpr();
1374
1375 if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
1376 areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
1377 return;
1378 diag(CondOp->getColonLoc(),
1379 "'true' and 'false' expressions are equivalent");
1380 }
1381
1382 if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
1384 return;
1385
1386 diag(Call->getOperatorLoc(),
1387 "both sides of overloaded operator are equivalent");
1388 }
1389
1390 if (const auto *Op = Result.Nodes.getNodeAs<Expr>("nested-duplicates")) {
1391 const auto *Call = dyn_cast<CXXOperatorCallExpr>(Op);
1392 if (Call && canOverloadedOperatorArgsBeModified(Call, true))
1393 return;
1394
1395 StringRef Message =
1396 Call ? "overloaded operator has equivalent nested operands"
1397 : "operator has equivalent nested operands";
1398
1399 const auto Diag = diag(Op->getExprLoc(), Message);
1400 for (const auto &KeyValue : Result.Nodes.getMap()) {
1401 if (StringRef(KeyValue.first).starts_with("duplicate"))
1402 Diag << KeyValue.second.getSourceRange();
1403 }
1404 }
1405
1406 if (const auto *NegateOperator =
1407 Result.Nodes.getNodeAs<UnaryOperator>("logical-bitwise-confusion")) {
1408 SourceLocation OperatorLoc = NegateOperator->getOperatorLoc();
1409
1410 auto Diag =
1411 diag(OperatorLoc,
1412 "ineffective logical negation operator used; did you mean '~'?");
1413 SourceLocation LogicalNotLocation = OperatorLoc.getLocWithOffset(1);
1414
1415 if (!LogicalNotLocation.isMacroID())
1416 Diag << FixItHint::CreateReplacement(
1417 CharSourceRange::getCharRange(OperatorLoc, LogicalNotLocation), "~");
1418 }
1419
1420 if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs<BinaryOperator>(
1421 "left-right-shift-confusion")) {
1422 const auto *ShiftingConst = Result.Nodes.getNodeAs<Expr>("shift-const");
1423 assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!");
1424 std::optional<llvm::APSInt> ShiftingValue =
1425 ShiftingConst->getIntegerConstantExpr(*Result.Context);
1426
1427 if (!ShiftingValue)
1428 return;
1429
1430 const auto *AndConst = Result.Nodes.getNodeAs<Expr>("and-const");
1431 assert(AndConst && "Expr* 'AndCont' is nullptr!");
1432 std::optional<llvm::APSInt> AndValue =
1433 AndConst->getIntegerConstantExpr(*Result.Context);
1434 if (!AndValue)
1435 return;
1436
1437 // If ShiftingConst is shifted left with more bits than the position of the
1438 // leftmost 1 in the bit representation of AndValue, AndConstant is
1439 // ineffective.
1440 if (AndValue->getActiveBits() > *ShiftingValue)
1441 return;
1442
1443 auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
1444 "ineffective bitwise and operation");
1445 }
1446
1447 // Check for the following bound expressions:
1448 // - "binop-const-compare-to-sym",
1449 // - "binop-const-compare-to-binop-const",
1450 // Produced message:
1451 // -> "logical expression is always false/true"
1452 checkArithmeticExpr(Result);
1453
1454 // Check for the following bound expression:
1455 // - "binop-const-compare-to-const",
1456 // - "ineffective-bitwise"
1457 // Produced message:
1458 // -> "logical expression is always false/true"
1459 // -> "expression always evaluates to ..."
1460 checkBitwiseExpr(Result);
1461
1462 // Check for te following bound expression:
1463 // - "comparisons-of-symbol-and-const",
1464 // Produced messages:
1465 // -> "equivalent expression on both sides of logical operator",
1466 // -> "logical expression is always false/true"
1467 // -> "expression is redundant"
1468 checkRelationalExpr(Result);
1469}
1470
1471} // 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 llvm::StringLiteral 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 bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value)
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 bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode, APSInt Value)
static OverloadedOperatorKind getOp(const BinaryOperator *Op)
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 exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value)
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 canOverloadedOperatorArgsBeModified(const CXXOperatorCallExpr *OperatorCall, bool CheckSecondParam)