namespace {
class FindIdenticalExprVisitor
: public RecursiveASTVisitor<FindIdenticalExprVisitor> {
+ BugReporter &BR;
+ const CheckerBase *Checker;
+ AnalysisDeclContext *AC;
public:
explicit FindIdenticalExprVisitor(BugReporter &B,
const CheckerBase *Checker,
bool VisitConditionalOperator(const ConditionalOperator *C);
private:
- BugReporter &BR;
- const CheckerBase *Checker;
- AnalysisDeclContext *AC;
+ void reportIdenticalExpr(const BinaryOperator *B, bool CheckBitwise,
+ ArrayRef<SourceRange> Sr);
+ void checkBitwiseOrLogicalOp(const BinaryOperator *B, bool CheckBitwise);
+ void checkComparisonOp(const BinaryOperator *B);
};
} // end anonymous namespace
+void FindIdenticalExprVisitor::reportIdenticalExpr(const BinaryOperator *B,
+ bool CheckBitwise,
+ ArrayRef<SourceRange> Sr) {
+ StringRef Message;
+ if (CheckBitwise)
+ Message = "identical expressions on both sides of bitwise operator";
+ else
+ Message = "identical expressions on both sides of logical operator";
+
+ PathDiagnosticLocation ELoc =
+ PathDiagnosticLocation::createOperatorLoc(B, BR.getSourceManager());
+ BR.EmitBasicReport(AC->getDecl(), Checker,
+ "Use of identical expressions",
+ categories::LogicError,
+ Message, ELoc, Sr);
+}
+
+void FindIdenticalExprVisitor::checkBitwiseOrLogicalOp(const BinaryOperator *B,
+ bool CheckBitwise) {
+ SourceRange Sr[2];
+
+ const Expr *LHS = B->getLHS();
+ const Expr *RHS = B->getRHS();
+
+ // Split operators as long as we still have operators to split on. We will
+ // get called for every binary operator in an expression so there is no need
+ // to check every one against each other here, just the right most one with
+ // the others.
+ while (const BinaryOperator *B2 = dyn_cast<BinaryOperator>(LHS)) {
+ if (B->getOpcode() != B2->getOpcode())
+ break;
+ if (isIdenticalStmt(AC->getASTContext(), RHS, B2->getRHS())) {
+ Sr[0] = RHS->getSourceRange();
+ Sr[1] = B2->getRHS()->getSourceRange();
+ reportIdenticalExpr(B, CheckBitwise, Sr);
+ }
+ LHS = B2->getLHS();
+ }
+
+ if (isIdenticalStmt(AC->getASTContext(), RHS, LHS)) {
+ Sr[0] = RHS->getSourceRange();
+ Sr[1] = LHS->getSourceRange();
+ reportIdenticalExpr(B, CheckBitwise, Sr);
+ }
+}
+
bool FindIdenticalExprVisitor::VisitIfStmt(const IfStmt *I) {
const Stmt *Stmt1 = I->getThen();
const Stmt *Stmt2 = I->getElse();
bool FindIdenticalExprVisitor::VisitBinaryOperator(const BinaryOperator *B) {
BinaryOperator::Opcode Op = B->getOpcode();
- if (!BinaryOperator::isComparisonOp(Op))
- return true;
+
+ if (BinaryOperator::isBitwiseOp(Op))
+ checkBitwiseOrLogicalOp(B, true);
+
+ if (BinaryOperator::isLogicalOp(Op))
+ checkBitwiseOrLogicalOp(B, false);
+
+ if (BinaryOperator::isComparisonOp(Op))
+ checkComparisonOp(B);
+
+ // We want to visit ALL nodes (subexpressions of binary comparison
+ // expressions too) that contains comparison operators.
+ // True is always returned to traverse ALL nodes.
+ return true;
+}
+
+void FindIdenticalExprVisitor::checkComparisonOp(const BinaryOperator *B) {
+ BinaryOperator::Opcode Op = B->getOpcode();
+
//
// Special case for floating-point representation.
//
(DeclRef2->getType()->hasFloatingRepresentation())) {
if (DeclRef1->getDecl() == DeclRef2->getDecl()) {
if ((Op == BO_EQ) || (Op == BO_NE)) {
- return true;
+ return;
}
}
}
} else if ((FloatLit1) && (FloatLit2)) {
if (FloatLit1->getValue().bitwiseIsEqual(FloatLit2->getValue())) {
if ((Op == BO_EQ) || (Op == BO_NE)) {
- return true;
+ return;
}
}
} else if (LHS->getType()->hasFloatingRepresentation()) {
// If any side of comparison operator still has floating-point
// representation, then it's an expression. Don't warn.
// Here only LHS is checked since RHS will be implicit casted to float.
- return true;
+ return;
} else {
// No special case with floating-point representation, report as usual.
}
"Compare of identical expressions",
categories::LogicError, Message, ELoc);
}
- // We want to visit ALL nodes (subexpressions of binary comparison
- // expressions too) that contains comparison operators.
- // True is always returned to traverse ALL nodes.
- return true;
}
bool FindIdenticalExprVisitor::VisitConditionalOperator(
i += 10;
}
}
+
+void test_identical_bitwise1() {
+ int a = 5 | 5; // expected-warning {{identical expressions on both sides of bitwise operator}}
+}
+
+void test_identical_bitwise2() {
+ int a = 5;
+ int b = a | a; // expected-warning {{identical expressions on both sides of bitwise operator}}
+}
+
+void test_identical_bitwise3() {
+ int a = 5;
+ int b = (a | a); // expected-warning {{identical expressions on both sides of bitwise operator}}
+}
+
+void test_identical_bitwise4() {
+ int a = 4;
+ int b = a | 4; // no-warning
+}
+
+void test_identical_bitwise5() {
+ int a = 4;
+ int b = 4;
+ int c = a | b; // no-warning
+}
+
+void test_identical_bitwise6() {
+ int a = 5;
+ int b = a | 4 | a; // expected-warning {{identical expressions on both sides of bitwise operator}}
+}
+
+void test_identical_bitwise7() {
+ int a = 5;
+ int b = func() | func(); // no-warning
+}
+
+void test_identical_logical1(int a) {
+ if (a == 4 && a == 4) // expected-warning {{identical expressions on both sides of logical operator}}
+ ;
+}
+
+void test_identical_logical2(int a) {
+ if (a == 4 || a == 5 || a == 4) // expected-warning {{identical expressions on both sides of logical operator}}
+ ;
+}
+
+void test_identical_logical3(int a) {
+ if (a == 4 || a == 5 || a == 6) // no-warning
+ ;
+}
+
+void test_identical_logical4(int a) {
+ if (a == func() || a == func()) // no-warning
+ ;
+}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wlogical-op-parentheses"
+void test_identical_logical5(int x, int y) {
+ if (x == 4 && y == 5 || x == 4 && y == 6) // no-warning
+ ;
+}
+
+void test_identical_logical6(int x, int y) {
+ if (x == 4 && y == 5 || x == 4 && y == 5) // expected-warning {{identical expressions on both sides of logical operator}}
+ ;
+}
+
+void test_identical_logical7(int x, int y) {
+ // FIXME: We should warn here
+ if (x == 4 && y == 5 || x == 4)
+ ;
+}
+
+void test_identical_logical8(int x, int y) {
+ // FIXME: We should warn here
+ if (x == 4 || y == 5 && x == 4)
+ ;
+}
+
+void test_identical_logical9(int x, int y) {
+ // FIXME: We should warn here
+ if (x == 4 || x == 4 && y == 5)
+ ;
+}
+#pragma clang diagnostic pop