From 0254344df53d543359d1dfac4f1c35a3070842a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 10 Aug 2012 14:06:24 +0200 Subject: [PATCH] Tokenizer::simplifyEnum: Reverted refactorings/optimisations as there were regressions (#3949, #3950, #4025) --- lib/tokenize.cpp | 280 ++++++++++++++---------------------- lib/tokenize.h | 14 +- test/testsimplifytokens.cpp | 19 +-- 3 files changed, 119 insertions(+), 194 deletions(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 826966598..5a2dbe822 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -6866,81 +6866,6 @@ bool Tokenizer::duplicateDefinition(Token ** tokPtr, const Token * name) const return false; } -class EnumValue { -public: - EnumValue() { - name = 0; - value = 0; - start = 0; - end = 0; - } - EnumValue(const EnumValue &ev) { - name = ev.name; - value = ev.value; - start = ev.start; - end = ev.end; - } - EnumValue(Token *name_, Token *value_, Token *start_, Token *end_) { - name = name_; - value = value_; - start = start_; - end = end_; - } - - void simplify(const std::map &enumValues) { - for (Token *tok = start; tok; tok = tok->next()) { - if (enumValues.find(tok->str()) != enumValues.end()) { - const EnumValue &other = enumValues.find(tok->str())->second; - if (other.value != NULL) - tok->str(other.value->str()); - else { - bool islast = (tok == end); - Token *last = Tokenizer::copyTokens(tok, other.start, other.end); - if (last == tok->next()) // tok->deleteThis() invalidates a pointer that points at the next token - last = tok; - tok->deleteThis(); - if (islast) { - end = last; - } - tok = last; - } - } - if (tok == end) - break; - } - - // Simplify calculations.. - while (Token::Match(start, "%num% %op% %num% %op%") && - start->strAt(1) == start->strAt(3)) { - const std::string &op = start->strAt(1); - if (op.size() != 1U) - break; - const std::string &val1 = start->str(); - const std::string &val2 = start->strAt(2); - const std::string result = MathLib::calculate(val1, val2, op[0]); - start->str(result); - start->deleteNext(2); - } - if (Token::Match(start, "%num% %op% %num% [,}]")) { - const std::string &op = start->strAt(1); - if (op.size() == 1U) { - const std::string &val1 = start->str(); - const std::string &val2 = start->strAt(2); - const std::string result = MathLib::calculate(val1, val2, op[0]); - start->str(result); - start->deleteNext(2); - value = start; - start = end = 0; - } - } - } - - Token *name; - Token *value; - Token *start; - Token *end; -}; - void Tokenizer::simplifyEnum() { // Don't simplify enums in java files @@ -7043,7 +6968,6 @@ void Tokenizer::simplifyEnum() // iterate over all enumerators between { and } // Give each enumerator the const value specified or if not specified, 1 + the // previous value or 0 if it is the first one. - std::map enumValues; for (; tok1 && tok1 != end; tok1 = tok1->next()) { Token * enumName = 0; Token * enumValue = 0; @@ -7070,7 +6994,7 @@ void Tokenizer::simplifyEnum() // value is previous expression + 1 tok1->insertToken("+"); tok1 = tok1->next(); - tok1->insertToken("1"); + tok1->insertToken(MathLib::toString(lastValue)); enumValue = 0; enumValueStart = valueStart->next(); enumValueEnd = tok1->next(); @@ -7094,10 +7018,19 @@ void Tokenizer::simplifyEnum() enumValueStart = tok1; enumValueEnd = tok1; int level = 0; - while (enumValueEnd->next() && (!Token::Match(enumValueEnd->next(), "[},]") || level)) { - if (Token::Match(enumValueEnd, "(|[")) + if (enumValueEnd->str() == "(" || + enumValueEnd->str() == "[" || + enumValueEnd->str() == "{") + ++level; + while (enumValueEnd->next() && + (!Token::Match(enumValueEnd->next(), "}|,") || level)) { + if (enumValueEnd->next()->str() == "(" || + enumValueEnd->next()->str() == "[" || + enumValueEnd->next()->str() == "{") ++level; - else if (Token::Match(enumValueEnd->next(), "]|)")) + else if (enumValueEnd->next()->str() == ")" || + enumValueEnd->next()->str() == "]" || + enumValueEnd->next()->str() == "}") --level; enumValueEnd = enumValueEnd->next(); @@ -7109,102 +7042,111 @@ void Tokenizer::simplifyEnum() tok1 = enumValueEnd; } - // add enumerator constant.. + // find all uses of this enumerator and substitute it's value for it's name if (enumName && (enumValue || (enumValueStart && enumValueEnd))) { - EnumValue ev(enumName, enumValue, enumValueStart, enumValueEnd); - ev.simplify(enumValues); - enumValues[enumName->str()] = ev; - lastEnumValueStart = ev.start; - lastEnumValueEnd = ev.end; - if (ev.start == NULL) - lastValue = MathLib::toLongNumber(ev.value->str()); - tok1 = ev.end ? ev.end : ev.value; - } - } + const std::string pattern = className.empty() ? + std::string("") : + std::string(className + " :: " + enumName->str()); + int level = 1; + bool inScope = true; - // Substitute enum values - { - const std::string pattern = className.empty() ? - std::string("") : - std::string(className + " :: "); - int level = 1; - bool inScope = true; + bool exitThisScope = false; + int exitScope = 0; + bool simplify = false; + bool hasClass = false; + const Token *endScope = 0; + for (Token *tok2 = tok1->next(); tok2; tok2 = tok2->next()) { + if (tok2->str() == "}") { + --level; + if (level < 0) + inScope = false; - std::stack > shadowId; // duplicate ids in inner scope - bool simplify = false; - bool hasClass = false; - EnumValue *ev = NULL; - for (Token *tok2 = tok1->next(); tok2; tok2 = tok2->next()) { - if (tok2->str() == "}") { - --level; - if (level < 0) - inScope = false; - - if (!shadowId.empty()) - shadowId.pop(); - } else if (tok2->str() == "{") { - // Is the same enum redefined? - const Token *begin = end->link(); - if (tok2->fileIndex() == begin->fileIndex() && - tok2->linenr() == begin->linenr() && - Token::Match(begin->tokAt(-2), "enum %type% {") && - Token::Match(tok2->tokAt(-2), "enum %type% {") && - begin->previous()->str() == tok2->previous()->str()) { - // remove duplicate enum - Token * startToken = tok2->tokAt(-3); - tok2 = tok2->link()->next(); - Token::eraseTokens(startToken, tok2); - if (!tok2) - break; - } else { - // Not a duplicate enum.. - ++level; - - // Create a copy of the shadow ids for the inner scope - if (!shadowId.empty()) - shadowId.push(shadowId.top()); - } - } else if (!pattern.empty() && Token::Match(tok2, pattern.c_str()) && enumValues.find(tok2->strAt(2)) != enumValues.end()) { - simplify = true; - hasClass = true; - ev = &(enumValues.find(tok2->strAt(2))->second); - } else if (inScope && // enum is in scope - (shadowId.empty() || shadowId.top().find(tok2->str()) == shadowId.top().end()) && // no shadow enum/var/etc of enum - enumValues.find(tok2->str()) != enumValues.end()) { // tok2 is a enum id with a known value - ev = &(enumValues.find(tok2->str())->second); - if (!duplicateDefinition(&tok2, ev->name)) { - if (tok2->strAt(-1) == "::" || - Token::Match(tok2->next(), "::|[")) { - // Don't replace this enum if: - // * it's preceded or followed by "::" - // * it's followed by "[" - } else { - simplify = true; - hasClass = false; - ev = &(enumValues.find(tok2->str())->second); + if (exitThisScope) { + if (level < exitScope) + exitThisScope = false; + } + } else if (tok2->str() == "{") { + // Is the same enum redefined? + const Token *begin = end->link(); + if (tok2->fileIndex() == begin->fileIndex() && + tok2->linenr() == begin->linenr() && + Token::Match(begin->tokAt(-2), "enum %type% {") && + Token::Match(tok2->tokAt(-2), "enum %type% {") && + begin->previous()->str() == tok2->previous()->str()) { + // remove duplicate enum + Token * startToken = tok2->tokAt(-3); + tok2 = tok2->link()->next(); + Token::eraseTokens(startToken, tok2); + if (!tok2) + break; + } else { + // Not a duplicate enum.. + ++level; + } + endScope = tok2->link(); + } else if (!pattern.empty() && Token::Match(tok2, pattern.c_str())) { + simplify = true; + hasClass = true; + } else if (inScope && !exitThisScope && tok2->str() == enumName->str()) { + if (!duplicateDefinition(&tok2, enumName)) { + if (tok2->strAt(-1) == "::" || + Token::Match(tok2->next(), "::|[")) { + // Don't replace this enum if: + // * it's preceded or followed by "::" + // * it's followed by "[" + } else { + simplify = true; + hasClass = false; + } + } else { + // something with the same name. + exitScope = level; + if (endScope) + tok2 = endScope->previous(); } - } else { - // something with the same name. - if (shadowId.empty()) - shadowId.push(std::set()); - shadowId.top().insert(tok2->str()); - } - } - - if (simplify) { - if (ev->value) - tok2->str(ev->value->str()); - else { - tok2 = tok2->previous(); - tok2->deleteNext(); - tok2 = copyTokens(tok2, ev->start, ev->end); } - if (hasClass) { - tok2->deleteNext(2); - } + if (simplify) { + // Simplify calculations.. + while (Token::Match(enumValueStart, "%num% %op% %num% %op%") && + enumValueStart->strAt(1) == enumValueStart->strAt(3)) { + const std::string &op = enumValueStart->strAt(1); + if (op.size() != 1U) + break; + const std::string &val1 = enumValueStart->str(); + const std::string &val2 = enumValueStart->strAt(2); + const std::string result = MathLib::calculate(val1, val2, op[0]); + enumValueStart->str(result); + enumValueStart->deleteNext(2); + } + if (Token::Match(enumValueStart, "%num% %op% %num% [,}]")) { + const std::string &op = enumValueStart->strAt(1); + if (op.size() == 1U) { + const std::string &val1 = enumValueStart->str(); + const std::string &val2 = enumValueStart->strAt(2); + const std::string result = MathLib::calculate(val1, val2, op[0]); + enumValueStart->str(result); + enumValueStart->deleteNext(2); + enumValue = tok1 = enumValueStart; + enumValueStart = enumValueEnd = 0; + } + } - simplify = false; + + if (enumValue) + tok2->str(enumValue->str()); + else { + tok2 = tok2->previous(); + tok2->deleteNext(); + tok2 = copyTokens(tok2, enumValueStart, enumValueEnd); + } + + if (hasClass) { + tok2->deleteNext(2); + } + + simplify = false; + } } } } diff --git a/lib/tokenize.h b/lib/tokenize.h index dccaa7b4b..732381c6f 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -727,6 +727,13 @@ public: return list.front(); } +private: + /** Disable copy constructor, no implementation */ + Tokenizer(const Tokenizer &); + + /** Disable assignment operator, no implementation */ + Tokenizer &operator=(const Tokenizer &); + /** * Copy tokens. * @param dest destination token where copied tokens will be inserted after @@ -737,13 +744,6 @@ public: */ static Token *copyTokens(Token *dest, const Token *first, const Token *last, bool one_line = true); -private: - /** Disable copy constructor, no implementation */ - Tokenizer(const Tokenizer &); - - /** Disable assignment operator, no implementation */ - Tokenizer &operator=(const Tokenizer &); - /** settings */ const Settings * _settings; diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index f1bf73bbb..a2093aad9 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -351,9 +351,6 @@ private: TEST_CASE(enum28); TEST_CASE(enum29); // ticket #3747 (bitwise or value) TEST_CASE(enum30); // ticket #3852 (false positive) - TEST_CASE(enum31); // ticket #3934 (calculation in first item) - TEST_CASE(enum32); // ticket #3998 (access violation) - TEST_CASE(enum33); // ticket #4015 (segmentation fault) // remove "std::" on some standard functions TEST_CASE(removestd); @@ -6773,7 +6770,7 @@ private: "1 + sizeof ( int ) + " "1 + sizeof ( int ) + 100 + " "1 + sizeof ( int ) + 100 + 1 + " - "1 + sizeof ( int ) + 100 + 1 + 1 + " + "1 + sizeof ( int ) + 100 + 2 + " "90 + " "91 ;"; @@ -7099,20 +7096,6 @@ private: TODO_ASSERT_EQUALS("","[test.cpp:12] -> [test.cpp:7]: (style) Variable 'two' hides enumerator with same name\n", errout.str()); } - void enum31() { // #3934 - calculation in first item - const char code[] = "enum { x=2*32, y }; i = y;"; - ASSERT_EQUALS("i = 65 ;", checkSimplifyEnum(code)); - } - - void enum32() { // #3998 - wrong enum simplification => access violation - const char code[] = "enum { x=(32), y=x, z }; { a, z }"; - ASSERT_EQUALS("{ a , ( 32 ) + 1 }", checkSimplifyEnum(code)); - } - - void enum33() { // #4015 - segmentation fault - const char code[] = "enum { A=SOME_VALUE, B=A };"; - ASSERT_EQUALS(";", checkSimplifyEnum(code)); - } void removestd() { ASSERT_EQUALS("; strcpy ( a , b ) ;", tok("; std::strcpy(a,b);"));