diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 80e97b727..0bb104994 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -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; diff --git a/lib/astutils.h b/lib/astutils.h index ddc34eb69..10d531bd4 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -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 diff --git a/lib/library.cpp b/lib/library.cpp index 653082e83..7eccee4b6 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -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 { diff --git a/lib/library.h b/lib/library.h index 9d58ef4cc..cb90ae702 100644 --- a/lib/library.h +++ b/lib/library.h @@ -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 xml node Error loadFunction(const tinyxml2::XMLElement * const node, const std::string &name, std::set &unknown_elements); diff --git a/lib/token.h b/lib/token.h index e3d61ce17..19946031b 100644 --- a/lib/token.h +++ b/lib/token.h @@ -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; diff --git a/test/testcondition.cpp b/test/testcondition.cpp index fc4b98c67..6f7e9dd1d 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -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 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"