4

Let's say I have movable and not copyable object and I have boost multi-index array with random_access index. I need to move my object out of array front, but I cannot find any method, that would give me rvalue/lvalue reference in documentation. I can only see front() which gives me constant reference and pop_front() which erases element, but does not return anything. So is there a way to move element out of boost multi-index?

sehe
  • 374,641
  • 47
  • 450
  • 633
Slava
  • 43,454
  • 1
  • 47
  • 90

2 Answers2

3

Adding to @sehe's answer, the following shows how to modify the code in case your moveable type is not default constructible:

Edited: changed code to properly deal with destruction of *extracted.
Edited: added alternative with std::unique_ptr.
Edited: added a second altrnative by sehe.

Live On Coliru

#include <boost/multi_index_container.hpp>
#include <boost/multi_index/random_access_index.hpp>
#include <iostream>
#include <type_traits>

struct moveonly {
    int x;
    moveonly(int x) noexcept : x(x) {}
    moveonly(moveonly&& o) noexcept : x(o.x) { o = {-1}; }
    moveonly& operator=(moveonly o) noexcept { using std::swap; swap(x, o.x); return *this; }
};

static_assert(not std::is_copy_constructible<moveonly>{}, "moveonly");

namespace bmi = boost::multi_index;
using Table   = bmi::multi_index_container<moveonly,
    bmi::indexed_by<
        bmi::random_access<bmi::tag<struct _ra> >
    > >;

template <typename Container>
void dump(std::ostream& os, Container const& c) { 
    for (auto& r: c) os << r.x << " ";
    os << "\n";
}

moveonly pop_front(Table& table) {
    std::aligned_storage<sizeof(moveonly), alignof(moveonly)>::type buffer;
    moveonly* extracted = reinterpret_cast<moveonly*>(&buffer);

    auto it = table.begin();
    if (it == table.end())
        throw std::logic_error("pop_front");

    if (table.modify(it, [&](moveonly& v) { new (extracted) moveonly{std::move(v)}; })) {
        table.erase(it);
    }

    try {
        moveonly ret = std::move(*extracted);
        extracted->~moveonly();
        return ret;
    } catch(...) {
        extracted->~moveonly();
        throw;
    }
}

int main() {
    Table table;

    table.push_back({1});
    table.push_back({2});
    table.push_back({3});

    dump(std::cout << "table before: ", table);

    std::cout << "Extracted: " << pop_front(table).x << "\n";

    dump(std::cout << "table after: ", table);
}

Same thing using std::unique_ptr for cleanup:

Live On Coliru

#include <boost/multi_index_container.hpp>
#include <boost/multi_index/random_access_index.hpp>
#include <iostream>
#include <memory>
#include <type_traits>

struct moveonly {
    int x;
    moveonly(int x) noexcept : x(x) {}
    moveonly(moveonly&& o) noexcept : x(o.x) { o = {-1}; }
    moveonly& operator=(moveonly o) noexcept { using std::swap; swap(x, o.x); return *this; }
};

static_assert(not std::is_copy_constructible<moveonly>{}, "moveonly");

namespace bmi = boost::multi_index;
using Table   = bmi::multi_index_container<moveonly,
    bmi::indexed_by<
        bmi::random_access<bmi::tag<struct _ra> >
    > >;

template <typename Container>
void dump(std::ostream& os, Container const& c) { 
    for (auto& r: c) os << r.x << " ";
    os << "\n";
}

moveonly pop_front(Table& table) {
    std::aligned_storage<sizeof(moveonly), alignof(moveonly)>::type buffer;
    moveonly* extracted = reinterpret_cast<moveonly*>(&buffer);

    auto it = table.begin();
    if (it == table.end())
        throw std::logic_error("pop_front");

    if (table.modify(it, [&](moveonly& v) { new (extracted) moveonly{std::move(v)}; })) {
        table.erase(it);
    }

    std::unique_ptr<moveonly,void(*)(moveonly*)> ptr = {
        extracted,
        [](moveonly* p){ p->~moveonly(); }
    };

    return std::move(*extracted);
}

int main() {
    Table table;

    table.push_back({1});
    table.push_back({2});
    table.push_back({3});

    dump(std::cout << "table before: ", table);

    std::cout << "Extracted: " << pop_front(table).x << "\n";

    dump(std::cout << "table after: ", table);
}

Sehe provides yet another alternative based on boost::optional which is the most elegant of all:

Live On Coliru

#include <boost/multi_index_container.hpp>
#include <boost/optional.hpp>
#include <boost/multi_index/random_access_index.hpp>
#include <iostream>
#include <memory>
#include <type_traits>

struct moveonly {
    int x;
    moveonly(int x) noexcept : x(x) {}
    moveonly(moveonly&& o) noexcept : x(o.x) { o = {-1}; }
    moveonly& operator=(moveonly o) noexcept { using std::swap; swap(x, o.x); return *this; }
};

