nathanburns
nathanburns

Reputation: 139

Proper way to emplace_back object into std::list

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?

Upvotes: 0

Views: 2398

Answers (2)

patatahooligan
patatahooligan

Reputation: 3321

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.

Upvotes: 2

n. m. could be an AI
n. m. could be an AI

Reputation: 119877

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.

  1. Store a copy of X.
  2. 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.

Upvotes: 0

Related Questions