From a6839aaa9bd7c75665263a558796d9d10ec89442 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 11 Mar 2014 16:52:29 +0000 Subject: [PATCH] [analyzer] Check all conditions in a chained if against each other. Like the binary operator check of r201702, this actually checks the condition of every if in a chain against every other condition, an O(N^2) operation. In most cases N should be small enough to make this practical, and checking all cases like this makes it much more likely to catch a copy-paste error within the same series of branches. Part of IdenticalExprChecker; patch by Daniel Fahlgren! llvm-svn: 203585 --- .../Checkers/IdenticalExprChecker.cpp | 24 ++++ clang/test/Analysis/identical-expressions.cpp | 105 ++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp index 6abfc14877e1..d5c52b4c6a31 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp @@ -108,6 +108,30 @@ bool FindIdenticalExprVisitor::VisitIfStmt(const IfStmt *I) { const Stmt *Stmt1 = I->getThen(); const Stmt *Stmt2 = I->getElse(); + // Check for identical conditions: + // + // if (b) { + // foo1(); + // } else if (b) { + // foo2(); + // } + if (Stmt1 && Stmt2) { + const Expr *Cond1 = I->getCond(); + const Stmt *Else = Stmt2; + while (const IfStmt *I2 = dyn_cast_or_null(Else)) { + const Expr *Cond2 = I2->getCond(); + if (isIdenticalStmt(AC->getASTContext(), Cond1, Cond2, false)) { + SourceRange Sr = Cond1->getSourceRange(); + PathDiagnosticLocation ELoc(Cond2, BR.getSourceManager(), AC); + BR.EmitBasicReport(AC->getDecl(), Checker, "Identical conditions", + categories::LogicError, + "expression is identical to previous condition", + ELoc, Sr); + } + Else = I2->getElse(); + } + } + if (!Stmt1 || !Stmt2) return true; diff --git a/clang/test/Analysis/identical-expressions.cpp b/clang/test/Analysis/identical-expressions.cpp index e1f51af7c6f1..85e3322002dd 100644 --- a/clang/test/Analysis/identical-expressions.cpp +++ b/clang/test/Analysis/identical-expressions.cpp @@ -1406,3 +1406,108 @@ void test_identical_logical9(int x, int y) { ; } #pragma clang diagnostic pop + +void test_warn_chained_if_stmts_1(int x) { + if (x == 1) + ; + else if (x == 1) // expected-warning {{expression is identical to previous condition}} + ; +} + +void test_warn_chained_if_stmts_2(int x) { + if (x == 1) + ; + else if (x == 1) // expected-warning {{expression is identical to previous condition}} + ; + else if (x == 1) // expected-warning {{expression is identical to previous condition}} + ; +} + +void test_warn_chained_if_stmts_3(int x) { + if (x == 1) + ; + else if (x == 2) + ; + else if (x == 1) // expected-warning {{expression is identical to previous condition}} + ; +} + +void test_warn_chained_if_stmts_4(int x) { + if (x == 1) + ; + else if (func()) + ; + else if (x == 1) // expected-warning {{expression is identical to previous condition}} + ; +} + +void test_warn_chained_if_stmts_5(int x) { + if (x & 1) + ; + else if (x & 1) // expected-warning {{expression is identical to previous condition}} + ; +} + +void test_warn_chained_if_stmts_6(int x) { + if (x == 1) + ; + else if (x == 2) + ; + else if (x == 2) // expected-warning {{expression is identical to previous condition}} + ; + else if (x == 3) + ; +} + +void test_warn_chained_if_stmts_7(int x) { + if (x == 1) + ; + else if (x == 2) + ; + else if (x == 3) + ; + else if (x == 2) // expected-warning {{expression is identical to previous condition}} + ; + else if (x == 5) + ; +} + +void test_warn_chained_if_stmts_8(int x) { + if (x == 1) + ; + else if (x == 2) + ; + else if (x == 3) + ; + else if (x == 2) // expected-warning {{expression is identical to previous condition}} + ; + else if (x == 5) + ; + else if (x == 3) // expected-warning {{expression is identical to previous condition}} + ; + else if (x == 7) + ; +} + +void test_nowarn_chained_if_stmts_1(int x) { + if (func()) + ; + else if (func()) // no-warning + ; +} + +void test_nowarn_chained_if_stmts_2(int x) { + if (func()) + ; + else if (x == 1) + ; + else if (func()) // no-warning + ; +} + +void test_nowarn_chained_if_stmts_3(int x) { + if (x++) + ; + else if (x++) // no-warning + ; +}