Refactoring: Add braces to an if-block, for-block, etc. in tokenizer.

Fixed #4521 (Tokenizer: Wrong braces for triple if else)

lib/tokenize.cpp

@@ -1666,10 +1666,7 @@ bool Tokenizer::tokenize(std::istream &code,
         }
     }
 
-    simplifyDoWhileAddBraces();
+    simplifyAddBraces();
-
-    if (!simplifyIfAddBraces())
-        return false;
 
     // Combine tokens..
     for (Token *tok = list.front(); tok && tok->next(); tok = tok->next()) {
@@ -4054,181 +4051,136 @@ void Tokenizer::removeRedundantSemicolons()
 }
 
 
-bool Tokenizer::simplifyIfAddBraces()
+void Tokenizer::simplifyAddBraces()
 {
-    for (Token *tok = list.front(); tok; tok = tok->next()) {
+    for (Token *tok = list.front(); tok; tok = tok->next())
-        if (tok->str() == "(" || tok->str() == "[" ||
+        simplifyAddBracesToCommand(tok);
-            (tok->str() == "{" && tok->previous() && tok->previous()->str() == "=")) {
-            tok = tok->link();
-            continue;
-        }
-
-        if (Token::Match(tok, "if|for|while|BOOST_FOREACH (")) {
-
-            if (tok->strAt(2) == ")") {
-                //no arguments inside round braces, abort
-                syntaxError(tok);
-                return false;
-            }
-            // don't add "{}" around ";" in "do {} while();" (#609)
-            const Token *prev = tok->previous();
-            if (prev && prev->str() == "}" && tok->str() == "while") {
-                prev = prev->link()->previous();
-                if (prev && prev->str() == "do")
-                    continue;
-            }
-
-            // Goto the ending ')'
-            tok = tok->next()->link();
-
-            // there's already '{' after ')', don't bother
-            if (tok->next() && tok->next()->str() == "{")
-                continue;
-        }
-
-        else if (tok->str() == "else") {
-            // An else followed by an if or brace don't need to be processed further
-            if (Token::Match(tok, "else if|{"))
-                continue;
-        }
-
-        else {
-            continue;
-        }
-
-        // If there is no code after the 'if()' or 'else', abort
-        if (!tok->next()) {
-            syntaxError(tok);
-            return false;
-        }
-
-        // insert open brace..
-        tok->insertToken("{");
-        tok = tok->next();
-        Token *tempToken = tok;
-
-        // if (cond1) for(;;) if (cond2) ; else ;
-        while (Token::Match(tempToken->next(), "for|while|BOOST_FOREACH ("))
-            tempToken = tempToken->linkAt(2);
-
-        bool innerIf = (tempToken->next() && tempToken->next()->str() == "if");
-
-        if (Token::simpleMatch(tempToken->next(), "do {"))
-            tempToken = tempToken->linkAt(2);
-
-        // insert close brace..
-        // In most cases it would work to just search for the next ';' and insert a closing brace after it.
-        // But here are special cases..
-        // * if (cond) for (;;) break;
-        // * if (cond1) if (cond2) { }
-        // * if (cond1) if (cond2) ; else ;
-        while (NULL != (tempToken = tempToken->next())) {
-            if (tempToken->str() == "{") {
-                if (tempToken->previous()->str() == "=") {
-                    tempToken = tempToken->link();
-                    continue;
-                }
-
-                if (tempToken->previous()->str() == "else") {
-                    if (innerIf)
-                        tempToken = tempToken->link();
-                    else
-                        tempToken = tempToken->tokAt(-2);
-                    break;
-                }
-                tempToken = tempToken->link();
-                if (!tempToken->next())
-                    break;
-                if (Token::simpleMatch(tempToken, "} else") && !Token::Match(tempToken->tokAt(2), "if|{"))
-                    innerIf = false;
-                else if (tempToken->next()->isName() && tempToken->next()->str() != "else")
-                    break;
-                continue;
-            }
-
-            if (tempToken->str() == "(" || tempToken->str() == "[") {
-                tempToken = tempToken->link();
-                continue;
-            }
-
-            if (tempToken->str() == "}") {
-                // insert closing brace before this token
-                tempToken = tempToken->previous();
-                break;
-            }
-
-            if (tempToken->str() == ";") {
-                if (!innerIf)
-                    break;
-
-                if (Token::simpleMatch(tempToken, "; else")) {
-                    if (tempToken->strAt(2) != "if")
-                        innerIf = false;
-                } else
-                    break;
-            }
-        }
-
-        if (tempToken) {
-            tempToken->insertToken("}");
-            Token::createMutualLinks(tok, tempToken->next());
-
-            // move '}' in the same line as 'else' if there's it after the new token,
-            // except for '}' which is after '{ ; }'
-            tempToken = tempToken->next();
-            if (!Token::simpleMatch(tempToken->link(), "{ ; }") && tempToken->next() && tempToken->next()->str() == "else" &&
-                tempToken->next()->linenr() != tempToken->linenr())
-                tempToken->linenr(tempToken->next()->linenr());
-        } else {
-            // Can't insert matching "}" so give up.  This is fatal because it
-            // causes unbalanced braces.
-            syntaxError(tok);
-            return false;
-        }
-    }
-    return true;
 }
 
-void Tokenizer::simplifyDoWhileAddBraces()
+Token *Tokenizer::simplifyAddBracesToCommand(Token *tok)
 {
-    //start from the last token and proceed backwards
+    Token * tokEnd=tok;
-    for (Token *tok = list.back(); tok; tok = tok->previous()) {
+    if (tok->str()=="for" ||
-        // fix for #988
+        tok->str()=="BOOST_FOREACH" ||
-        if (tok->str() == ")" || tok->str() == "]" ||
+        tok->str()=="switch") {
-            (tok->str() == "}" && tok->link()->previous() &&
+        tokEnd=simplifyAddBracesPair(tok,true);
-             tok->link()->previous()->str() == "="))
+    } else if (tok->str()=="while") {
-            tok = tok->link();
+        Token *tokPossibleDo=tok->previous();
+        if (tokPossibleDo &&
+            tokPossibleDo->str()=="}")
+            tokPossibleDo=tokPossibleDo->link();
+        if (tokPossibleDo)
+            tokPossibleDo=tokPossibleDo->previous();
+        if (!tokPossibleDo ||
+            tokPossibleDo->str()!="do")
+            tokEnd=simplifyAddBracesPair(tok,true);
+    } else if (tok->str()=="do") {
+        tokEnd=simplifyAddBracesPair(tok,false);
+        if (tokEnd!=tok) {
+            // walk on to next token, i.e. "while"
+            // such that simplifyAddBracesPair does not close other braces
+            // before the "while"
+            if (tokEnd) {
+                tokEnd=tokEnd->next();
+                if (!tokEnd)
+                    // no while return input token
+                    tokEnd=tok;
+            }
+        }
+    } else if (tok->str()=="if") {
+        tokEnd=simplifyAddBracesPair(tok,true);
+        Token * tokEndNext=tokEnd->next();
+        if (tokEndNext && tokEndNext->str()=="else") {
+            Token * tokEndNextNext=tokEndNext->next();
+            if (tokEndNextNext && tokEndNextNext->str()=="if") {
+                // do not change "else if ..." to "else { if ... }"
+                tokEnd=simplifyAddBracesToCommand(tokEndNextNext);
+            } else
+                tokEnd=simplifyAddBracesPair(tokEndNext,false);
+        }
+    }
 
-        if (!Token::Match(tok, "do !!{"))
+    return tokEnd;
-            continue;
+}
 
-        Token *tok1 = tok;  // token with "do"
+Token *Tokenizer::simplifyAddBracesPair(Token *tok, bool commandWithCondition)
-        Token *tok2 = NULL; // token with "while"
+{
-
+    Token * tokCondition=tok->next();
-        for (Token *tok3 = tok->next(); tok3; tok3 = tok3->next()) {
+    Token *tokAfterCondition=tokCondition;
-            if (tok3->str() == "(" || tok3->str() == "[" || tok3->str() == "{") {
+    if (commandWithCondition) {
-                tok3 = tok3->link();
+        if (!tokCondition) {
-            } else if (tok3->str() == "while") {
+            // Missing condition
-                tok2 = tok3;
+            return tok;
-                break;
+        }
-            } else if (Token::simpleMatch(tok3, "do {")) {
+        if (tokCondition->str()=="(")
-                // Skip 'do { } while' inside the current "do"
+            tokAfterCondition=tokCondition->link();
-                tok3 = tok3->next()->link();
+        else if (tokCondition->type()==Token::eName) {
-                if (tok3->strAt(1) == "while")
+            // Macro condition without braces, e.g "if FAILED(hr) ...
-                    tok3 = tok3->next();
+            // see for example TestMemleakInFunction::switch4
+            tokAfterCondition=tokCondition;
+            tokAfterCondition=tokAfterCondition->next();
+            if (tokAfterCondition &&
+                tokAfterCondition->str()=="(")
+                tokAfterCondition=tokAfterCondition->link();
+        }
+        if (tokAfterCondition) {
+            if (tokAfterCondition->str()!=")") {
+                // Bad condition
+                return tok;
+            }
+            tokAfterCondition=tokAfterCondition->next();
+        }
+    }
+    if (!tokAfterCondition ||
+        ((tokAfterCondition->type()==Token::eBracket ||
+          tokAfterCondition->type()==Token::eExtendedOp)&&
+         Token::Match(tokAfterCondition,")|}|>|,"))) {
+        // No tokens left where to add braces around
+        return tok;
+    }
+    Token * tokBracesEnd=NULL;
+    if (tokAfterCondition->str()=="{") {
+        // already surounded by braces
+        tokBracesEnd=tokAfterCondition->link();
+    } else {
+        Token * tokEnd = simplifyAddBracesToCommand(tokAfterCondition);
+        if (tokEnd->str()!="}") {
+            // Token does not end with brace
+            // Look for ; to add own closing brace after it
+            while (tokEnd &&
+                   tokEnd->str()!=";" &&
+                   !((tokEnd->type()==Token::eBracket ||
+                      tokEnd->type()==Token::eExtendedOp)&&
+                     Token::Match(tokEnd,")|}|>"))) {
+                if (tokEnd->type()==Token::eBracket ||
+                    (tokEnd->type()==Token::eExtendedOp && tokEnd->str()=="(")) {
+                    Token *tokInnerCloseBraket=tokEnd->link();
+                    if (!tokInnerCloseBraket) {
+                        // Inner bracket does not close
+                        return tok;
+                    }
+                    tokEnd=tokInnerCloseBraket;
+                }
+                tokEnd=tokEnd->next();
+            }
+            if (!tokEnd ||
+                tokEnd->str()!=";") {
+                // No trailing ;
+                return tok;
             }
         }
 
-        if (tok2) {
+        tokAfterCondition->previous()->insertToken("{");
-            // insert "{" after "do"
+        Token * tokOpenBrace=tokAfterCondition->previous();
-            tok1->insertToken("{");
 
-            // insert "}" before "while"
+        tokEnd->insertToken("}");
-            tok2->previous()->insertToken("}");
+        Token * TokCloseBrace=tokEnd->next();
 
-            Token::createMutualLinks(tok1->next(), tok2->previous());
+        Token::createMutualLinks(tokOpenBrace,TokCloseBrace);
-        }
+        tokBracesEnd=TokCloseBrace;
     }
+
+    return tokBracesEnd;
 }
 
 void Tokenizer::simplifyCompoundAssignment()

lib/tokenize.h

@@ -282,15 +282,21 @@ public:
      */
     void simplifyComma();
 
-    /** Add braces to an if-block
+    /** Add braces to an if-block, for-block, etc.
-     * @return true if no syntax errors
      */
-    bool simplifyIfAddBraces();
+    void simplifyAddBraces();
 
-    /**
+    /** Add braces to an if-block, for-block, etc.
-     * Add braces to an do-while block
+     * for command starting at token including else-block
+     * @return last token of command (or input token in case of error or no braces have been added)
      */
-    void simplifyDoWhileAddBraces();
+    Token * simplifyAddBracesToCommand(Token * tok);
+
+    /** Add pair of braces to an single if-block, else-block, for-block, etc.
+     * for command starting at token
+     * @return last token of command (or input token in case of error or no braces have been added)
+     */
+    Token * simplifyAddBracesPair(Token *tok, bool commandWithCondition);
 
     /**
      * typedef A mytype;

test/testtokenize.cpp

@@ -1105,9 +1105,8 @@ private:
             "}";
         const char expected[] =
             "void f ( ) { "
-            "for ( int k = 0 ; k < VectorSize ; k ++ ) { "
+            "for ( int k = 0 ; k < VectorSize ; k ++ ) "
             "LOG_OUT ( ID_Vector [ k ] ) "
-            "} "
             "}";
         ASSERT_EQUALS(expected, tokenizeAndStringify(code, true));
     }
@@ -1115,7 +1114,7 @@ private:
     void ifAddBraces10() {
         // ticket #1361
         const char code[] = "{ DEBUG(if (x) y; else z); }";
-        const char expected[] = "{ DEBUG ( if ( x ) y ; else z ) ; }";
+        const char expected[] = "{ DEBUG ( if ( x ) { y ; } else z ) ; }";
         ASSERT_EQUALS(expected, tokenizeAndStringify(code, true));
     }
 
@@ -1156,14 +1155,14 @@ private:
 
     void ifAddBraces16() { // ticket # 2739 (segmentation fault)
         tokenizeAndStringify("if()x");
-        ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
 
         // ticket #2873 - the fix is not needed anymore.
         {
             const char code[] = "void f() { "
                                 "(void) ( { if(*p) (*p) = x(); } ) "
                                 "}";
-            ASSERT_EQUALS("void f ( ) { ( void ) ( { if ( * p ) ( * p ) = x ( ) ; } ) }",
+            ASSERT_EQUALS("void f ( ) { ( void ) ( { if ( * p ) { ( * p ) = x ( ) ; } } ) }",
                           tokenizeAndStringify(code));
         }
     }
@@ -1180,8 +1179,9 @@ private:
         ASSERT_EQUALS("void f ( )\n"
                       "{\n"
                       "if ( a ) {\n"
-                      "bar1 ( ) ;\n\n"
+                      "bar1 ( ) ; }\n"
-                      "} else {\n"
+                      "\n"
+                      "else {\n"
                       "bar2 ( ) ; }\n"
                       "}", tokenizeAndStringify(code, true));
     }
@@ -1190,6 +1190,9 @@ private:
         // ticket #3424 - if if { } else else
         ASSERT_EQUALS("{ if ( x ) { if ( y ) { } else { ; } } else { ; } }",
                       tokenizeAndStringify("{ if(x) if(y){}else;else;}", false));
+
+        ASSERT_EQUALS("{ if ( x ) { if ( y ) { if ( z ) { } else { ; } } else { ; } } else { ; } }",
+                      tokenizeAndStringify("{ if(x) if(y) if(z){}else;else;else;}", false));
     }
 
     void ifAddBraces19() {
@@ -1208,8 +1211,8 @@ private:
                       "if ( a ) {\n"
                       "for ( ; ; ) {\n"
                       "if ( b ) {\n"
-                      "bar1 ( ) ;\n"
+                      "bar1 ( ) ; }\n"
-                      "} else {\n"
+                      "else {\n"
                       "bar2 ( ) ; } } }\n"
                       "}", tokenizeAndStringify(code, true));
     }
@@ -7241,7 +7244,7 @@ private:
     }
 
     void simplifyNull() {
-        ASSERT_EQUALS("if ( p == 0 )", tokenizeAndStringify("if (p==NULL)"));
+        ASSERT_EQUALS("if ( ! p )", tokenizeAndStringify("if (p==NULL)"));
         ASSERT_EQUALS("f ( NULL ) ;", tokenizeAndStringify("f(NULL);"));
     }
 

test/testuninitvar.cpp

@@ -1020,7 +1020,7 @@ private:
                        "    else c = in + strlen(in) - 1;\n"
                        "    *c = 0;\n"
                        "}\n");
-        TODO_ASSERT_EQUALS("", "[test.cpp:5]: (error) Uninitialized variable: c\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
     }
 
     // switch..