1

With the following code, "hello2" is not displayed as the temporary string created on Line 3 dies before Line 4 is executed. Using a #define as on Line 1 avoids this issue, but is there a way to avoid this issue without using #define? (C++11 code is okay)

#include <iostream>
#include <string>

class C
{
public:
  C(const std::string& p_s) : s(p_s) {}
  const std::string& s;
};

int main()
{
  #define x1 C(std::string("hello1")) // Line 1
  std::cout << x1.s << std::endl; // Line 2

  const C& x2 = C(std::string("hello2")); // Line 3
  std::cout << x2.s << std::endl; // Line 4
}

Clarification:

Note that I believe Boost uBLAS stores references, this is why I don't want to store a copy. If you suggest that I store by value, please explain why Boost uBLAS is wrong and storing by value will not affect performance.

Clinton
  • 22,361
  • 15
  • 67
  • 163

4 Answers4

5

Expression templates that do store by reference typically do so for performance, but with the caveat they only be used as temporaries

Taken from the documentation of Boost.Proto (which can be used to create expression templates):

Note An astute reader will notice that the object y defined above will be left holding a dangling reference to a temporary int. In the sorts of high-performance applications Proto addresses, it is typical to build and evaluate an expression tree before any temporary objects go out of scope, so this dangling reference situation often doesn't arise, but it is certainly something to be aware of. Proto provides utilities for deep-copying expression trees so they can be passed around as value types without concern for dangling references.

In your initial example this means that you should do:

std::cout << C(std::string("hello2")).s << std::endl;

That way the C temporary never outlives the std::string temporary. Alternatively you could make s a non reference member as others pointed out.

Since you mention C++11, in the future I expect expression trees to store by value, using move semantics to avoid expensive copying and wrappers like std::reference_wrapper to still give the option of storing by reference. This would play nicely with auto.

A possible C++11 version of your code:

class C
{
public:
    explicit
    C(std::string const& s_): s { s_ } {}

    explicit
    C(std::string&& s_): s { std::move(s_) } {}

    std::string const&
    get() const& // notice lvalue *this
    { return s; }

    std::string
    get() && // notice rvalue *this
    { return std::move(s); }

private:
    std::string s; // not const to enable moving
};

This would mean that code like C("hello").get() would only allocate memory once, but still play nice with

std::string clvalue("hello");
auto c = C(clvalue);
std::cout << c.get() << '\n'; // no problem here
Luc Danton
  • 34,649
  • 6
  • 70
  • 114
  • Thanks for the answer. However, even with std::move, is storing by value a performance issue? The expression objects may become quite large when combined, particularly if they're std::array's for example, which I can't see std::move helping with (you still have to do a copy in this case, as it's not a heap allocated pointer). – Clinton May 03 '11 at 04:51
  • There's no all-in-one solution I'm afraid. The above allows for both using expressions as temporary and storing intermediate results, but does rely on the `s` member being of an acceptable size and on move-construction being cheap enough (`std::move` doesn't do magic after all). Note however that with real expression templates usually care can be taken to 'do the right thing': using an `std::decay`-like traits facility to store a `T*` instead of a `T[n]` for instance. This could be applied for `std::array` too. Personally I'd rely on the default move constructor of `std::array`. – Luc Danton May 03 '11 at 05:00
4

but is there a way to avoid this issue without using #define?

Yes.

Define your class as: (don't store the reference)

class C
{
public:
  C(const std::string & p_s) : s(p_s) {}
  const std::string s; //store the copy!
};

Store the copy!

Demo : http://www.ideone.com/GpSa2


The problem with your code is that std::string("hello2") creates a temporary, and it remains alive as long as you're in the constructor of C, and after that the temporary is destroyed but your object x2.s stills points to it (the dead object).

Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • Nawaz: Sorry I wasn't clear in the initial question. See the current question clarification. – Clinton May 03 '11 at 03:13
  • @Clinton: Without creating a copy, you cannot do that, if you want to pass a temporary string. – Nawaz May 03 '11 at 03:17
0

After your edit:

Storing by reference is dangerous and error prone sometimes. You should do it only when you are 100% sure that the variable reference will never go out of scope until its death.

C++ string is very optimized. Until you change a string value, all will refer to the same string only. To test it, you can overload operator new (size_t) and put a debug statement. For multiple copies of same string, you will see that the memory allocation will happen only once.

You class definition should not be storing by reference, but by value as,

class C {
  const std::string s;  // s is NOT a reference now
};

If this question is meant for general sense (not specific to string) then the best way is to use dynamic allocation.

class C {
  MyClass *p;
  C() : p (new MyClass()) {}  // just an example, can be allocated from outside also
 ~C() { delete p; }
};
iammilind
  • 68,093
  • 33
  • 169
  • 336
  • @Nawaz, no I missed the reference part. have edited it. thanks – iammilind May 03 '11 at 03:10
  • iammilind: std::string is an example. It could well be "big and scary class which sizeof() is 1000, composed of lots of other big and scary classes". If I really should store by value, why does Boost uBLAS store by reference? Is Boost doing the wrong thing there? – Clinton May 03 '11 at 03:27
  • @Clinton, In such case, you can simply `new` it up and store a pointer inside the class. Inside destructor you can `delete` it. I have updated the answer. – iammilind May 03 '11 at 03:31
  • @iammilind: Dynamic allocation for expression templates? Isn't that going to be ridiculously slow? – Clinton May 03 '11 at 03:34
  • 1
    `Until you change a string value, all will refer to the same string only.` <-- Not true in most recent STL implementations. While this kind of implementation (that is, a reference counted string implementation) is allowed by the standard and allows some copy elision to occur, in multi threaded scenarios it actually makes performance worse (because messing with the reference count needs to be done under a lock). For this reason, most STLs no longer do this. – Billy ONeal May 03 '11 at 05:02
0

Without looking at BLAS, expression templates typically make heavy use of temporary objects of types you aren't supposed to even know exists. If Boost is storing references like this within theirs, then they would suffer the same problem you see here. But as long as those temporary objects remain temporary, and the user doesnt store them for later, everything is fine because the temporaries they reference remain alive for as long as the temporary objects do. The trick is you perform a deep copy when the intermediate object is turned into the final object that the user stores. You've skipped this last step here.

In short, it's a dangerous move, which is perfectly safe as long as the user of your library doesn't do anything foolish. I wouldn't recommend making use of it unless you have a clear need, and you're well aware of the consequences. And even then, there might be a better alternative, I've never worked with expression templates in any serious capacity.

As an aside, since you tagged this C++0x, auto x = a + b; seems like it would be one of those "foolish" things users of your code can do to make your optimization dangerous.

Dennis Zickefoose
  • 10,791
  • 3
  • 29
  • 38