summaryrefslogtreecommitdiff
path: root/compilerplugins
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2017-11-07 23:27:54 +0100
committerStephan Bergmann <sbergman@redhat.com>2017-11-08 11:59:09 +0100
commitadf3cf074e05e633e907f72215d56720c01fd2db (patch)
tree05dbfa1d5790514a75e8671638083dc810dcd30c /compilerplugins
parent56fe49c013cc5573c239fee37c6201a3ca1e7749 (diff)
Fix loplugin:flatten's skipping of if/then/else/if chains
(but which finds no new hits) Change-Id: I5d5f351402797b662a08ec8dca301bd174e22a50 Reviewed-on: https://gerrit.libreoffice.org/44433 Tested-by: Jenkins <ci@libreoffice.org> Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'compilerplugins')
-rw-r--r--compilerplugins/clang/flatten.cxx23
-rw-r--r--compilerplugins/clang/test/flatten.cxx27
2 files changed, 45 insertions, 5 deletions
diff --git a/compilerplugins/clang/flatten.cxx b/compilerplugins/clang/flatten.cxx
index 7d546ef5b448..7dc2bf019738 100644
--- a/compilerplugins/clang/flatten.cxx
+++ b/compilerplugins/clang/flatten.cxx
@@ -47,6 +47,7 @@ private:
std::stack<bool> mLastStmtInParentStack;
std::vector<std::pair<char const *, char const *>> mvModifiedRanges;
+ Stmt const * mElseBranch = nullptr;
};
static Stmt const * containsSingleThrowExpr(Stmt const * stmt)
@@ -84,10 +85,20 @@ bool Flatten::TraverseCXXCatchStmt(CXXCatchStmt* )
bool Flatten::TraverseIfStmt(IfStmt * ifStmt)
{
- // ignore if we are part of an if/then/else/if chain
- if (ifStmt->getElse() && isa<IfStmt>(ifStmt->getElse()))
- return true;
- return RecursiveASTVisitor<Flatten>::TraverseIfStmt(ifStmt);
+ if (!WalkUpFromIfStmt(ifStmt)) {
+ return false;
+ }
+ auto const saved = mElseBranch;
+ mElseBranch = ifStmt->getElse();
+ auto ret = true;
+ for (auto const sub: ifStmt->children()) {
+ if (!TraverseStmt(sub)) {
+ ret = false;
+ break;
+ }
+ }
+ mElseBranch = saved;
+ return ret;
}
bool Flatten::TraverseCompoundStmt(CompoundStmt * compoundStmt)
@@ -110,6 +121,10 @@ bool Flatten::VisitIfStmt(IfStmt const * ifStmt)
if (!ifStmt->getElse())
return true;
+ // ignore if we are part of an if/then/else/if chain
+ if (ifStmt == mElseBranch || isa<IfStmt>(ifStmt->getElse()))
+ return true;
+
auto const thenThrowExpr = containsSingleThrowExpr(ifStmt->getThen());
auto const elseThrowExpr = containsSingleThrowExpr(ifStmt->getElse());
// If neither contains a throw, nothing to do; if both contain throws, no
diff --git a/compilerplugins/clang/test/flatten.cxx b/compilerplugins/clang/test/flatten.cxx
index dcc7cb3b81c9..e7b53519de97 100644
--- a/compilerplugins/clang/test/flatten.cxx
+++ b/compilerplugins/clang/test/flatten.cxx
@@ -10,7 +10,7 @@
#include <exception>
extern int foo();
-extern int bar();
+extern int bar(int = 0);
class Class {};
void top1() {
@@ -104,4 +104,29 @@ void top6() {
(void)x;
}
+void top7() {
+ // no warning expected
+ if (foo() == 1) {
+ throw std::exception();
+ } else if (foo() == 2) {
+ throw std::exception();
+ } else {
+ throw std::exception();
+ }
+}
+
+void top8() {
+ if (foo() == 1) {
+ if (foo() == 2) {
+ throw std::exception(); // expected-error {{unconditional throw in then branch, just flatten the else [loplugin:flatten]}}
+ } else {
+ bar();
+ }
+ } else if (foo() == 2) {
+ bar(1);
+ } else {
+ bar(2);
+ }
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */