Deleted Function in Std::Pair When Using a Unique_Ptr Inside a Map

Deleted Function in std::pair when using a unique_ptr inside a map

The problem originates from the following post-condition of std::vector<T>::resize, [vector.capacity]:

Remarks: If an exception is thrown other than by the move constructor of a non-CopyInsertable T there are no effects.

That is, a vector must remain unchanged if relocation fails. One of the reasons relocation may fail is due to an exception, specifically, when a copy or move constructor, used for shifting elements from an old storage to a new one, throws an exception.

Does copying elements change the original storage in any way? No¹. Does moving elements change the original storage? Yes. Which operation is more efficient? Moving. Can a vector always prefer moving to copying? Not always.

If a move constructor can throw an exception, there's no possibility to restore the original content of the old storage, because an attempt to move the already shifted elements back to the old chunk may fail again. In such a case, a vector will use a move constructor to relocate its elements from the old storage to a new one only if that move constructor guarantees it will not throw an exception (or a move constructor is the only option when a copy constructor is not available). How does a function promise it will not throw an exception? One will be annotated with the noexcept specifier and tested with the noexcept operator.

Testing the below code with icc:

std::map<int, std::unique_ptr<int>> m;
static_assert(noexcept(std::map<int, std::unique_ptr<int>>(std::move(m))), "!");

fails on the assertion. This means that m is not nothrow-MoveConstructible.

Does the standard require it to be noexcept? [map.overview]:

// [map.cons], construct/copy/destroy:
map(const map& x);
map(map&& x);

std::map is both Move- and CopyConstructible. Neither is required not to throw an exception.

However, an implementation is allowed to provide this guarantee ^{{{citation needed}}}. Your code uses the following definition:

map(map&&) = default;

Is an implicitly generated move constructor required to be noexcept? [except.spec]:

An inheriting constructor ([class.inhctor]) and an implicitly declared special member function (Clause [special]) have an exception-specification. If f is an inheriting constructor or an implicitly declared default constructor, copy constructor, move constructor, destructor, copy assignment operator, or move assignment operator, its implicit exception-specification specifies the type-id T if and only if T is allowed by the exception-specification of a function directly invoked by f's implicit definition; f allows all exceptions if any function it directly invokes allows all exceptions, and f has the exception-specification noexcept(true) if every function it directly invokes allows no exceptions.

At this point, it's hard to say whether the implicitly generated by icc move constructor should be noexcept or not. Either way, std::map itself was not required to be nothrow-MoveConstructible, so it's more a quality of implementation issue (implementation of the library or implementation of implicit generation of constructors) and icc gets away with it regardless of this being an actual bug or not.

Eventually, std::vector will fall back to using the safer option which is a copy constructor to relocate its elements (maps of unique pointers), but since std::unique_ptr is not CopyConstructible, an error is reported.

On the other hand, std::unique_ptr's move constructor is required to be noexcept, [unique.ptr.single.ctor]:

unique_ptr(unique_ptr&& u) noexcept;

A vector of unique pointers can safely move its elements when relocation is required.

---

In a newer version of stl_map.h there's the following user-provided definition of map's move constructor:

map(map&& __x)
  noexcept(is_nothrow_copy_constructible<_Compare>::value)
  : _M_t(std::move(__x._M_t)) { }

which explicitly makes noexcept dependent only on whether or not copying a comparator throws.

---

_{¹ Technically, a copy constructor accepting a non-const l-value reference can change the original object, e.g., std::auto_ptr, but MoveInsertable requires vector elements to be constructible from r-values, that cannot bind to non-const l-value references.}

C++ std::unique_ptr stored inside std::map use of deleted function ill formed

You can't do much about it: elements in an initializer list are copied. This will not get along with classes that are move-only.

There's a way to bypass this "defect" but it isn't very nice to read; you decide

using map_type  = std::map< std::string, std::unique_ptr< A > >;
using pair_type = map_type::value_type;
pair_type elements[] = { { "A", std::make_unique< A >() }, { "B", std::make_unique< A >() } };

map_type myMap { std::make_move_iterator( begin(elements) ), std::make_move_iterator( end(elements) ) };

which will make myMap iterate over the range and move the elements within, rather than copying. Method kindly taken from this other question.

c++ attempting to reference deleted function

There are two errors in your for_each code. First, map::value_type is pair<const Key, Value>. Second, your lambda expression is taking the argument by value, which means it attempts to copy the unique_ptr, hence the error. To fix it, take the argument by reference.

[&](std::pair<const std::size_t, std::unique_ptr<Foo>>& data_pair)
//            ^^^^^                                  ^^^
{
  m_list2[data_pair.first] = std::unique_ptr<Foo>(std::move(data_pair.second));
}

---

A better option is to not mention those types explicitly, instead use decltype

[&](decltype(m_list1)::value_type& data_pair)
{
  m_list2[data_pair.first] = std::unique_ptr<Foo>(std::move(data_pair.second));
}

