walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Check more opposite conditions

This commit is contained in:
Paul 2018-03-24 07:58:37 +01:00 committed by Daniel Marjamäki
parent dbb7e98711
commit 43be20a824
6 changed files with 142 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
lib/astutils.cpp
View File

@ -238,6 +238,16 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
return commutativeEquals;
}
bool isEqualKnownValue(const Token * const tok1, const Token * const tok2)
{
return tok1->hasKnownValue() && tok2->hasKnownValue() && tok1->values() == tok2->values();
}
bool isDifferentKnownValues(const Token * const tok1, const Token * const tok2)
{
return tok1->hasKnownValue() && tok2->hasKnownValue() && tok1->values() != tok2->values();
}
bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure)
{
if (!cond1 || !cond2)
@ -256,6 +266,23 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
if (cond2->str() == "!")
return isOppositeCond(isNot, cpp, cond2, cond1, library, pure);
if(!isNot) {
if (cond1->str() == "==" && cond2->str() == "==") {
if(isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure))
return isDifferentKnownValues(cond1->astOperand2(), cond2->astOperand2());
if(isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure))
return isDifferentKnownValues(cond1->astOperand1(), cond2->astOperand1());
}
if(Library::isContainerYield(cond1, Library::Container::EMPTY, "empty") && Library::isContainerYield(cond2->astOperand1(), Library::Container::SIZE, "size")) {
return !(cond2->str() == "==" && cond2->astOperand2()->getValue(0));
}
if(Library::isContainerYield(cond2, Library::Container::EMPTY, "empty") && Library::isContainerYield(cond1->astOperand1(), Library::Container::SIZE, "size")) {
return !(cond1->str() == "==" && cond1->astOperand2()->getValue(0));
}
}
if (!cond1->isComparisonOp() || !cond2->isComparisonOp())
return false;

4
lib/astutils.h
View File

@ -56,6 +56,10 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure);
bool isEqualKnownValue(const Token * const tok1, const Token * const tok2);
bool isDifferentKnownValues(const Token * const tok1, const Token * const tok2);
/**
* Are two conditions opposite
* @param isNot do you want to know if cond1 is !cond2 or if cond1 and cond2 are non-overlapping. true: cond1==!cond2 false: cond1==true => cond2==false

20
lib/library.cpp
View File

@ -921,6 +921,26 @@ const Library::Container* Library::detectContainer(const Token* typeStart, bool
return nullptr;
}
bool Library::isContainerYield(const Token * const cond, Library::Container::Yield y, const std::string& fallback)
{
if(!cond)
return false;
if (cond->str() == "(") {
const Token* tok = cond->astOperand1();
if(tok && tok->str() == ".") {
if(tok->astOperand1() && tok->astOperand1()->valueType()) {
if(const Library::Container *container = tok->astOperand1()->valueType()->container) {
return tok->astOperand2() && y == container->getYield(tok->astOperand2()->str());
}
}
else if(!fallback.empty()) {
return Token::simpleMatch(cond, "( )") && cond->previous()->str() == fallback;
}
}
}
return false;
}
// returns true if ftok is not a library function
bool Library::isNotLibraryFunction(const Token *ftok) const
{

2
lib/library.h
View File

@ -434,6 +434,8 @@ public:
*/
std::string getFunctionName(const Token *ftok) const;
static bool isContainerYield(const Token * const cond, Library::Container::Yield y, const std::string& fallback="");
private:
// load a <function> xml node
Error loadFunction(const tinyxml2::XMLElement * const node, const std::string &name, std::set<std::string> &unknown_elements);

4
lib/token.h
View File

@ -798,6 +798,10 @@ public:
return _values && _values->size() == 1U && _values->front().isKnown() && _values->front().isIntValue();
}
bool hasKnownValue() const {
return _values && _values->size() == 1U && _values->front().isKnown();
}
const ValueFlow::Value * getValue(const MathLib::bigint val) const {
if (!_values)
return nullptr;

85
test/testcondition.cpp
View File

@ -37,6 +37,8 @@ private:
Settings settings1;
void run() {
LOAD_LIB_2(settings0.library, "qt.cfg");
settings0.addEnabled("style");
settings0.addEnabled("warning");
@ -81,7 +83,9 @@ private:
TEST_CASE(oppositeInnerConditionUndeclaredVariable);
TEST_CASE(oppositeInnerConditionAlias);
TEST_CASE(oppositeInnerCondition2);
TEST_CASE(oppositeInnerCondition3);
TEST_CASE(oppositeInnerConditionAnd);
TEST_CASE(oppositeInnerConditionEmpty);
TEST_CASE(identicalConditionAfterEarlyExit);
@ -1774,6 +1778,64 @@ private:
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
}
void oppositeInnerCondition3() {
check("void f3(char c) { if(c=='x') if(c=='y') {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n"
"[test.cpp:1] -> [test.cpp:1]: (style) Condition 'c=='y'' is always false\n", errout.str());
check("void f4(char *p) { if(*p=='x') if(*p=='y') {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f5(const char * const p) { if(*p=='x') if(*p=='y') {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f5(const char * const p) { if('x'==*p) if('y'==*p) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f6(char * const p) { if(*p=='x') if(*p=='y') {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f7(const char * p) { if(*p=='x') if(*p=='y') {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f8(int i) { if(i==4) if(i==2) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n"
"[test.cpp:1] -> [test.cpp:1]: (style) Condition 'i==2' is always false\n", errout.str());
check("void f9(int *p) { if (*p==4) if(*p==2) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f10(int * const p) { if (*p==4) if(*p==2) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f11(const int *p) { if (*p==4) if(*p==2) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f12(const int * const p) { if (*p==4) if(*p==2) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("struct foo {\n"
" int a;\n"
" int b;\n"
"};\n"
"void f(foo x) { if(x.a==4) if(x.b==2) {}} ");
ASSERT_EQUALS("", errout.str());
check("struct foo {\n"
" int a;\n"
" int b;\n"
"};\n"
"void f(foo x) { if(x.a==4) if(x.b==4) {}} ");
ASSERT_EQUALS("", errout.str());
check("void f3(char a, char b) { if(a==b) if(a==0) {}} ");
ASSERT_EQUALS("", errout.str());
check("void f(int x) { if (x == 1) if (x != 1) {} }");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n"
"[test.cpp:1] -> [test.cpp:1]: (style) Condition 'x!=1' is always false\n", errout.str());
}
void oppositeInnerConditionAnd() {
check("void f(int x) {\n"
" if (a>3 && x > 100) {\n"
@ -1783,6 +1845,29 @@ private:
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
}
void oppositeInnerConditionEmpty() {
check("void f1(const std::string &s) { if(s.size() > 42) if(s.empty()) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f1(const std::string &s) { if(s.empty()) if(s.size() > 42) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("template<class T> void f1(const T &s) { if(s.size() > 42) if(s.empty()) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f2(const std::wstring &s) { if(s.empty()) if(s.size() > 42) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f1(QString s) { if(s.isEmpty()) if(s.length() > 42) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f1(const std::string &s) { if(s.empty()) if(s.size() == 0) {}} ");
ASSERT_EQUALS("", errout.str());
check("void f1(const std::string &s, bool b) { if(s.empty() || ((s.size() == 1) && b)) {}} ");
ASSERT_EQUALS("", errout.str());
}
void identicalConditionAfterEarlyExit() {
check("void f(int x) {\n"
" if (x > 100) { return; }\n"