static_assert(not std::is_copy_constructible<moveonly>{}, "moveonly");

namespace bmi = boost::multi_index;
using Table   = bmi::multi_index_container<moveonly,
    bmi::indexed_by<
        bmi::random_access<bmi::tag<struct _ra> >
    > >;

template <typename Container>
void dump(std::ostream& os, Container const& c) { 
    for (auto& r: c) os << r.x << " ";
    os << "\n";
}

moveonly pop_front(Table& table) {
    boost::optional<moveonly> extracted;

    auto it = table.begin();
    if (it == table.end())
        throw std::logic_error("pop_front");

    if (table.modify(it, [&](moveonly& v) { extracted = std::move(v); })) {
        table.erase(it);
    }

    return std::move(*extracted);
}

int main() {
    Table table;

    table.push_back({1});
    table.push_back({2});
    table.push_back({3});

    dump(std::cout << "table before: ", table);

    std::cout << "Extracted: " << pop_front(table).x << "\n";

    dump(std::cout << "table after: ", table);
}
Joaquín M López Muñoz
  • 5,243
  • 1
  • 15
  • 20
  • Ah. That was simpler than I imagined. Somehow, dealing with raw storage intimidates me more than strictly required :) – sehe Sep 07 '17 at 12:12
  • Unfortunately hardly predictable cost of rebuilding indexes makes changing element to be copyable is preferred over using this solution. I am surprised that something like `element_type&& move_front();` and more generic `element_type &&extract( iterator it );` was not implemented. Are there complications with it or nobody requested? That method does not seem to be significantly more complicated than simple `pop_front()` and `erase()` but I am not aware of internals. – Slava Sep 07 '17 at 12:52
  • Btw why not to use `std::unique_ptr` and at the end just `return std::move( *ptr );`, that would eliminate placement new complications and creating unnecessary instance. – Slava Sep 07 '17 at 13:03
  • @Slava using `std::unique_ptr` without a custom deleter implies there's a heap allocation in the process, which is usually more expensive than any stack-based stuff you can use as an alternative. – Joaquín M López Muñoz Sep 07 '17 at 13:05
  • I see, but intermediate instance is not free either. Anyway without changing array itself there is seem to be no way to do it properly. – Slava Sep 07 '17 at 13:08
  • You always have to have two intermediate move constructions, one for extracting the value prior to erasure and the other returning it. This is regardless of whether you use `std::unique_ptr` or not. See my addition to the answer. – Joaquín M López Muñoz Sep 07 '17 at 13:14
  • 1
    Looking at the disassembly, it seems that using `boost::optional` internally does the "complicated" things with aligned storage: http://coliru.stacked-crooked.com/a/b8908ffafeecc228 – sehe Sep 07 '17 at 16:34
  • Didn't think of `optional`! This is certainly the most elegant and concise solution. Adding this to my answer. – Joaquín M López Muñoz Sep 07 '17 at 21:18
  • Cheers. Maybe this related question (about ptree) will interest you as well https://stackoverflow.com/questions/29561852/is-there-a-convenient-way-to-erase-a-node-from-a-property-tree-preserving-its-c/29563086?noredirect=1#comment79714972_29563086 – sehe Sep 23 '17 at 09:50
1

Non-const element operations are not supported because they could leave elements in a state which would break invariants placed on them by the various indexes.

The closest thing you can do is using modify:

moveonly pop_front(Table& table) {
    moveonly extracted;

    auto it = table.begin();
    if (it == table.end())
        throw std::logic_error("pop_front");

    if (table.modify(it, [&](moveonly& v) { extracted = std::move(v); })) {
        table.erase(it);
    }

    return extracted;
}

Note that modify does incur the cost of checking all indexes, and may fail. Fortunately, if it does fail, the effect is that iterator is erased:

  • Effects: Calls mod(e) where e is the element pointed to by position and rearranges *position into all the indices of the multi_index_container. Rearrangement on sequenced indices does not change the position of the element with respect to the index; rearrangement on other indices may or might not succeed. If the rearrangement fails, the element is erased.
  • Postconditions: Validity of position is preserved if the operation succeeds.

And here's a live demo:

Live On Coliru

#include <boost/multi_index_container.hpp>
#include <boost/multi_index/random_access_index.hpp>
#include <iostream>

struct moveonly {
    int x;
    moveonly(int x = -1) noexcept : x(x) {}
    moveonly(moveonly&& o) noexcept : x(o.x) { o = {}; }
    moveonly& operator=(moveonly o) noexcept { using std::swap; swap(x, o.x); return *this; }
};

static_assert(not std::is_copy_constructible<moveonly>{}, "moveonly");

namespace bmi = boost::multi_index;
using Table   = bmi::multi_index_container<moveonly,
    bmi::indexed_by<
        bmi::random_access<bmi::tag<struct _ra> >
    > >;

