From 89560564ed92fa96e33a2cf2c15ec67db950e1f9 Mon Sep 17 00:00:00 2001 From: Frank Zingsheim Date: Sat, 2 Feb 2013 16:01:34 +0100 Subject: [PATCH] Refactoring: Add braces to an if-block, for-block, etc. in tokenizer. Fixed #4521 (Tokenizer: Wrong braces for triple if else) --- lib/tokenize.cpp | 286 +++++++++++++++++------------------------ lib/tokenize.h | 18 ++- test/testtokenize.cpp | 23 ++-- test/testuninitvar.cpp | 2 +- 4 files changed, 145 insertions(+), 184 deletions(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 7b60e21fd..334d8d85c 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -1666,10 +1666,7 @@ bool Tokenizer::tokenize(std::istream &code, } } - simplifyDoWhileAddBraces(); - - if (!simplifyIfAddBraces()) - return false; + simplifyAddBraces(); // 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()) { - if (tok->str() == "(" || tok->str() == "[" || - (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; + for (Token *tok = list.front(); tok; tok = tok->next()) + simplifyAddBracesToCommand(tok); } -void Tokenizer::simplifyDoWhileAddBraces() +Token *Tokenizer::simplifyAddBracesToCommand(Token *tok) { - //start from the last token and proceed backwards - for (Token *tok = list.back(); tok; tok = tok->previous()) { - // fix for #988 - if (tok->str() == ")" || tok->str() == "]" || - (tok->str() == "}" && tok->link()->previous() && - tok->link()->previous()->str() == "=")) - tok = tok->link(); + Token * tokEnd=tok; + if (tok->str()=="for" || + tok->str()=="BOOST_FOREACH" || + tok->str()=="switch") { + tokEnd=simplifyAddBracesPair(tok,true); + } else if (tok->str()=="while") { + 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 !!{")) - continue; + return tokEnd; +} - Token *tok1 = tok; // token with "do" - Token *tok2 = NULL; // token with "while" - - for (Token *tok3 = tok->next(); tok3; tok3 = tok3->next()) { - if (tok3->str() == "(" || tok3->str() == "[" || tok3->str() == "{") { - tok3 = tok3->link(); - } else if (tok3->str() == "while") { - tok2 = tok3; - break; - } else if (Token::simpleMatch(tok3, "do {")) { - // Skip 'do { } while' inside the current "do" - tok3 = tok3->next()->link(); - if (tok3->strAt(1) == "while") - tok3 = tok3->next(); +Token *Tokenizer::simplifyAddBracesPair(Token *tok, bool commandWithCondition) +{ + Token * tokCondition=tok->next(); + Token *tokAfterCondition=tokCondition; + if (commandWithCondition) { + if (!tokCondition) { + // Missing condition + return tok; + } + if (tokCondition->str()=="(") + tokAfterCondition=tokCondition->link(); + else if (tokCondition->type()==Token::eName) { + // Macro condition without braces, e.g "if FAILED(hr) ... + // 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) { - // insert "{" after "do" - tok1->insertToken("{"); + tokAfterCondition->previous()->insertToken("{"); + Token * tokOpenBrace=tokAfterCondition->previous(); - // insert "}" before "while" - tok2->previous()->insertToken("}"); + tokEnd->insertToken("}"); + Token * TokCloseBrace=tokEnd->next(); - Token::createMutualLinks(tok1->next(), tok2->previous()); - } + Token::createMutualLinks(tokOpenBrace,TokCloseBrace); + tokBracesEnd=TokCloseBrace; } + + return tokBracesEnd; } void Tokenizer::simplifyCompoundAssignment() diff --git a/lib/tokenize.h b/lib/tokenize.h index a52f05a0e..5d2ac53cb 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -282,15 +282,21 @@ public: */ void simplifyComma(); - /** Add braces to an if-block - * @return true if no syntax errors + /** Add braces to an if-block, for-block, etc. */ - bool simplifyIfAddBraces(); + void simplifyAddBraces(); - /** - * Add braces to an do-while block + /** Add braces to an if-block, for-block, etc. + * 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; diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 34366fd69..2d8981af9 100644 --- a/test/testtokenize.cpp +++ b/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" - "} else {\n" + "bar1 ( ) ; }\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" - "} else {\n" + "bar1 ( ) ; }\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);")); } diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index b8366c396..4cd0e95d6 100644 --- a/test/testuninitvar.cpp +++ b/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..