Fixed #3075 (False positive => Improve tokenizer: remove redundant code after a 'return' state)

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

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.
      */

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 =(int *other) { return (*this); };\n"
-            "  inline A &operator =(long *other) { return (*this = 0;) };\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 =(int *other) { return (*this); };\n"
-            "A &A::operator =(long *other) { return (*this = 0;) };");
+            "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());
     }

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;

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));
         }

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 ;"
+                " return ; "
-                " str [ i ] = 0 ; "
                 "}",
                 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)