Global const string& smells bad to me, is it truly safe?

Question

I'm reviewing a collegue's code, and I see he has several constants defined in the global scope as:

const string& SomeConstant = "This is some constant text";

Personally, this smells bad to me because the reference is referring to what I'm assuming is an "anonymous" object constructed from the given char array.

Syntactically, it's legal (at least in VC++ 7), and it seems to run, but really I'd rather have him remove the & so there's no ambiguity as to what it's doing.

So, is this TRULY safe and legal and I'm obsessing? Does the temp object being constructed have a guaranteed lifetime? I had always assumed anonymous objects used in this manner were destructed after use...

---

So my question could also be generalized to anonymous object lifetime. Does the standard dictate the lifetime of an anonymous object? Would it have the same lifetime as any other object in that same scope? Or is it only given the lifetime of the expression?

---

Also, when doing it as a local, it's obviously scoped differently:

class A
{
    string _str;

public:
    A(const string& str) :
        _str(str)
    {
     cout << "Constructing A(" << _str << ")" << endl;
    }

    ~A()
    {
     cout << "Destructing A(" << _str << ")" << endl;
    }
};

void TestFun()
{
    A("Outer");
    cout << "Hi" << endl;
}

Shows:

Constructing A(Outer); Destructing A(Outer); Hi

Answer 1

It's completely legal. It will not be destructed until the program ends.

EDIT: Yes, it's guaranteed:

"All objects which do not have dynamic storage duration, do not have thread storage duration, and are not local have static storage duration. The storage for these objects shall last for the duration of the program (3.6.2, 3.6.3)."

-- 2008 Working Draft, Standard for Programming Language C++, § 3.7.1 p. 63

As Martin noted, this is not the whole answer. The standard draft further notes (§ 12.2, p. 250-1):

"Temporaries of class type are created in various contexts: binding an rvalue to a reference (8.5.3) [...] Even when the creation of the temporary object is avoided (12.8), all the semantic restrictions shall be respected as if the temporary object had been created. [...] Temporary objects are destroyed as the last step in evaluating the full-expression (1.9) that (lexically) contains the point where they were created. [...] There are two contexts in which temporaries are destroyed at a different point than the end of the full-expression. [...] The second context is when a reference is bound to a temporary. The temporary to which the reference is bound or the temporary that is the complete object of a subobject to which the reference is bound persists for the lifetime of the reference except as specified below."

I tested in g++ if that makes you feel any better. ;)

Answer 2

It might be legal, but still ugly. Leave out the reference !

const string SomeConstant = "This is some constant text";

Answer 3

It's legal to extend a temporary variable with a const reference, this is used by Alexandrescu's ScopeGaurd see this excellent explanation by Herb Sutter called A candidate for the "Most important const".

That being said this specific case is an abuse of this feature of C++ and the reference should be removed leaving a plain const string.

Answer 4

It's as legal as it's ugly.

Answer 5

Yes it is valid and legal.

const string& SomeConstant = "This is some constant text";

// Is equivalent too:

const string& SomeConstant = std::string("This is some constant text");

Thus you are creating a temporary object.
This temporary object is bound to a const& and thus has its lifetime extended to the lifespan of the variable it is bound too (ie longer than the expression in which it was created).

This is guranteed by the standard.

Note:

Though it is legal. I would not use it. The easist solution would be to convert it into a const std::string.

Usage:

In this situation because the variable is in global scope it is valid for the full length of the program. So it can be used as soon as execution enters main() and should not be accessed after executiuon exits main().

Though it technically may be avilable before this your usage of it in constructors/destructors of global objects should be tempered with the known problem of global variable initialization order.

Extra Thoughts:

This on the other hand will not suffer from the problem:

char const* SomeConstant = "This is some constant text";

And can be used at any point. Just a thought.

Answer 6

By declaring it as const (which means it can't be changed) and then making it a reference, which implies that someone might change it, seems like bad form, at the very least. Plus, as I am sure you understand, global variables are BAD, and rarely necessary.

Answer 7

Okay, folks correct me if I'm off the deep end, but here's my conclusions listening to all of your excellent responses:

A) it is syntactically and logically legal, the & extends the lifetime of the temp/anonymous from beyond expression level to the life of the reference. I verified this in VC++7 with:

class A { 
    public: A() { cout << "constructing A" << endl; }
    public: ~A() { cout << "destructing A" << endl; }
};

void Foo()
{
    A();
    cout << "Foo" << endl;
}

void Bar()
{
    const A& someA = A();
    cout << "Bar" << endl;
}

int main()
{
    Foo();    // outputs constructing A, destructing A, Foo
    Bar();    // outputs constructing A, Bar, destructing A
    return 0;
}

B) Though it is legal, it can lead to some confusion as to the actual lifetime and the reference in these cases give you no benefit of declaring it as a non-reference, thus the reference should probably be avoided and may even be extra space. Since there's no benefit to it, it's unnecessary obfuscation.

Thanks for all the answers it was a very interesting dicussion. So the long and short of it: Yes, it's syntactically legal, no it's not technically dangerous as the lifetime is extended, but it adds nothing and may add cost and confusion, so why bother.

Sound right?