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>
This commit is contained in:
@@ -47,6 +47,7 @@ private:
|
|||||||
|
|
||||||
std::stack<bool> mLastStmtInParentStack;
|
std::stack<bool> mLastStmtInParentStack;
|
||||||
std::vector<std::pair<char const *, char const *>> mvModifiedRanges;
|
std::vector<std::pair<char const *, char const *>> mvModifiedRanges;
|
||||||
|
Stmt const * mElseBranch = nullptr;
|
||||||
};
|
};
|
||||||
|
|
||||||
static Stmt const * containsSingleThrowExpr(Stmt const * stmt)
|
static Stmt const * containsSingleThrowExpr(Stmt const * stmt)
|
||||||
@@ -84,10 +85,20 @@ bool Flatten::TraverseCXXCatchStmt(CXXCatchStmt* )
|
|||||||
|
|
||||||
bool Flatten::TraverseIfStmt(IfStmt * ifStmt)
|
bool Flatten::TraverseIfStmt(IfStmt * ifStmt)
|
||||||
{
|
{
|
||||||
// ignore if we are part of an if/then/else/if chain
|
if (!WalkUpFromIfStmt(ifStmt)) {
|
||||||
if (ifStmt->getElse() && isa<IfStmt>(ifStmt->getElse()))
|
return false;
|
||||||
return true;
|
}
|
||||||
return RecursiveASTVisitor<Flatten>::TraverseIfStmt(ifStmt);
|
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)
|
bool Flatten::TraverseCompoundStmt(CompoundStmt * compoundStmt)
|
||||||
@@ -110,6 +121,10 @@ bool Flatten::VisitIfStmt(IfStmt const * ifStmt)
|
|||||||
if (!ifStmt->getElse())
|
if (!ifStmt->getElse())
|
||||||
return true;
|
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 thenThrowExpr = containsSingleThrowExpr(ifStmt->getThen());
|
||||||
auto const elseThrowExpr = containsSingleThrowExpr(ifStmt->getElse());
|
auto const elseThrowExpr = containsSingleThrowExpr(ifStmt->getElse());
|
||||||
// If neither contains a throw, nothing to do; if both contain throws, no
|
// If neither contains a throw, nothing to do; if both contain throws, no
|
||||||
|
@@ -10,7 +10,7 @@
|
|||||||
#include <exception>
|
#include <exception>
|
||||||
|
|
||||||
extern int foo();
|
extern int foo();
|
||||||
extern int bar();
|
extern int bar(int = 0);
|
||||||
class Class {};
|
class Class {};
|
||||||
|
|
||||||
void top1() {
|
void top1() {
|
||||||
@@ -104,4 +104,29 @@ void top6() {
|
|||||||
(void)x;
|
(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: */
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
|
||||||
|
Reference in New Issue
Block a user