marl::containers::vector fixes

[ben-clayton]

Default copy constructor and copy assignment operators were still being generated, which produce bad implementations due to raw internal allocations.

The tests were only exercising copy and assignment for vectors of different base capacities, which we do have non-default implementations for.

Delete the copy constructor - it discourages use allocators, which is a Bad Thing.

Implement the assignment operator.

Add tests.

Note: Nothing in marl was exercising these broken default constructors.

[amaiorano]

In general, a type that implements copy assignment also implements copy construction - they go together. Couldn't you implement copy assignment to use the allocator from the assignee?

[ben-clayton]

I could, but that'll behave differently to the other copy constructors that default to marl::Allocator::Default.

I don't consider the marl::containers namespace as a public API, and I prefer to not have the default copy constructor as it forces me to consider memory allocation, which is important for fixing #131.

[amaiorano]

I could, but that'll behave differently to the other copy constructors that default to marl::Allocator::Default.

I don't consider the marl::containers namespace as a public API, and I prefer to not have the default copy constructor as it forces me to consider memory allocation, which is important for fixing #131.

Just for reference, std::vector has 2 copy constructors: one that takes an 'other' and an 'allocator', and there's the default one that only takes the 'other'. As per #6 in the doc:

6) Copy constructor. Constructs the container with the copy of the contents of other. If alloc is not provided, allocator is obtained as if by calling std::allocator_traits<allocator_type>::select_on_container_copy_construction(other.get_allocator())

So I'm with you on not allowing a default constructor to be generated. Just saying you can write one that does the right thing and uses the allocator from 'other'.

Anyway, your call.

[ben-clayton]

Just saying you can write one that does the right thing and uses the allocator from 'other'.

I'm unsure about wanting to re-use the allocator from other.
All the places where I can imagine we need to copy a marl::containers::vector, we're doing it to make a copy of a public API object. These public objects should all take an allocator themselves, so copying into the allocator of other seems broken.

If I do end up needing cases where I want to copy from other, I can explicitly do this at the call site.

Maybe I'll find I'm barking up the wrong tree, and do end up needing this. However, it's always easier to undelete an API than deleting one that might be in use somewhere.