Proper way to emplace_back object into std::list

Question

I have a class with a variety of members (largely ints, floats and some dynamic containers). I have a std::list myclassobjects. In the constructor of this class after setting all of the parameters I call

myclassobjects.emplace_back(*this)

This works and doesn't appear to cause any issues with allocation; I never have sigsegv issues or similar. However I'm not completely convinced this is the right thing to do, or that it doesn't cause a slow burn memory leak.

Is this method safe to use or does it cause a memory leak? If yes, what's the 'proper' way to go about constructing an object and immediately putting it into a std::list?

What you're doing is no different than `myclassobjects.push_back(*this)`. I.e. it creates a *copy* of the current object and appends it to the end of the list. — Some programmer dude, Sep 28 '17 at 06:57
typically if you need to keep a set of all constructed objects, you'd store *pointers* to them, or... you'd have a factory/singleton that would make and store them and return a pointer or reference to the created object — kmdreko, Sep 28 '17 at 07:13

patatahooligan · Accepted Answer · 2017-09-29T12:36:17.820

First of all emplace_back constructs an object in place by forwarding its arguments. So what you end up doing is calling your class's copy constructor with the argument *this.

I'm trying to think of a case where adding objects to a list inside the constructor is needed and I'm having a hard time. I think there are better alternatives.

Emplace it in the list from outside the constructor

Simply create it as follows.

myclassobjects.emplace_back(/*constructor params*/);

In C++17 this even returns a reference to the newly created object

MyClass& myclass_ref = myclassobjects.emplace_back(/*constructor params*/);
do_something_with_my_class_object(myclass_ref);

This is the cleanest and most efficient way to do it. It has the added benefit that you can create local objects without adding them to the list if needed.

Use a factory method that adds it to the list

If you absolutely must have every object in the list and you don't want it to be a copy, use a static factory method. If the list and the callee must share ownership, you can use a list of std::shared_ptrs and do the following.

MyClass {
private:
    MyClass();    // private constructor forbids on the stack variables

public:
    static std::shared_ptr<MyClass> create_instance() {
        auto ptr = make_shared<MyClass>();    // We can access the constructor here
        myclass_objects.emplace_back(ptr);
        return ptr;
    }
}

On the other hand, if your list is guaranteed to outlive the callee's handler on the object, it is more appropriate to have a list of std::unique_ptrs that hold the objects and return references to them.

MyClass {
private:
    MyClass();    // private constructor forbids on the stack variables

public:
    // pre C++17
    static MyClass& create_instance() {
        auto ptr = make_unique<MyClass>();
        auto& ref = *ptr;    // Store ref before we move pointer
        myclass_objects.emplace_back(std::move(ptr));
        return ref;
    }

    // C++17
    static MyClass& create_instance() {
        auto ptr = make_unique<MyClass>();
        return *(myclass_objects.emplace_back(std::move(ptr)));
    }
}

Of course, these examples only cover default constructors. Because you will usually need to work with more constructors, you will probably need a templated create_instance that forwards its arguments to the constructor.

For the first example of calling `emplace_back()`, why did you construct the object in place instead of passing the constructor parameters directly? — Andy, Sep 28 '17 at 18:57
If I don't do it this way, I'll create a local object and push a copy of it in the list. The class might offer a move constructor to make things cheaper, but I've still created an extra object for no reason. With `emplace_back`, the list allocates the space and creates the object directly in this space by forwarding the arguments. — patatahooligan, Sep 28 '17 at 19:01
Right, it makes sense to use `emplace_back()` instead of `push_back`(), but why did you call `emplace_back()` the way you did instead of calling it like this: `emplace_back(/* constructor params*/)`? — Andy, Sep 29 '17 at 12:33
Ok, that makes sense. Just wanted to make sure I wasn't missing something. :) — Andy, Sep 29 '17 at 12:35
Thanks @patatahooligan, this gave me sufficient knowledge and what seems to be the 'proper' way to do this. I went with list.emplace_back(constructor parameters). — nathanburns, Oct 04 '17 at 08:58

score 0 · Answer 2 · answered Sep 28 '17 at 07:14

It is impossible to store your object in std::list or any other container indeed. An object is a region of storage, by the standard definition of object. A region of storage cannot be stored, it just is.

Suppose X is an existing object. A container cannot store X itself, but a value related to X in some way. There are two options.

Store a copy of X.
Store a (smart) pointer or a reference wrapper to X.

Since you are emplacing *this, you are storing a copy. This may or may not be what you need.

To store a naked pointer, you would have to emplace_back(this) and of course change the type of the list. This is dangerous, because when X ceases to exist, your list may still have a dangling pointer to it. You need to make sure it doesn't happen.

You may want to consider storing shared pointers, but if you want to enable it by inheriting enable_shared_from_this, be aware that shared_from_this is a dangerous thing to use in a constructor.

Proper way to emplace_back object into std::list

2 Answers2

Emplace it in the list from outside the constructor

Use a factory method that adds it to the list