Le Blog-Notes d'Olivier

I recently stumbled upon code that looked very suspicious to me.

Basically, it was a class which amongst other things held a vector and some iterators from that vector. Something like this:

struct Wrapper {
    typedef std::vector<int> Data;
    Data data;
    Data::iterator active;
    Wrapper() :
        data(),
        active(data.end())
    { }
    Wrapper(std::size_t n, int v) :
        data(n, v),
        active(data.begin())
    { }
};

Then in there was the move constructor (and similar move operator):

    Wrapper(Wrapper&& rhs) :
        data(std::move(rhs.data)),
        active(data.begin() + std::distance(rhs.data.begin(), rhs.active))
    { }

And there we have a problem: the source data is already moved (left in an invalid state after the move) when the distance between the iterator and the beginning of the vector is computed. This can only result in undefined behaviour and most probably fails.

If compiled with the G++ STL debug flag (-D_GLIBCXX_DEBUG), a call to the move constructor triggers an abort:

Objects involved in the operation:
iterator "this" @ 0x0x7fff7a3494f0 {
type = N11__gnu_debug14_Safe_iteratorIN9__gnu_cxx17__normal_iteratorIPiNSt9__cxx19986vectorIiSaIiEEEEENSt7__debug6vectorIiS6_EEEE (mutable iterator);
 state = dereferenceable;
 references sequence with type `NSt7__debug6vectorIiSaIiEEE' @ 0x0x7fff7a3494f0
}
iterator "other" @ 0x0x7fff7a349630 {
type = N11__gnu_debug14_Safe_iteratorIN9__gnu_cxx17__normal_iteratorIPiNSt9__cxx19986vectorIiSaIiEEEEENSt7__debug6vectorIiS6_EEEE (mutable iterator);
 state = singular;
 references sequence with type `NSt7__debug6vectorIiSaIiEEE' @ 0x0x7fff7a349630
}

The solution to this problem is not trivial. It involves the creation of a temporary object to calculate the iterator distance before the move. It looks like this:

private:
    struct MoveHandler {
        std::size_t activePos;
        Data data;
        MoveHandler(Wrapper&& rhs) :
            activePos(std::distance(rhs.active, rhs.data.begin())),
            data(std::move(rhs.data))
        { }
    };
    Wrapper(MoveHandler&& rhs) :
        data(std::move(rhs.data)),
        active(data.begin() + rhs.activePos)
    { }

public:
    Wrapper(Wrapper&& rhs) : Wrapper(MoveHandler(std::move(rhs))) { }

A similar treatment must be applied on the move operator:

    Wrapper& operator=(Wrapper&& rhs)
    {
        MoveHandler m(std::move(rhs));
        data = std::move(m.data);
        active = data.begin() + m.activePos;
        return *this;
    }

Now, of course, some might wonder why keep an iterator in the object instead of a distance. But that’s another debate…