It's a C++ STL Rant : Constness and Vector Erase

I believe I am right and correct to be seethingly annoyed that this code does not compile...

#include <vector>

#include <iostream>

#include <algorithm>

struct Test

{

    const float value;

    int changable;

};

int main()

{

    std::vector<Test> items;

    items.push_back({42.5f, 0});

    items.push_back({1.23f, 1});

    items.erase(items.begin());

}

It just really annoys me this won't compile, do you know why?  Don't cheat, just have a look why does this PERFECTLY REASONABLE AND ACTUALLY I THINK CORRECT CODE NOT COMPILE?

Yes, because of the ISO Standard, and it's really annoying.

ISO/IEC 14882:2003 23.1 [lib.container.requirements] / 3:

The type of objects stored in these components must meet the requirements of CopyConstructible types (20.1.3), and the additional requirements of Assignable types.

The compiler could have been told this far more simply if the std::vector had an earlier and cleaner static_assert that the type is copy constructible before it got into the weeds of the real message we get.

For can't compile because it seems the standard implementation of erase results in a bunch of move semantics in the vector, which then devolve into copies.  And to my simple monkey brain it's all just babbling horrible template junk that comes out the compiler as well.

You can of course move a struct of type Test perfectly well, or at least mark it for move (cast it thus):

#include <vector>

#include <iostream>

#include <algorithm>

struct Test

{

    const float value;

    int changable;

};

int main()

{

    std::vector<Test> items;

    items.push_back({42.5f, 0});

    items.push_back({1.23f, 1});

    Test bar { 123.456f, 2};

    items.push_back(std::move(bar));    

}

This compiles and runs just fine!

But it's not copy assignable.  Let us take a look at the compiler spew:

In file included from <source>:1:

In file included from /opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/vector:60:

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/stl_algobase.h:427:4: error: static assertion failed due to requirement 'is_move_assignable<Test>::value': type must be assignable

          static_assert( __assignable::value, "type must be assignable" );

          ^              ~~~~~~~~~~~~~~~~~~~

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/stl_algobase.h:495:22: note: in instantiation of function template specialization 'std::__copy_move<true, true, std::random_access_iterator_tag>::__copy_m<Test>' requested here

                              _Category>::__copy_m(__first, __last, __result);

                                          ^

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/stl_algobase.h:522:19: note: in instantiation of function template specialization 'std::__copy_move_a2<true, Test *, Test *>' requested here

    { return std::__copy_move_a2<_IsMove>(__first, __last, __result); }

                  ^

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/stl_algobase.h:530:8: note: in instantiation of function template specialization 'std::__copy_move_a1<true, Test *, Test *>' requested here

                std::__copy_move_a1<_IsMove>(std::__niter_base(__first),

                     ^

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/stl_algobase.h:652:19: note: in instantiation of function template specialization 'std::__copy_move_a<true, __gnu_cxx::__normal_iterator<Test *, std::vector<Test>>, __gnu_cxx::__normal_iterator<Test *, std::vector<Test>>>' requested here

      return std::__copy_move_a<true>(std::__miter_base(__first),

                  ^

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/vector.tcc:179:2: note: in instantiation of function template specialization 'std::move<__gnu_cxx::__normal_iterator<Test *, std::vector<Test>>, __gnu_cxx::__normal_iterator<Test *, std::vector<Test>>>' requested here

        _GLIBCXX_MOVE3(__position + 1, end(), __position);

        ^

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/stl_algobase.h:656:44: note: expanded from macro '_GLIBCXX_MOVE3'

#define _GLIBCXX_MOVE3(_Tp, _Up, _Vp) std::move(_Tp, _Up, _Vp)

                                           ^

/opt/compiler-explorer/gcc-12.2.0/lib/gcc/x86_64-linux-gnu/12.2.0/../../../../include/c++/12.2.0/bits/stl_vector.h:1530:16: note: in instantiation of member function 'std::vector<Test>::_M_erase' requested here

      { return _M_erase(begin() + (__position - cbegin())); }

               ^

<source>:20:11: note: in instantiation of member function 'std::vector<Test>::erase' requested here

    items.erase(items.begin());

          ^

1 error generated.

Compiler returned: 1

We read this from the bottom up, first erase needs to call "_M_erase(begin() + (__position - cbegin())) what on earth?  Well it appears this is trying to perform a copy-move of the rest of the vector; the rest of the stack all the way to the top shows us various iterator types to the top two messages where we have __copy_m and then a compile-time static assert that the type must be assignable.

With the intentionally const member we can't compile we therefore have two options, first we make the member non-constant.  This isn't very practical because the type is derived from a bunch of runtime values, they're not known upfront and can be different for each instance of the struct.

The second option therefore is even less appealing and looks like this hideous mess:

#include <vector>

#include <iostream>

#include <algorithm>

struct Test

{

    const float value;

    int changable;

    Test& operator=(Test pOther)

    {

        float& mutableV { const_cast<float&>(this->value) };

        mutableV = pOther.value;

        this->changable = pOther.changable;

        return *this;

    }

};

int main()

{

    std::vector<Test> items;

    items.push_back({42.5f, 0});

    items.push_back({1.23f, 1});

    Test bar { 123.456f, 2};

    items.push_back(std::move(bar));

    items.erase(items.begin());

}

We define this assignment operator performing this const reference magic and just.... ergh, don't do this folks, just don't.  Not only is this an excellent lesson in const member flagging in STL and it's pitfalls.  But then it's also a lesson that const doesn't really mean const.  AND it's a lesson on whether you want to keep a constness or nor given such awful hacks to get around it.

I could even go further than this, the operator= could be made private and the STL type "std::vector<Test>" could be made a friend, or the __copy_m could be made a friend; though in that last case the MSVC implementation of the STL will be different, so we'll have to work around platform differences too.

No, the most annoying thing here is picking open quite why __copy_m wants to end up assigning.  I am still digging into why, but oh boy am I annoyed by this one.  For in a module I unironically had to repeat at University solved this in 1996.... I would use a linked list, and simply pluck out the value... or I would skip to the index of the item to remove and copy those after (if they were host memory contiguous then memcpy speed achieved) and just write over and this erase is achieved.

To just see this wanting to copy assign at the element level is just hideous, and I hate it, and I with the ISO standard would change in this regard,