diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index a9b6f2fb7..373814405 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -2742,6 +2742,8 @@ bool Tokenizer::tokenize(std::istream &code, } } + removeRedundantCodeAfterReturn(); + _tokens->assignProgressValues(); removeRedundantSemicolons(); @@ -4543,6 +4545,8 @@ bool Tokenizer::simplifyTokenList() simplifyGoto(); + removeRedundantCodeAfterReturn(); + // Combine wide strings for (Token *tok = _tokens; tok; tok = tok->next()) { @@ -4888,6 +4892,195 @@ void Tokenizer::removeRedundantAssignment() } } +void Tokenizer::removeRedundantCodeAfterReturn() +{ + unsigned int indentlevel = 0; + unsigned int indentcase = 0; + unsigned int indentret = 0; //this is the indentation level when 'return ;' token is found; + unsigned int indentswitch = 0; + unsigned int indentlabel = 0; + bool ret = false; //is it already found a 'return' token? + bool switched = false; //is it there a switch code? + for (Token *tok = _tokens; tok; tok = tok->next()) + { + if (tok->str() == "{") + { + ++indentlevel; + if (ret) + { + unsigned int indentlevel1 = indentlevel; + for (Token *tok2 = tok->next(); tok2; tok2 = tok2->next()) + { + if (tok2->str() == "{") + ++indentlevel1; + else if (tok2->str() == "}") + { + if (indentlevel1 == indentlevel) + break; + --indentlevel1; + } + else if (Token::Match(tok2, "%var% : ;") + && tok2->str()!="case" && tok2->str()!="default") + { + indentlabel = indentlevel1; + break; + } + } + if (indentlevel > indentlabel) + { + tok = tok->previous(); + tok->deleteNext(); + } + } + } + + else if (tok->str() == "}") + { + if (indentlevel == 0) + break; // break out - it seems the code is wrong + //there's already a 'return ;' and more indentation! + if (ret) + { + if (!switched || indentlevel > indentcase) + { + if (indentlevel > indentret && indentlevel > indentlabel) + { + tok = tok->previous(); + tok->deleteNext(); + } + } + else + { + if (indentcase >= indentret && indentlevel > indentlabel) + { + tok = tok->previous(); + tok->deleteNext(); + } + } + } + if (indentlevel == indentret) + ret = false; + --indentlevel; + if (indentlevel <= indentcase) + { + if (!indentswitch) + { + indentcase = 0; + switched = false; + } + else + { + --indentswitch; + indentcase = indentlevel-1; + } + } + } + else if (!ret) + { + if (tok->str() == "switch") + { + if (indentlevel == 0) + break; + if (!switched) + { + for (Token *tok2 = tok->next(); tok2; tok2 = tok2->next()) + { + if (tok2->str() == "{") + { + switched = true; + tok = tok2; + break; + } + else if (tok2->str() == "}") + break; //bad code + } + if (!switched) + break; + ++indentlevel; + indentcase = indentlevel; + } + else + { + ++indentswitch; + //todo this case + } + } + else if (switched + && (tok->str() == "case" || tok->str() == "default")) + { + if (indentlevel > indentcase) + { + --indentlevel; + } + for (Token *tok2 = tok->next(); tok2; tok2 = tok2->next()) + { + if (tok2->str() == ":" || Token::Match(tok2, ": ;")) + { + if (indentlevel == indentcase) + { + ++indentlevel; + } + tok = tok2; + break; + } + else if (tok2->str() == "}" || tok2->str() == "{") + break; //bad code + } + } + else if (tok->str() == "return") + { + if (indentlevel == 0) + break; // break out - never seen a 'return' not inside a scope; + + //catch the first ';' after the return + for (Token *tok2 = tok->next(); tok2; tok2 = tok2->next()) + { + if (tok2->str() == ";") + { + ret = true; + tok = tok2; + break; + } + else if (tok2->str() == "{" || tok2->str() == "}") + break; //I think this is an error code... + } + if (!ret) + break; + indentret = indentlevel; + } + } + else if (ret) //there's already a "return;" declaration + { + if (!switched || indentlevel > indentcase+1) + { + if (indentlevel >= indentret && (!(Token::Match(tok, "%var% : ;")) || tok->str()=="case" || tok->str()=="default")) + { + tok = tok->previous(); + tok->deleteNext(); + } + else + { + ret = false; + } + } + else + { + if (!(Token::Match(tok, "%var% : ;")) && tok -> str() != "case" && tok->str() != "default") + { + tok = tok->previous(); + tok->deleteNext(); + } + else + { + ret = false; + tok = tok->previous(); + } + } + } + } +} + + bool Tokenizer::removeReduntantConditions() { // Return value for function. Set to true if there are any simplifications diff --git a/lib/tokenize.h b/lib/tokenize.h index 2573bd5f0..b0b417279 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -199,6 +199,9 @@ public: /** Remove redundant assignment */ void removeRedundantAssignment(); + /** Remove redudant code after return */ + void removeRedundantCodeAfterReturn(); + /** * Replace sizeof() to appropriate size. */ diff --git a/test/testclass.cpp b/test/testclass.cpp index 0ca29ac05..81d20bdf3 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -467,8 +467,8 @@ private: checkOpertorEqRetRefThis( "class A {\n" "public:\n" - " inline A &operator =(int *other) { return (*this;) };\n" - " inline A &operator =(long *other) { return (*this = 0;) };\n" + " inline A &operator =(int *other) { return (*this); };\n" + " inline A &operator =(long *other) { return (*this = 0); };\n" "};"); ASSERT_EQUALS("", errout.str()); @@ -478,14 +478,14 @@ private: " A &operator =(int *other);\n" " A &operator =(long *other);\n" "};\n" - "A &A::operator =(int *other) { return (*this;) };\n" - "A &A::operator =(long *other) { return (*this = 0;) };"); + "A &A::operator =(int *other) { return (*this); };\n" + "A &A::operator =(long *other) { return (*this = 0); };"); ASSERT_EQUALS("", errout.str()); checkOpertorEqRetRefThis( "class A {\n" "public:\n" - " inline A &operator =(int *other) { return (*this;) };\n" + " inline A &operator =(int *other) { return (*this); };\n" " inline A &operator =(long *other) { return operator = (*(int *)other); };\n" "};"); ASSERT_EQUALS("", errout.str()); @@ -496,7 +496,7 @@ private: " A &operator =(int *other);\n" " A &operator =(long *other);\n" "};\n" - "A &A::operator =(int *other) { return (*this;) };\n" + "A &A::operator =(int *other) { return (*this); };\n" "A &A::operator =(long *other) { return operator = (*(int *)other); };"); ASSERT_EQUALS("", errout.str()); @@ -506,7 +506,7 @@ private: " A &operator =(int *other);\n" " A &operator =(long *other);\n" "};\n" - "A &A::operator =(int *other) { return (*this;) };\n" + "A &A::operator =(int *other) { return (*this); };\n" "A &A::operator =(long *other) { return this->operator = (*(int *)other); };"); ASSERT_EQUALS("", errout.str()); } diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index c9cb1fede..5d570bac9 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -349,6 +349,7 @@ private: // #2662: segfault because of endless recursion (call_func -> getAllocationType -> functionReturnType -> call_func ..) TEST_CASE(trac2662); + TEST_CASE(redundantCodeAfterReturnGoto); } @@ -3838,6 +3839,22 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + //ticket # 3083 + void redundantCodeAfterReturnGoto() + { + check("void foo()\n" + "{\n" + " char * data;\n" + " data = NULL;\n" + " goto source;\n" + " data = (char *)malloc((10+1)*sizeof(char));\n" + "source:\n" + " data = (char *)malloc(10*sizeof(char));\n" + " free(data);\n" + "}"); + ASSERT_EQUALS("",errout.str()); + } }; static TestMemleakInFunction testMemleakInFunction; diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index 68b30e749..f923effc7 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -441,7 +441,7 @@ private: "}\n"; const char expected[] = "static void crash ( ) " - "{ foo ( ) ; return ; foo ( ) ; }"; + "{ foo ( ) ; return ; }"; ASSERT_EQUALS(expected, tok(code)); } diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index c98d8214e..9dd3c5136 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -355,6 +355,9 @@ private: TEST_CASE(multipleAssignment); TEST_CASE(simplifyIfAddBraces); // ticket # 2739 (segmentation fault) + + //remove redundant code after the 'return ;' statement + TEST_CASE(removeRedundantCodeAfterReturn); } @@ -1568,8 +1571,7 @@ private: "{" " int i ;" " for ( i = 0 ; i < 10 ; ++ i ) { }" - " return ;" - " str [ i ] = 0 ; " + " return ; " "}", simplifyKnownVariables(code)); } @@ -5869,6 +5871,49 @@ private: tokenizeAndStringify(code)); } } + + void removeRedundantCodeAfterReturn() + { + ASSERT_EQUALS("void f ( ) { return ; }", tokenizeAndStringify("void f() { return; foo();}")); + ASSERT_EQUALS("void f ( int n ) { if ( n ) { return ; } foo ( ) ; }",tokenizeAndStringify("void f(int n) { if (n) return; foo();}")); + + ASSERT_EQUALS("int f ( int n ) { switch ( n ) { case 0 : return 0 ; default : ; return n ; } return -1 ; }", + tokenizeAndStringify("int f(int n) { switch (n) {case 0: return 0; n*=2; default: return n; n*=6;} return -1; foo();}")); + + { + const char code[] = "void f(){ " + "if (k>0) goto label; " + "return; " + "if (tnt) " + " { " + " { " + " check(); " + " k=0; " + " } " + " label: " + " bar(); " + " } " + "}"; + ASSERT_EQUALS("void f ( ) { if ( 0 < k ) { goto label ; } return ; { label : ; bar ( ) ; } }",simplifyKnownVariables(code)); + } + + { + const char code[] = "int f() { " + "switch (x) { case 1: return 1; bar(); tack; { ticak(); return; } return; " + "case 2: return 2; { reere(); } tack(); " + "switch(y) { case 1: return 0; case 2: return 7; } " + "return 2; } return 3; }"; + ASSERT_EQUALS("int f ( ) { switch ( x ) { case 1 : return 1 ; case 2 : return 2 ; } return 3 ; }",simplifyKnownVariables(code)); + } + + { + const char code[] = "int f() { " + "switch (x) { case 1: return 1; bar(); tack; { ticak(); return; } return; " + "case 2: switch(y) { case 1: return 0; bar2(); foo(); case 2: return 7; } " + "return 2; } return 3; }"; + ASSERT_EQUALS("int f ( ) { switch ( x ) { case 1 : return 1 ; case 2 : switch ( y ) { case 1 : return 0 ; case 2 : return 7 ; } return 2 ; } return 3 ; }",simplifyKnownVariables(code)); + } + } }; REGISTER_TEST(TestTokenizer)