---

Now, your range-based for worked because you were binding the elements of the map to a reference by using for(auto& data_pair : m_list1). You'd have run into the same error as before if you'd instead used for(auto data_pair : m_list1) because that'd have attempted to make a copy of the elements.

std::unique_ptr attempting to reference a deleted function

A std::unique_ptr cannot be copied. Even if the managed object could (but yours can't).

You have a couple of alternatives here (all with different effects):

• Change the type of keep to a non-owning raw pointer (aka A *) and use keep = object.get();. This is safe if and only if you know you won't use keep longer than ObjectList (or, more precisely, the object you take the address of) exists.

• Move the std::unique_ptr out of the container, that is, use keep = std::move(object);. Of course, now you have a gap in ObjectList. (I realize you have edited your question to say that ObjectList is const which means that you cannot modify and hence not move objects out of it.)

• Change the type of ObjPtr to std::shared_ptr<A> if you want shared ownership semantics.

• If you absolutely want a copy of the object, you could add a virtual member function to A that clones the object polymorphically.

  class A
  {
  public:
    virtual std::unique_ptr<A> clone() = 0;
    …
  };
  

  You then implement that function in every leaf class derived from A. In your loop, you then use keep = object->clone();. For this, you probably want to make the copy constructor protected but don't delete it.

  Don't use keep = std::make_unique<A>(*object); because it would not respect the actual (dynamic) type of the object and always slice to an A. (Since your A is not copyable, it wouldn't work anyway.)

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));.

Class with std::map object and deleted copy assignment and constructor causes C2280 with std::pair's copy constructor

An error with the std pair constructor is caused by a map operation that invokes the copy constructor, such as std::map::insert. A call to insert won't work, because one cannot copy a unique_ptr, so the compiler will complain about a deleted constructor. Instead, you will have to std::move the unique_ptr into the map when you call insert, or construct the pointer in place using std::map::emplace.

Hope that helps - if not, a small reproducible example is the way to go as others have said, I'm mostly guessing :)

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

Storing a unique_ptr with custom deleter in a map

The problem is that m[0] calls the default constructor of std::unique_ptr<int, decltype(&deleter)>, which is not available because it requires the deleter pointer.

Fix:

struct Deleter {
    void operator()(int* i) { delete i; }
};

std::map<int, std::unique_ptr<int, Deleter>> m;

void foo(int* i) {
    m[0] = std::unique_ptr<int, Deleter>(i);
}

This is also more efficient than std::unique_ptr<int, decltype(&deleter)> because it does not have to store the very same pointer to deleter in each instance of std::unique_ptr. I.e. sizeof(std::unique_ptr<int, Deleter>) < sizeof(std::unique_ptr<int, decltype(&deleter)>).

Using unique_ptr in a pair

This is because you don't move anything in your function. They recieve const T&, but constants cannot be moved. Calling std::move on a const won't move and does absolutely nothing.

See, move is simply a cast to rvalue. The actual moving happen in the move constructor and move assignment. Such move constructors are declared like this:

unique_ptr(unique_ptr&& other);

As you can see, it's a non const rvalue reference. Since you cannot call the move constructor, it tries to copy instead. That is the source of the error.

How can you fix it then?

Simply add the required overloads and remove the superfluous moves:

template <typename T>
class AtomicQueue {
    std::mutex m;
    std::queue<T> q;
    T value_if_empty;

public:
    // copy, it's an lvalue
    AtomicQueue(const T& emptyval) : value_if_empty(emptyval) {};

    // move, it's an rvalue
    AtomicQueue(T&& emptyval) : value_if_empty(std::move(emptyval)) {};

    void push(const T& t)
    {
        m.lock();
        q.push(t); // same here
        m.unlock();
    }

    void push(T&& t)
    {
        m.lock();
        q.push(std::move(t)); // same here
        m.unlock();
    }
    T pop()
    {
        T a = std::move(value_if_empty);
        m.lock();
        if (!q.empty())
        {
            a = std::move(q.front());
            q.pop();
        }
        m.unlock();
        return a;
    }
};

Live example

---

Also note that there's a fundamental bug in your class. Look at this line:

T a = std::move(value_if_empty);

If pop is called more than once, value_if_empty will be a moved from value and you return it. Your pop function can only be called one time before returning unspecified values.

You'll need either to copy value_if_empty, default construct it or replace it by a factory function that will return a new value to take if empty.

My favourite solution would be to default construct it.

Here's nonetheless an example with a factory function:

template <typename T>
class AtomicQueue {
    std::function<T()> make_empty_value;

public:
    T pop()
    {
        T a = make_empty_value();
        // ...
    }
};

Then pass it to your class when creating it:

AtomicQueue<std::unique_ptr<Class1>> B([]{ return std::unique_ptr<Class1>{nullptr}; });

If you want to avoid the overhead of std::function, you can replace the member variable by a template parameter of the lambda type.

---

Related Topics