Search code examples
c++goto

Goto before variable initialization causes compiler error


Consider this code (VS2008):

void WordManager::formatWords(std::string const& document)
{
    document_ = document;
    unsigned int currentLineNo = 1;

    size_t oldEndOfLine = 0;
    size_t endOfLine    = document_.find('\n');
    while(endOfLine != std::string::npos)
    {
        std::string line = document_.substr(oldEndOfLine, (endOfLine - oldEndOfLine));
        if(line.size() < 2)
        {
            oldEndOfLine = endOfLine + 1;
            endOfLine    = document_.find('\n', oldEndOfLine);
            continue;
        }

        std::vector<std::string> words = Utility::split(line);
        for(unsigned int i(0); i < words.size(); ++i)
        {
            if(words[i].size() < 2)
                continue;
            Utility::trim(words[i], WordManager::delims);
            Utility::normalize(words[i], WordManager::replace, WordManager::replaceWith);

            if(ruleOne(words[i]) && ruleTwo(words[i]))
            {
                std::set<Word>::iterator sWIter(words_.find(Word(words[i])));

                if(sWIter == words_.end())
                    words_.insert(Word(words[i])).first->addLineNo(currentLineNo);
                else
                    sWIter->addLineNo(currentLineNo);
            }
        }
        ++currentLineNo;

        oldEndOfLine = endOfLine + 1;
        endOfLine    = document_.find('\n', oldEndOfLine);
    }
}

If it's important: this is code from a homework assignment used to filter out and modify words in a document. document holds the document (previously read from file)

I wish to introduce a malicious goto because I think it's actually cleaner in this case like so:

void WordManager::formatWords(std::string const& document)
{
    document_ = document;
    unsigned int currentLineNo = 1;

    size_t oldEndOfLine = 0;
    size_t endOfLine    = document_.find('\n');
    while(endOfLine != std::string::npos)
    {
        std::string line = document_.substr(oldEndOfLine, (endOfLine - oldEndOfLine));
        // HERE!!!!!!
        if(line.size() < 2)
            goto SkipAndRestart;

        std::vector<std::string> words = Utility::split(line);
        for(unsigned int i(0); i < words.size(); ++i)
        {
            if(words[i].size() < 2)
                continue;
            Utility::trim(words[i], WordManager::delims);
            Utility::normalize(words[i], WordManager::replace, WordManager::replaceWith);

            if(ruleOne(words[i]) && ruleTwo(words[i]))
            {
                std::set<Word>::iterator sWIter(words_.find(Word(words[i])));

                if(sWIter == words_.end())
                    words_.insert(Word(words[i])).first->addLineNo(currentLineNo);
                else
                    sWIter->addLineNo(currentLineNo);
            }
        }

SkipAndRestart:
        ++currentLineNo;

        oldEndOfLine = endOfLine + 1;
        endOfLine    = document_.find('\n', oldEndOfLine);
    }
}

Whether or not this is a good design choice is irrelevant at this moment. The compiler complains error C2362: initialization of 'words' is skipped by 'goto SkipAndRestart'

I don't understand this error. Why is it important, and an error, that the words initialization is skipped? That's exactly what I want to happen, I don't want it to do more work, just restart the bloody loop. Doesn't the continue macro do more or less the exact same thing?


Solution

  • You are skipping over the construction of the words array:

        if(line.size() < 2)
            goto SkipAndRestart;
        std::vector<std::string> words = Utility::split(line);
        // ...
    SkipAndRestart:
    

    You could have used words after the SkipAndRestart: label, which would have been a problem. You don't use it in your case, but the words variable won't be destructed until the end of the scope in which it is introduced, so as far as the compiler is concerned, the variable is still in use at the point of the label.

    You can avoid this by putting words inside its own scope:

        if(line.size() < 2)
            goto SkipAndRestart;
        {
            std::vector<std::string> words = Utility::split(line);
            // ...
        }
    SkipAndRestart:
    

    Note that the continue statement jumps to the end of the loop, at a point where you actually can't put a label. This is a point after the destruction of any local variables inside the loop, but before the jump back up to the top of the loop.