How to move a non-copyable element from std::set to std::map using extract?

Question

Consider the following:

#include <set>
#include <map>

struct MyClass
{
    MyClass(int i);
    MyClass(MyClass const&) = delete;
    ~MyClass();

    bool operator<(const MyClass& r) const { return v < r.v; }
    int v;
};

void map_set_move_test()
{
    std::set<MyClass> s1, s2;
    std::map<int, MyClass> m1;

    s1.emplace(123);

    s2.insert(std::move(s1.extract(s1.begin())));

    // This fails
    m1.insert(std::move(std::make_pair(1, std::move(s2.extract(s2.begin()).value()))));
}

I used std::set::extract to successfully move an element from std::set to another std::set, as in:

s2.insert(std::move(s1.extract(s1.begin())));

But the compiler doesn't allow moving the element to std::map this way:

m1.insert(std::move(std::make_pair(1, std::move(s2.extract(s2.begin()).value()))));

Any help would be appreciated. Link to compiler explorer.

I am not sure whether transferring implementation-defined node handles between different container types is the intended use case?! — lubgr, Aug 15 '20 at 17:37
@lubgr But `value()` can get the actual value of the node handle, if I'm not mistaken? — Sprite, Aug 15 '20 at 17:40
You cannot do this, it's as simple as that. `extract` does not work this way. — Sam Varshavchik, Aug 15 '20 at 17:40
Writing `std::move(std::make_pair(...));` doesn't make a lot of sense to me. — Waqar, Aug 15 '20 at 17:55

einpoklum · Accepted Answer · 2020-08-15T20:20:05.713

This is a problematic example, because MyClass is not merely non-copyable, but implicitly non-movable - which is probably not what you meant. (I think MyClass becomes immovable as a result of you explicitly deleting the copy constructor; see this answer.)

If you do make MyClass movable, like so:

class MyClass : NonCopyable
{
public:
    MyClass(int i) : v(i) {
        std::cout << "constructor" << std::endl;
    }
    MyClass(MyClass&& other) : v(other.v) {
        std::cout << "move constructor" << std::endl;
    }
    ~MyClass() {
        std::cout << "destructor" << std::endl;
    }
    bool operator<(const MyClass& r) const {
        return v < r.v;
    }

    int v;
};

all seems to go well, when you write, say.

m1.emplace(
    1,
    std::move( 
        s2.extract(s2.begin()).value() 
    )
);

You can see this working on GodBolt.

_{PS - As @DavisHerring suggests, you might want to use the try_emplace() method instead, if you don't want to overwrite an existing value.}

Thank you, you're right! In the real case of this question, `MyClass` is similar to `std::unique_ptr`. This is my test example error. — Sprite, Aug 15 '20 at 18:09
You’ve answered the literal question; I seem to have answered the title (or the spirit of the question?). I don’t know which is more useful! — Davis Herring, Aug 15 '20 at 18:14

score 3 · Answer 2 · answered Aug 15 '20 at 17:46

3

The point of extract and the corresponding overload of insert is to manipulate node-based containers without memory allocation. (One interesting such manipulation is to change std::map keys.) That cannot work here because the nodes are different types (viz., one has a key and the other doesn’t), so you need to just use the value with a regular insert (or the more ergonomic try_emplace).

answered Aug 15 '20 at 17:46

Davis Herring

36,443
4
48
76

Heh, interesting wording question - does a set contain keys or does a set contain values? I'd always thought of it as the former but apparently you think of it as the latter. – Barry Aug 15 '20 at 17:48
@JeJo Why should it not work? The fact that the value in question here comes from an `extract()` call doesn't really matter - it's just that you made `MyClass` movable (so this call can move it just fine) but in OP's case it was not (so it cannot). – Barry Aug 15 '20 at 18:00
1

@Barry: The standard calls them keys, and that’s certainly appropriate in a context where various container types are in play (*cf.* Python, where `set(my_dict)` copies the keys, since they’re what’s hashable and unique). I think of them as values because it lets all containers have values; also, iterating over a `set` (which yields `value_type` for *any* container) is very common and there is no `operator[]` (although of course the key-like `find` can be useful for canonicalization and `count` for membership testing). – Davis Herring Aug 15 '20 at 18:11
@Barry: Two more points: node handles for a `std::set` call it `value_type` only (since you can’t look anything up there, I guess), and this question happened to want to use the `set` elements as the `mapped_type` (which I would also tend to call “values” despite `std::map<…>::value_type` being the `pair`). – Davis Herring Aug 15 '20 at 19:59

How to move a non-copyable element from std::set to std::map using extract?

2 Answers2