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

user_8272263 0
Viewed 71

I am having trouble (compile time, gcc 17) inserting an object in an std::map which contains a unique_ptr. If I use a regular pointer it compiles (if I take the copy constructor out!). Here is the struct containing a unique_ptr. With a ctor, copy ctor, etc, with std::move semantics everywhere. Has anyone seen this issue before?

#include <utility>      // std::pair, std::make_pair
#include <string>       // std::string
#include <iostream>     // std::cout
#include <map>
#include <memory>

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

Here is the main. Anything I try I get a "no matching function" for insert

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,ste); // copy into a pair
    // std::pair<std::string, Struct1> p(s,std::move(ste)); // copy into a pair
    // auto p = std::make_pair(s, std::move(ste));
    // auto p = std::make_pair(s, ste);
    // list.insert(std::move(p));
    list.insert(p);
stdmap c++
2 Answers
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

user_48853210

When you have opt into structs, you can avoid creating your own constructor unless it's really needed. Instead, Struct1 could simply be:

struct Struct1
{
    std::unique_ptr<std::string> pStr_;
};

All the copy/move constructors and assignment operators will be generated automatically if possible. However, since you have a unique_ptr in the struct, who does not have a copy constructor or assignment operator, those will not be generated for you, which you don't need them in your case anyways.

Now the rest will be pretty simply, as long you remember to call move on your objects properly:

std::string s("key");
std::map<std::string, Struct1> list;

// prefer using `auto` when the type can already be deduced from `make_unique`
auto pStr = std::make_unique<std::string>("Hello");

// You must move `pStr` to construct `ste` since `pStr` is a `unique_ptr`.
// Also note you are using bracket `{}`, instead of parentheses `()` to create `Struct1`
Struct1 ste{std::move(pStr)};
// With C++20, parentheses is also allowed:
Struct1 ste(std::move(pStr));

// Similarly, you must move `ste` since it contents a `unique_ptr` in it
// Also note the type of the pair, the Key must be `const`
std::pair<const std::string, Struct1> p(s, std::move(ste));

// Once again, you must move `p` for the same reason
list.insert(std::move(p));

Moreover, there are two steps that can be shortened:

std::string s("key");
std::map<std::string, Struct1> list;

// You can create `ste` directly without creating `pStr` first.
// This will also reduce the chance for accidentally using `pStr` again later.
Struct1 ste{std::make_unique<std::string>("Hello")};

// You can then simply emplace `ste` into `list`, or use the subscript operator`[]` without creating the `pair` first
list.emplace(s, std::move(ste));
// or
list[s] = std::move(ste);

You can even reduce your code more aggressively:

std::string s("key");
std::map<std::string, Struct1> list;

list[s] = {std::make_unique<std::string>("Hello")};
// or since C++20
list.emplace(s, std::make_unique<std::string>("Hello"));
user_128616390
Related