C++ Inserting Unique_Ptr in Map

C++ inserting unique_ptr in map

As a first remark, I wouldn't call it ObjectArray if it is a map and not an array.

Anyway, you can insert objects this way:

ObjectArray myMap;
myMap.insert(std::make_pair(0, std::unique_ptr<Class1>(new Class1())));

Or this way:

ObjectArray myMap;
myMap[0] = std::unique_ptr<Class1>(new Class1());

The difference between the two forms is that the former will fail if the key 0 is already present in the map, while the second one will overwrite its value with the new one.

In C++14, you may want to use std::make_unique() instead of constructing the unique_ptr from a new expression. For instance:

myMap[0] = std::make_unique<Class1>();

std::unique_ptr with std::map

return _cache.rbegin()->second.get(); does not do what you want, as std::map does not append elements but sorts them. However emplace returns an iterator to what it just inserted, so you only need:

return _cache.emplace(foo, std::make_unique<Bar>(bar))->first->second.get();

Or even, since you don't actually need to store and copy the Bar, and you can also sacrifice foo:

return _cache.emplace(
    std::move(foo),
    std::make_unique<Bar>(ThirdPartLibrary::CreateBar())
)->first->second.get();

I'd also personally flip the (it == _cache.end()) condition to make it an early return, but that's just a matter of taste.

Otherwise, what you have looks good to me.

Compile error trying to put a unique_ptr into a map

You cannot copy a unique_ptr, because then it will not be unique. You have to move it - AddItem(std::move(someItem)); and myMap.emplace("test", std::move(item));.

c++ map insert fails to compile when inserting an object that contains a unique_ptr

Struct1(Struct1& other)

As per language standard, it's the copy constructor; what you did here (by typo or on purpose, I don't know) is a auto_ptr-style neither-move-nor-copy-semantics. Though with that done, the copy assignment is auto-generated, while the move operations are completely removed (unless defined manually).

Didn't you want that instead?

#include <utility>
#include <string>      
#include <iostream>    
#include <map>
#include <memory>

struct Struct1
{
    Struct1(std::unique_ptr<std::string> pStr)
        : pStr_(std::move(pStr)){}
   ~Struct1(){}
    Struct1(Struct1&& other) //double! ampersand
        : pStr_(std::move(other.pStr_)){}
    std::unique_ptr<std::string> pStr_;
};

int main () {
    std::string s("key");
    std::unique_ptr<std::string> pStr = std::make_unique<std::string>("Hello");

    std::map<std::string, Struct1>list;
    Struct1 ste(std::move(pStr));
    std::pair<std::string, Struct1> p{s,std::move(ste)}; //note the move 
    list.insert(std::move(p)); //and here
}

https://godbolt.org/z/fs44j69ja

C++ unique_ptr and map

I don't believe it's possible to correctly use unique_ptr in a map::insert yet. Here's the GCC bug for it:

Bug 44436 - [C++0x] Implement insert(&&) and emplace* in associative and unordered containers

It looks like it may be fixed for GCC 4.6, but it won't build from SVN on vanilla Ubuntu 10.04.1 to confirm.

Update0

This is not working as of GCC svn r165422 (4.6.0).

How do you add abstract base class unique_ptrs to a map?

The first cause of an error message is that a class type can never be used as the type for a dynamic_cast. The target type of dynamic_cast must always be either a pointer to class type (meaning the result is null if the cast fails) or a reference to class type (meaning to throw an exception if the cast fails).

So improvement #1:

my_map[0] = dynamic_cast<GameObject*>(std::unique_ptr<Enemy>().get());

But this won't work because GameObject is a private base class of Enemy. You probably meant to use public inheritance, but (when using class instead of struct) you must say so:

class Enemy : public GameObject
// ...

Next we'll find that the = within the map statement is invalid. The left side has type std::unique_ptr<GameObject>, which does not have any operator= that can take a GameObject* pointer. But it does have a reset member for setting a raw pointer:

my_map[0].reset(dynamic_cast<GameObject*>(std::unique_ptr<Enemy>().get()));

Now the statement should compile - but it's still wrong.

Before getting to why it's wrong, we can make a simplification. dynamic_cast is needed for getting a pointer to derived class from a pointer to base class, or for many other type changes within a more complicated inheritance tree. But it's not needed at all to get a pointer to base class from a pointer to derived class: this is a valid implicit conversion, since every object with a derived class type must always contain a subobject of the base class type, and there's no "failure" case. So the dynamic_cast here can just be dropped.

my_map[0].reset(std::unique_ptr<Enemy>().get());

The next problem is that std::unique_ptr<Enemy>() creates a null unique_ptr, and no Enemy object is created at all. To create an actual Enemy, we can write instead either std::unique_ptr<Enemy>(new Enemy) or std::make_unique<Enemy>().

my_map[0].reset(std::make_unique<Enemy>().get());

Still wrong, and in a slightly tricky way. Now the problem is that the created Enemy object is owned by the temporary std::unique_ptr<Enemy> object returned by make_unique. The reset tells the std::unique_ptr<GameObject> within the map that it should own a pointer to the same object. But at the end of the statement, the temporary std::unique_ptr<Enemy> gets destroyed, and it destroys the Enemy object. So the map is left with a pointer to a dead object, which is invalid to use - not what you wanted.

But the solution here is that we don't need to mess around with get() and reset() at all. There is an operator= that allows assigning an rvalue std::unique_ptr<Enemy> to a std::unique_ptr<GameObject>, and it does the right thing here. It makes use of the implicit conversion from Enemy* to GameObject*.

my_map[0] = std::make_unique<Enemy>();

(Note if you had a named std::unique_ptr<Enemy>, you would need to std::move it to allow the assignment, as in my_map[0] = std::move(enemy_ptr);. But std::move is not needed above because the result of make_unique is already an rvalue.)

Now this statement is much shorter and more legible, and will actually do what you want.

A comment also suggested the possibility

my_map.emplace(0, std::make_unique<Enemy>());

This is also valid, but there's a possibly important difference: if the map already has an object with key zero, the = version will destroy and replace the old one, but the emplace version will leave the map alone and the just-created Enemy will be destroyed instead.

Insert unique_ptr into map, pointer gets destroyed

I don't think your pointer is getting destroyed. What you are seeing here:

template <class T>
void insertOperand(std::string &s, T o = T()) {
    op.insert(std::pair<std::string, std::unique_ptr<StreamOperand>>(
        s, std::move(std::unique_ptr<T>(new T(o)))
    );
}

is the destruction of o, after it has been used to construct the heap allocated T used in the unique_ptr.

The map being empty is not a symptom of the pointer being destroyed. If this were the case (the pointer being destroyed), you would have an entry in the map for a given key, with an invalid unique_ptr.

why does C++ give me an error when inserting unique pointer into unordered map?

This:

m_Tests.emplace(testName, test);

will try to copy test, because (a) it is an lvalue expression, (b) expressions never have reference type, and (c) emplace does not take an lvalue reference.

unique_ptrs cannot be copied.

This is what std::move is for!

m_Tests.emplace(testName, std::move(test));

Now you have an rvalue expression, which is perfectly suited to bind to the rvalue reference that emplace does take.

I'm actually slightly slightly lying here, and things are a bit more complicated since emplace is variadic. But whatever. The important thing is that you're creating another link in the chain of ownership for this unique_ptr, ultimately passing ownership on to the map, and std::move is the way to do that.

---

Related Topics