Can changing a function which used to return a const ref to an std::string to return by value break calling code?

Question

We have a function like so:

const std::string& ConvertToString(Enum e)
{
    static const std::string enumStrings[4] =
    {
        std::string("first"),
        std::string("second"),
        std::string("third"),
        std::string("unknown")
    }

    int enumOrd = static_cast<int>(e);
    if (e < 0 || e > 2)
    {
        return enumStrings[3];
    }

    return enumStrings[enumOrd];
}

Now, the problem is that this function is being called after main() returns, and it's crashing since that static array has already been destroyed. I want to change it to be like so:

std::string ConvertToString(Enum e)
{
    switch (static_cast<int>(e))
    {
        case 0: return std::string("first");
        case 1: return std::string("second");
        case 2: return std::string("third");
        default: return std::string("unknown");
    }
}

I wondering if it's possible for this change to break any code which calls this function. I can't think of any problems (as long as the class being returned didn't do sneaky things in const methods, but std::string should be fine, especially in the transition of const ref -> value, but maybe I'm missing something.

Answer 1

One way this could go wrong is if the calling code does something like:

const char* s = ConvertToString(first).c_str();

And then stores s somewhere and accesses it later, thinking that the std::string object referenced by the return value of ConvertToString will never be destroyed.

Of course, that calling code sucks anyway, in that case.

Answer 2

You can break code that depends on different calls to ConvertToString yielding the same object:

for (std::string::const_iterator it = ConvertToString(first).begin();
     it != ConvertToString(first).end(); ++it) {
   // undefined behavior: cannot compare iterators referring 
   // to two different containers/strings
}

One other side note, your implementation might not do what you expect. In particular if your enumeration is defined as enum Enum { first, second, third }; the code:

if (e < 0 || e > 2) {
   return enumStrings[3];
}

can be completely removed by an optimizing compiler and unknown might never be returned by the function. The reason is that the Enum enumeration cannot legally hold any value that is not the bitmask of the enumerators (in this case: 0, 1, 2, 3). Doing so is undefined behavior. The compiler can use this knowledge to optimize the code and remove the e < 0 and/or convert the test condition into a single e == 3.