Fixed ticket #3528 (Tokenizer: improve simplifyFunctionParameters to take count of square brackets)

lib/tokenize.cpp

@@ -1992,7 +1992,8 @@ bool Tokenizer::tokenize(std::istream &code,
 
     // Convert K&R function declarations to modern C
     simplifyVarDecl(true);
-    simplifyFunctionParameters();
+    if (!simplifyFunctionParameters())
+        return false;
 
     // check for simple syntax errors..
     for (const Token *tok = _tokens; tok; tok = tok->next()) {
@@ -4801,7 +4802,7 @@ void Tokenizer::simplifyCasts()
 }
 
 
-void Tokenizer::simplifyFunctionParameters()
+bool Tokenizer::simplifyFunctionParameters()
 {
     for (Token *tok = _tokens; tok; tok = tok->next()) {
         if (tok->str() == "{" || tok->str() == "[" || tok->str() == "(") {
@@ -4812,79 +4813,123 @@ void Tokenizer::simplifyFunctionParameters()
         else if (Token::Match(tok, "%var% ( %var% [,)]")) {
             // We have found old style function, now we need to change it
 
-            // backup pointer to the '(' token
+            // First step: Get list of argument names in parenthesis
-            Token * const tok1 = tok->next();
+            std::map<std::string, Token *> argumentNames;
-
-            // Get list of argument names
-            std::map<std::string, Token*> argumentNames;
             bool bailOut = false;
-            for (tok = tok->tokAt(2); tok; tok = tok->tokAt(2)) {
+            Token * tokparam = NULL;
-                if (!Token::Match(tok, "%var% [,)]")) {
+
+            //floating token used to check for parameters
+            Token *tok1 = tok;
+
+            //goto '('
+            tok = tok->next();
+
+            while (NULL != (tok1 = tok1->tokAt(2))) {
+                if (!Token::Match(tok1, "%var% [,)]")) {
                     bailOut = true;
                     break;
                 }
 
-                if (argumentNames.find(tok->str()) != argumentNames.end()) {
+                //same parameters: take note of the parameter
-                    // Invalid code, two arguments with the same name.
+                if (argumentNames.find(tok1->str()) != argumentNames.end())
-                    // syntaxError(tok);
+                    tokparam = tok1;
-                    // If you uncomment it, testrunner will fail:
+                else
-                    // testclass.cpp:3910
+                    argumentNames[tok1->str()] = tok1;
-                    // because of void Fred::foo5(int, int).
-                    // how should this be handled?
-                    bailOut = true;
-                    break;
-                }
 
-                argumentNames[tok->str()] = tok;
+                if (tok1->next()->str() == ")") {
-                if (tok->next()->str() == ")") {
+                    tok1 = tok1->tokAt(2);
-                    tok = tok->tokAt(2);
+                    //expect at least a type name after round brace..
+                    if (!tok1 || !tok1->isName())
+                        bailOut = true;
                     break;
                 }
             }
 
             if (bailOut) {
-                tok = tok1->link();
+                tok = tok->link();
                 continue;
             }
 
-            Token *start = tok;
+            //there should be the sequence '; {' after the round parenthesis
-            while (tok && tok->str() != "{") {
+            if (!Token::findsimplematch(tok1, "; {")) {
-                if (tok->str() == ";") {
+                tok = tok->link();
-                    tok = tok->previous();
+                continue;
-                    // Move tokens from start to tok into the place of
+            }
-                    // argumentNames[tok->str()] and remove the ";"
 
-                    if (argumentNames.find(tok->str()) == argumentNames.end()) {
+            // Last step: check out if the declarations between ')' and '{' match the parameters list
+            std::map<std::string, Token *> argumentNames2;
+
+            while (tok1 && tok1->str() != "{") {
+                if (tok1->str() == ";") {
+                    if (tokparam) {
+                        syntaxError(tokparam);
+                        return false;
+                    }
+                    Token *tok2 = tok1->previous();
+                    while (tok2->str() == "]")
+                        tok2 = tok2->link()->previous();
+
+                    //it should be a name..
+                    if (!tok2->isName()) {
                         bailOut = true;
                         break;
                     }
 
-                    // Remove the following ";"
+                    if (argumentNames2.find(tok2->str()) != argumentNames2.end()) {
-                    Token *temp = tok->tokAt(2);
+                        //same parameter names...
-                    tok->deleteNext();
+                        syntaxError(tok1);
+                        return false;
+                    } else
+                        argumentNames2[tok2->str()] = tok2;
 
-                    // Replace "x" with "int x" or similar
+                    if (argumentNames.find(tok2->str()) == argumentNames.end()) {
-                    Token::replace(argumentNames[tok->str()], start, tok);
+                        //non-matching parameter... bailout
-                    argumentNames.erase(tok->str());
+                        bailOut = true;
-                    tok = temp;
+                        break;
-                    start = tok;
+                    }
-                } else {
-                    tok = tok->next();
                 }
-            }
+                tok1 = tok1->next();
-
-            if (Token::simpleMatch(tok, "{"))
-                tok = tok->link();
-
-            if (tok == NULL) {
-                break;
             }
 
             if (bailOut) {
+                tok = tok->link();
                 continue;
             }
+
+            //the two containers should hold the same size..
+            if (argumentNames.size() != argumentNames2.size()) {
+                tok = tok->link();
+                //error if the second container is not empty
+                if (!argumentNames2.empty()) {
+                    syntaxError(tok);
+                    return false;
+                }
+                continue;
+            }
+
+            while (tok->str() != ")") {
+                //initialize start and end tokens to be moved
+                Token *declStart = argumentNames2[tok->next()->str()];
+                Token *declEnd = declStart;
+                while (declStart->previous()->str() != ";" && declStart->previous()->str() != ")")
+                    declStart = declStart->previous();
+                while (declEnd->next()->str() != ";" && declEnd->next()->str() != "{")
+                    declEnd = declEnd->next();
+
+                //remove ';' after declaration
+                declEnd->deleteNext();
+
+                //replace the parameter name in the parenthesis with all the declaration
+                Token::replace(tok->next(), declStart, declEnd);
+
+                //since there are changes to tokens, put tok where tok1 is
+                tok = declEnd->next();
+            }
+            //goto forward and continue
+            tok = tok->next()->link();
         }
     }
+    return true;
 }
 
 void Tokenizer::simplifyPointerToStandardType()

lib/tokenize.h

@@ -449,8 +449,9 @@ public:
     /**
      * Simplify functions like "void f(x) int x; {"
      * into "void f(int x) {"
+     * @return false only if there's a syntax error
      */
-    void simplifyFunctionParameters();
+    bool simplifyFunctionParameters();
 
     /**
      * Simplify templates

test/testsimplifytokens.cpp

@@ -170,11 +170,10 @@ private:
         TEST_CASE(strcat1);
         TEST_CASE(strcat2);
 
-        // Syntax error
-        TEST_CASE(argumentsWithSameName)
-
         TEST_CASE(simplifyAtol)
+
         TEST_CASE(simplifyHexInString)
+
         TEST_CASE(simplifyTypedef1)
         TEST_CASE(simplifyTypedef2)
         TEST_CASE(simplifyTypedef3)
@@ -3392,25 +3391,6 @@ private:
         ASSERT_EQUALS(expect, tok(code));
     }
 
-    void argumentsWithSameName() {
-        // This code has syntax error, two variables can not have the same name
-        {
-            const char code[] = "void foo(x, x)\n"
-                                " int x;\n"
-                                " int x;\n"
-                                "{}\n";
-            ASSERT_EQUALS("void foo ( x , x ) int x ; int x ; { }", tok(code));
-        }
-
-        {
-            const char code[] = "void foo(x, y)\n"
-                                " int x;\n"
-                                " int x;\n"
-                                "{}\n";
-            ASSERT_EQUALS("void foo ( int x , y ) int x ; { }", tok(code));
-        }
-    }
-
     void simplifyAtol() {
         ASSERT_EQUALS("a = std :: atol ( x ) ;", tok("a = std::atol(x);"));
         ASSERT_EQUALS("a = atol ( \"text\" ) ;", tok("a = atol(\"text\");"));

test/testtokenize.cpp

@@ -240,7 +240,8 @@ private:
 
         TEST_CASE(macrodoublesharp);
 
-        TEST_CASE(simplify_function_parameters);
+        TEST_CASE(simplifyFunctionParameters);
+        TEST_CASE(simplifyFunctionParametersErrors);
 
         TEST_CASE(removeParentheses1);       // Ticket #61
         TEST_CASE(removeParentheses2);
@@ -3985,7 +3986,7 @@ private:
         ASSERT_EQUALS("DBG ( fmt , args . . . ) printf ( fmt , ## args ) ", ostr.str());
     }
 
-    void simplify_function_parameters() {
+    void simplifyFunctionParameters() {
         {
             const char code[] = "char a [ ABC ( DEF ) ] ;";
             ASSERT_EQUALS(code, tokenizeAndStringify(code, true));
@@ -3998,6 +3999,7 @@ private:
 
         ASSERT_EQUALS("void f ( int x ) { }", tokenizeAndStringify("void f(x) int x; { }", true));
         ASSERT_EQUALS("void f ( int x , char y ) { }", tokenizeAndStringify("void f(x,y) int x; char y; { }", true));
+        ASSERT_EQUALS("int main ( int argc , char * argv [ ] ) { }", tokenizeAndStringify("int main(argc,argv) int argc; char *argv[]; { }", true));
 
         // #1067 - Not simplified. Feel free to fix so it is simplified correctly but this syntax is obsolete.
         ASSERT_EQUALS("int ( * d ( a , b , c ) ) ( ) int a ; int b ; int c ; { }", tokenizeAndStringify("int (*d(a,b,c))()int a,b,c; { }", true));
@@ -4014,6 +4016,32 @@ private:
         }
     }
 
+    void simplifyFunctionParametersErrors() {
+        //same parameters...
+        tokenizeAndStringify("void foo(x, x)\n"
+                             " int x;\n"
+                             " int x;\n"
+                             "{}\n");
+        ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n", errout.str());
+
+        tokenizeAndStringify("void foo(x, y)\n"
+                             " int x;\n"
+                             " int x;\n"
+                             "{}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) syntax error\n", errout.str());
+
+        tokenizeAndStringify("void foo(int, int)\n"
+                             "{}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        //non-matching arguments after round braces
+        tokenizeAndStringify("void foo(x, y, z)\n"
+                             " int x;\n"
+                             " int y;\n"
+                             "{}\n");
+        ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n", errout.str());
+    }
+
     // Simplify "((..))" into "(..)"
     void removeParentheses1() {
         const char code[] = "void foo()"