Tokenizer::simplifyEnum: Reverted refactorings/optimisations as there were regressions (#3949, #3950, #4025)

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<std::string, EnumValue> &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<std::string,EnumValue> 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<MathLib::bigint>(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 (enumValueEnd->str() == "(" ||
-                        if (Token::Match(enumValueEnd, "(|["))
+                        enumValueEnd->str() == "[" ||
+                        enumValueEnd->str() == "{")
                         ++level;
-                        else if (Token::Match(enumValueEnd->next(), "]|)"))
+                    while (enumValueEnd->next() &&
+                           (!Token::Match(enumValueEnd->next(), "}|,") || level)) {
+                        if (enumValueEnd->next()->str() == "(" ||
+                            enumValueEnd->next()->str() == "[" ||
+                            enumValueEnd->next()->str() == "{")
+                            ++level;
+                        else if (enumValueEnd->next()->str() == ")" ||
+                                 enumValueEnd->next()->str() == "]" ||
+                                 enumValueEnd->next()->str() == "}")
                             --level;
 
                         enumValueEnd = enumValueEnd->next();
@@ -7109,39 +7042,29 @@ 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;
-                }
-            }
-
-            // Substitute enum values
-            {
                     const std::string pattern = className.empty() ?
                                                 std::string("") :
-                                            std::string(className + " :: ");
+                                                std::string(className + " :: " + enumName->str());
                     int level = 1;
                     bool inScope = true;
 
-                std::stack<std::set<std::string> > shadowId;  // duplicate ids in inner scope
+                    bool exitThisScope = false;
+                    int exitScope = 0;
                     bool simplify = false;
                     bool hasClass = false;
-                EnumValue *ev = NULL;
+                    const Token *endScope = 0;
                     for (Token *tok2 = tok1->next(); tok2; tok2 = tok2->next()) {
                         if (tok2->str() == "}") {
                             --level;
                             if (level < 0)
                                 inScope = false;
 
-                        if (!shadowId.empty())
+                            if (exitThisScope) {
-                            shadowId.pop();
+                                if (level < exitScope)
+                                    exitThisScope = false;
+                            }
                         } else if (tok2->str() == "{") {
                             // Is the same enum redefined?
                             const Token *begin = end->link();
@@ -7159,20 +7082,13 @@ void Tokenizer::simplifyEnum()
                             } 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()) {
+                            endScope = tok2->link();
+                        } else if (!pattern.empty() && Token::Match(tok2, pattern.c_str())) {
                             simplify = true;
                             hasClass = true;
-                        ev = &(enumValues.find(tok2->strAt(2))->second);
+                        } else if (inScope && !exitThisScope && tok2->str() == enumName->str()) {
-                    } else if (inScope &&    // enum is in scope
+                            if (!duplicateDefinition(&tok2, enumName)) {
-                               (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:
@@ -7181,23 +7097,48 @@ void Tokenizer::simplifyEnum()
                                 } else {
                                     simplify = true;
                                     hasClass = false;
-                                ev = &(enumValues.find(tok2->str())->second);
                                 }
                             } else {
                                 // something with the same name.
-                            if (shadowId.empty())
+                                exitScope = level;
-                                shadowId.push(std::set<std::string>());
+                                if (endScope)
-                            shadowId.top().insert(tok2->str());
+                                    tok2 = endScope->previous();
                             }
                         }
 
                         if (simplify) {
-                        if (ev->value)
+                            // Simplify calculations..
-                            tok2->str(ev->value->str());
+                            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;
+                                }
+                            }
+
+
+                            if (enumValue)
+                                tok2->str(enumValue->str());
                             else {
                                 tok2 = tok2->previous();
                                 tok2->deleteNext();
-                            tok2 = copyTokens(tok2, ev->start, ev->end);
+                                tok2 = copyTokens(tok2, enumValueStart, enumValueEnd);
                             }
 
                             if (hasClass) {
@@ -7208,6 +7149,7 @@ void Tokenizer::simplifyEnum()
                         }
                     }
                 }
+            }
 
             // check for a variable definition: enum {} x;
             if (end->next() && end->next()->str() != ";") {

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;
 

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