Index: clang-tidy/misc/RedundantExpressionCheck.cpp =================================================================== --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -25,6 +25,7 @@ #include #include #include +#include using namespace clang::ast_matchers; using namespace clang::tidy::matchers; @@ -198,7 +199,7 @@ } // Returns whether the ranges covered by the union of both relational -// expressions covers the whole domain (i.e. x < 10 and x > 0). +// expressions cover the whole domain (i.e. x < 10 and x > 0). static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS, const APSInt &ValueLHS, BinaryOperatorKind OpcodeRHS, @@ -571,7 +572,7 @@ } // Retrieves integer constant subexpressions from binary operator expressions -// that have two equivalent sides +// that have two equivalent sides. // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5. static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp, BinaryOperatorKind &MainOpcode, @@ -690,7 +691,7 @@ const auto IneffBitwiseConst = matchIntegerConstantExpr("ineff-bitwise"); const auto IneffBitwiseSymExpr = matchSymbolicExpr("ineff-bitwise"); - // Match ineffective bitwise operator expressions like: x & 0 or x |= ~0. + // Match ineffective or redundant bitwise operator expressions like: x & 0 or x |= ~0. Finder->addMatcher( binaryOperator(anyOf(hasOperatorName("|"), hasOperatorName("&"), hasOperatorName("|="), hasOperatorName("&=")), @@ -699,6 +700,33 @@ .bind("ineffective-bitwise"), this); + // Match expressions like: !(1 | 2 | 3) + Finder->addMatcher( + implicitCastExpr( + hasImplicitDestinationType(isInteger()), + has(unaryOperator( + hasOperatorName("!"), + hasUnaryOperand(ignoringParenImpCasts(binaryOperator( + anyOf(hasOperatorName("|"), hasOperatorName("&")), + hasLHS(anyOf(binaryOperator(anyOf(hasOperatorName("|"), + hasOperatorName("&"))), + integerLiteral())), + hasRHS(integerLiteral()))))) + .bind("logical-bitwise-confusion"))), + this); + + // Match expressions like: (X << 8) & 0xFF + Finder->addMatcher( + binaryOperator( + hasOperatorName("&"), + hasEitherOperand(ignoringParenImpCasts( + binaryOperator(hasOperatorName("<<"), + hasRHS(integerLiteral().bind("shift-const"))) + .bind("shift-operator"))), + hasEitherOperand(integerLiteral().bind("and-const"))) + .bind("left-right-shift-confusion"), + this); + // Match common expressions and apply more checks to find redundant // sub-expressions. // a) Expr K1 == K2 @@ -1014,6 +1042,47 @@ "both sides of overloaded operator are equivalent"); } + if(const auto *NegateOperator = Result.Nodes.getNodeAs("logical-bitwise-confusion")){ + SourceLocation OperatorLoc = NegateOperator->getOperatorLoc(); + + auto Diag = + diag(OperatorLoc, " logical negation operator might be confused with " + "bitwise negation operator"); + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(OperatorLoc, OperatorLoc.getLocWithOffset(1)), + "~"); + } + + if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs( + "left-right-shift-confusion")) { + const auto *ShiftOperator = + Result.Nodes.getNodeAs("shift-operator"); + const auto *ShiftingConst = Result.Nodes.getNodeAs("shift-const"); + const auto *AndConst = Result.Nodes.getNodeAs("and-const"); + + assert(ShiftOperator && "Expr* 'ShiftOperator' is nullptr!"); + assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!"); + assert(AndConst && "Expr* 'AndCont' is nullptr!"); + + APSInt ShiftingValue; + if (!ShiftingConst->isIntegerConstantExpr(ShiftingValue, *Result.Context)) + return; + + APSInt AndValue; + if (!AndConst->isIntegerConstantExpr(AndValue, *Result.Context)) + return; + + // If ShiftingConst is shifted left with more bits than the position of the + // leftmost 1 in the bit representation of AndValue, AndConstant is + // ineffective. + if (floor(log2(AndValue.getExtValue())) >= ShiftingValue) + return; + + auto Diag = diag(BinaryAndExpr->getOperatorLoc(), + " ineffective bitwise and operation. Did you intend " + "'>>' instead of '<<'?"); + } + // Check for the following bound expressions: // - "binop-const-compare-to-sym", // - "binop-const-compare-to-binop-const", @@ -1023,8 +1092,10 @@ // Check for the following bound expression: // - "binop-const-compare-to-const", + // - "ineffective-bitwise" // Produced message: // -> "logical expression is always false/true" + // -> "expression always evaluates to ..." checkBitwiseExpr(Result); // Check for te following bound expression: Index: test/clang-tidy/misc-redundant-expression.cpp =================================================================== --- test/clang-tidy/misc-redundant-expression.cpp +++ test/clang-tidy/misc-redundant-expression.cpp @@ -154,6 +154,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent if (S >= S) return true; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent + + return true; } #define LT(x, y) (void)((x) < (y)) @@ -696,3 +698,46 @@ return 0; } + +#define FLAG1 1 +#define FLAG2 2 +#define FLAG3 4 +#define FLAGS (FLAG1 | FLAG2 | FLAG3) +int operatorConfusion(int X, int Y) +{ + // Uneffective & expressions. + Y = (Y << 8) & 0xff; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: ineffective bitwise and operation. Did you intend '>>' instead of '<<'? + Y = (Y << 12) & 0xfff; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: ineffective bitwise and + Y = (Y << 12) & 0xff; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: ineffective bitwise and + Y = (Y << 8) & 0x77; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: ineffective bitwise and + Y = (Y << 5) & 0x11; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: ineffective bitwise and + + // Effective expressions. Do not check. + Y = (Y << 4) & 0x15; + Y = (Y << 3) & 0x250; + Y = (Y << 9) & 0xF33; + + int K = !(1 | 2 | 4); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: logical negation operator might be confused with bitwise negation operator + // CHECK-FIXES: {{^}} int K = ~(1 | 2 | 4);{{$}} + K = !(FLAG1 & FLAG2 & FLAG3); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: logical negation operator might be confused with bitwise negation operator + // CHECK-FIXES: {{^}} K = ~(FLAG1 & FLAG2 & FLAG3);{{$}} + K = !(3 | 4); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: logical negation operator might be confused with bitwise negation operator + // CHECK-FIXES: {{^}} K = ~(3 | 4);{{$}} + int NotFlags = !FLAGS; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical negation operator might be confused with bitwise negation operator + // CHECK-FIXES: {{^}} int NotFlags = ~FLAGS;{{$}} + return !(1 | 2 | 4); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: logical negation operator might be confused with bitwise negation operator + // CHECK-FIXES: {{^}} return ~(1 | 2 | 4);{{$}} +} +#undef FLAG1 +#undef FLAG2 +#undef FLAG3