template <typename Container>
void dump(std::ostream& os, Container const& c) { 
    for (auto& r: c) os << r.x << " ";
    os << "\n";
}

moveonly pop_front(Table& table) {
    moveonly extracted;

    auto it = table.begin();
    if (it == table.end())
        throw std::logic_error("pop_front");

    if (table.modify(it, [&](moveonly& v) { extracted = std::move(v); })) {
        table.erase(it);
    }

    return extracted;
}

int main() {
    Table table;

    table.push_back({1});
    table.push_back({2});
    table.push_back({3});

    dump(std::cout << "table before: ", table);

    std::cout << "Extracted: " << pop_front(table).x << "\n";

    dump(std::cout << "table after: ", table);
}

Which prints:

table before: 1 2 3 
Extracted: 1
table after: 2 3 
sehe
  • 374,641
  • 47
  • 450
  • 633
  • I understand, why object are not allowed to be modified. I do not understand why there is no way to extract an element from array rather than just remove it. This method is pretty bad workaround as could be unnecessary expensive (rearranging indexes could be costly) and it also puts unnecessary requirement to element. – Slava Sep 06 '17 at 20:45
  • Btw, if `modify` returns false it probably should just return, rather than throwing exception. as element would be erased in this case (what we actually need) – Slava Sep 06 '17 at 20:47
  • @Slava Re: the second comment; Yeah. In this particular operation that seems fine. When I wrote the code I hadn't fully realized the implication of full iterator stability across the operation. Updated the answer. – sehe Sep 06 '17 at 20:55
  • I agree it could be nice having erase operations, move-iterators or maybe an outbound splice-operation that takes a non-BMI container or reference for the output. I'm pretty sure Joaquín M López Muñoz reads this tag, but you could always file a feature request – sehe Sep 06 '17 at 20:56
  • "I agree it could be nice having..." yes it would be nice to have full set of such operations. But there must be at least one way to move element from array. Currently it is one way road - you can move object in, but cannot move it out. :( – Slava Sep 06 '17 at 20:59
  • Hold on: I think my code shows it's not impossible. Also, when the move operation does *not* invalidate indices there isn't even going to be that much of a cost (yeah, the checks are redundant, but e.g. in your example there's no such checks since you had a RA index only). – sehe Sep 06 '17 at 21:05
  • I do have other indexes, otherwise I could use `std::vector` or `std::deque` instead and problem solved. – Slava Sep 06 '17 at 21:07
  • A `stable_vector` would actually be closer, but yeah, I guessed you did :) – sehe Sep 06 '17 at 21:08
  • 1
    `stable_vector` could be closer to `random_access` index, but maybe not what I need. What I am trying to implement is priority queue with ability to find and remove task by additional keys. But that details would be irrelevant and unnecessary here I think. – Slava Sep 06 '17 at 21:13
  • Hi, I skimmed through this and seems like C++17 `extract/merge` functonality would serve here, right? If someone files a ticket in Boost I can try to find the time to implement this. – Joaquín M López Muñoz Sep 07 '17 at 07:44
  • @sehe, there's a bug in your sample code: when `modify` fails the element is erased, so `table.erase(it)` would crash. This is not happening for your particular `Table`, but it will if you add some unique index. – Joaquín M López Muñoz Sep 07 '17 at 08:43
  • @JoaquínMLópezMuñoz I don't see how erase could fall in that case. If you look up ~6 comments you can see I specifically removed the check when the OP mentioned it. – sehe Sep 07 '17 at 09:16
  • @sehe Yes, `modify` won't ever fail in your example, but it might in @Slava's scenrario, where there's an additional (presumably unique) index. In any case, it doesn't hurt to change the logic to `if(table.modify(...)) table.erase(it);` – Joaquín M López Muñoz Sep 07 '17 at 10:43
  • @JoaquínMLópezMuñoz Oops. More to the point, my earlier ["realization"](https://stackoverflow.com/questions/46082394/move-element-from-boost-multi-index-array/46083899?noredirect=1#comment79130400_46083899) was completely flawed. Rolled back that change (and added doc quote about post-conditions of `modify`). – sehe Sep 07 '17 at 11:31
  • @sehe Your solution requires that the moved type be default constructible. For the fun of it, maybe you're in the mood of showing the OP how this restriction can be lifted by using stack-allocated aligned storage. – Joaquín M López Muñoz Sep 07 '17 at 11:42
  • @JoaquínMLópezMuñoz I'm not :) I was, instead, looking at what I remembered to be the downsides of C++17 extract/merge. – sehe Sep 07 '17 at 11:43
  • 1
    @sehe OK, I did it myself. Looking fwd to your feedback on `extract/merge`. – Joaquín M López Muñoz Sep 07 '17 at 11:55