Index: clang-tidy/misc/RedundantExpressionCheck.cpp =================================================================== --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -133,6 +133,24 @@ } } +// Returns the first subexpr of an expr that is not a cast or a copy +// constructor's materialize expr. +// Used by matcher `overloadedOperatorsArgumentsAreEquivalent` for stripping +// unnecessary layers from arguments before deciding whether they are +// equivalent. +const Expr *ignoreCastsAndCopyConstructorCalls(const Expr *CallExpression) { + + const Expr *NewExpr; + + if (auto *ConstructorExpr = dyn_cast(CallExpression)) { + if (ConstructorExpr->getNumArgs() == 1) + NewExpr = ConstructorExpr->getArg(0); + } + + NewExpr = NewExpr->IgnoreCasts(); + return NewExpr; +} + // For a given expression 'x', returns whether the ranges covered by the // relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5). static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS, @@ -484,7 +502,7 @@ // These functions must be declared const in order to not be able to modify // the instance of the class they are called through. if (ParamCount == 1 && - !OperatorDecl->getType()->getAs()->isConst()) + !cast(OperatorDecl->getType().getTypePtr())->isConst()) return true; if (isParamNonConstReferenceType(OperatorDecl->getParamDecl(0)->getType())) @@ -686,6 +704,18 @@ .bind("call"), this); + const auto IneffBitwiseConst = matchIntegerConstantExpr("ineff-bitwise"); + const auto IneffBitwiseSymExpr = matchSymbolicExpr("ineff-bitwise"); + + // Match ineffective bitwise operator expressions like: x & 0 or x |= ~0. + Finder->addMatcher( + binaryOperator(anyOf(hasOperatorName("|"), hasOperatorName("&"), + hasOperatorName("|="), hasOperatorName("&=")), + hasEitherOperand(IneffBitwiseConst), + hasEitherOperand(IneffBitwiseSymExpr)) + .bind("ineffective-bitwise"), + this); + // Match common expressions and apply more checks to find redundant // sub-expressions. // a) Expr K1 == K2 @@ -794,6 +824,23 @@ } } +static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value){ + return ((Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0); +} + +static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode, APSInt Value){ + return ((Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0); +} + +static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value){ + if ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) + return true; + if ((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0) + return true; + + return false; +} + void RedundantExpressionCheck::checkBitwiseExpr( const MatchFinder::MatchResult &Result) { if (const auto *ComparisonOperator = Result.Nodes.getNodeAs( @@ -827,6 +874,42 @@ else if (Opcode == BO_NE) diag(Loc, "logical expression is always true"); } + } else if (const auto *IneffectiveOperator = + Result.Nodes.getNodeAs( + "ineffective-bitwise")) { + APSInt Value; + const Expr *Sym = nullptr, *ConstExpr = nullptr; + BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode(); + + if (!retrieveSymbolicExpr(Result, "ineff-bitwise", Sym) || + !retrieveIntegerConstantExpr(Result, "ineff-bitwise", Value, ConstExpr)) + return; + + if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID()) + return; + + SourceLocation Loc = IneffectiveOperator->getOperatorLoc(); + + if (exprEvaluatesToZero(Opcode, Value)) { + diag(Loc, "expression always evaluates to 0"); + } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) { + SourceRange ConstExprRange(ConstExpr->getLocStart(), + ConstExpr->getLocEnd()); + std::string ConstExprText = Lexer::getSourceText( + CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager, + Result.Context->getLangOpts()); + + diag(Loc, + "expression always evaluates to '" + ConstExprText + "'"); + } else if (exprEvaluatesToSymbolic(Opcode, Value)) { + SourceRange SymExprRange(Sym->getLocStart(), Sym->getLocEnd()); + + std::string ExprText = Lexer::getSourceText( + CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager, + Result.Context->getLangOpts()); + + diag(Loc, "expression always evaluates to '" + ExprText + "'"); + } } } Index: docs/clang-tidy/checks/misc-redundant-expression.rst =================================================================== --- docs/clang-tidy/checks/misc-redundant-expression.rst +++ docs/clang-tidy/checks/misc-redundant-expression.rst @@ -13,7 +13,9 @@ - always ``false``, -- always a constant (zero or one). +- always a constant (zero or one), + +- ineffective, the operation does not change the expression. Examples: @@ -23,3 +25,4 @@ (p->x == p->x) // always true (p->x < p->x) // always false (speed - speed + 1 == 12) // speed - speed is always zero + x |& 0 // operation is ineffective Index: test/clang-tidy/misc-redundant-expression.cpp =================================================================== --- test/clang-tidy/misc-redundant-expression.cpp +++ test/clang-tidy/misc-redundant-expression.cpp @@ -268,6 +268,42 @@ } int TestBitwise(int X, int Y) { + if(0 & X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: expression always evaluates to 0 + if(~0 | X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression always evaluates to '~0' + + if(X & 0) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: expression always evaluates to 0 + if(X | ~0) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: expression always evaluates to '~0' + if(X | 0xffffffff) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: expression always evaluates to '0xffffffff' + + if(~0 & X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression always evaluates to 'X' + if(0 | X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: expression always evaluates to 'X' + + if(X & ~0) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: expression always evaluates to 'X' + if(X | 0) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: expression always evaluates to 'X' + + X &= ~0; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: expression always evaluates to 'X' + X |= 0; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: expression always evaluates to 'X' + + X &= 0; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: expression always evaluates to 0 + X |= ~0; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: expression always evaluates to '~0' + X |= 0xffffffff; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: expression always evaluates to '0xffffffff' + X |= -1; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: expression always evaluates to '-1' + if ((X & 0xFF) == 0xF00) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false if ((X & 0xFF) != 0xF00) return 1;