Using move semantics to avoid copying when pushing_back to a custom container does not avoid copying

Question

I have implemented a custom container (same vein as std::vector) and I am trying to make it so that its 'push_back' function would use leverage on move semantics to avoid creating a copy of whatever is being pushed back - specially when the object to be pushed into the container is returned by an external function.

After reading quite a bit about move semantics and custom containers, I still can't find why my approach is still generating a copy instead of just moving the passed object into the container's inner dynamic array.

Here is a simplified version of my container looks like:

template<class T>
class Constructor
{
private:
    size_t size = 0;
    size_t cap = 0;
    T *data = nullptr;

public:
Constructor()
{
    cap = 1;
    size = 0;
    data = static_cast<T*>(malloc(cap * sizeof(T)));
}

~Constructor()
{ delete[] data; }

template<typename U>
void push_back(U &&value)
{
    if (size + 1 >= cap)
    {
        size_t new_cap = (cap * 2);
        T* new_data = static_cast<T*>(malloc(new_cap * sizeof(T)));
        memmove(new_data, data, (size) * sizeof(T));
        for (size_t i = 0; i<cap; i++)
        {
            data[i].~T();
        }

        delete[] data;

        cap = new_cap;
        data = new_data;
        new(data + size) T(std::forward<U>(value));
    }
    else
    {
        new(data + size) T(std::forward<U>(value));
    }

    ++size;
}

const T& operator[](const size_t index) const //access [] overloading
{
    return data[index];
}
};

Here is a custom class that will print messages when its instances are created, copied or moved, in order to help debugging:

class MyClass
{
size_t id;

public:
MyClass(const size_t new_id)
{
    id = new_id;
    std::cout << "new instance with id " << id << std::endl;
}
MyClass(const MyClass &passedEntity)
{
    id = passedEntity.id;
    std::cout << "copied instance" << std::endl;
}
MyClass(MyClass &&passedEntity)
{
    id = passedEntity.id;
    std::cout << "moved instance" << std::endl;
}

void printID() const
{
    std::cout << "this instance's id is " << id << std::endl;
}
};

And here is the external function:

MyClass &foo(MyClass &passed)
{
    return passed;
}

Lastly, here is the main function that runs a test case using the above function and classes to show the problem:

int main()
{
MyClass a(33);
std::cout << std::endl;

std::cout << "Using my custom container: " << std::endl;
Constructor<MyClass> myContainer;
myContainer.push_back(foo(a));
myContainer[0].printID();
std::cout << std::endl;

std::cout << "Using dinamic array: " << std::endl;
MyClass *dinArray = static_cast<MyClass*>(malloc(1 * sizeof(MyClass)));
dinArray = new(dinArray + 1) MyClass(std::forward<MyClass>(foo(a)));
dinArray[0].printID();
std::cout << std::endl;


system("Pause");
return 0;
}

The output is:

new instance with id 33

Using my custom container:
copied instance
this instance's id is 33

Using dinamic array:
moved instance
this instance's id is 33

As it can be seen, if the instance of MyClass is put directly into a dynamic array, then just the move conmstructor is called and not the copy constructor. However, if I push_back the yClass instance into an instance of Container, a copy constructor is still being called.

Could someone help me understand what exactly am I doing wrong here? How could I make it so that elements are pushed into the container without generating a copy?

If you want to emulate move semantics in C++03, use std::swap: `void emplace(vector& v, T& value) { v.resize(v.size()+ 1); std::swap(v.back(), value); }` You may have to define std::swap for some of your types as well. A pseudo move constructor of some sort. — Michaël Roy, Dec 05 '17 at 13:12

score 0 · Answer 1 · answered Dec 05 '17 at 10:19

It is not safe to perform a memmove with objects in C++. You can find more information here Will memcpy or memmove cause problems copying classes?

If this is C++11 onwards, then what you want to use is placement new and the move constructor. (you could probably just bin the placement new though unless you really want to keep allocating memory yourself)

If this is any other version of C++ then you'll have to just accept that that either you're going to have to copy the object (like the rest of the stl) or that your object will have to implement a function like void moveTo(T& other)

score 0 · Accepted Answer · edited Jun 20 '20 at 09:12

When you call this line

myContainer.push_back(foo(a));

L-value is passed into push_back method, and now read about using std::forward - http://www.cplusplus.com/reference/utility/forward/,

Returns an rvalue reference to arg if arg is not an lvalue reference.

If arg is an lvalue reference, the function returns arg without modifying its type.

and in your push_back you call

new(data + size) T(std::forward<U>(value));

but value was passed as L-value, and only constructor MyClass(const MyClass &passedEntity) can be invoked.

If you want a object to be moved, you can write

myContainer.push_back(std::move(a)); // cast to R-reference

EDIT

You should not use move in your push_back function, below is simple example. Suppose you have class like this:

 struct Foo {
 int i;
 Foo (int i = 0) : i(i) { 
 }
 ~Foo () { 
 }
 Foo (const Foo& ) { 
 }
 Foo& operator=(const Foo&) { 
    return *this;
 }
 Foo (Foo&& f) 
 { 
    i = f.i; 
    f.i = 0;   // this is important
 }
 Foo& operator=(Foo&& f) 
 {   
    i = f.i; 
    f.i = 0; // this is important
    return *this;
 }
};

we have also 2 functions

template<class T> 
void process1 (const T& ) {
    cout << "process1" << endl;
}

template<class T>
void process (T&& obj) {
    cout << "process2" << endl;
    T newObj = forward<T>(obj);
}

and bars functions are counterparts of your push_back method.

template <typename T>
void bar1 (T&& value) {
    process (move(value));   // you use move in your push_back method 
}

template <typename T>
void bar2 (T&& value) {
    process (forward<T>(value));
}

now we must consider 4 cases:

[1] pass L-value, version with forward

Foo f(20);
bar2 (f);
cout << (f.i) << endl; // 20

[2] pass R-value, version with forward

Foo f(20);
bar2 (move(f));
cout << (f.i) << endl; // 0, it is OK bacuse we wanted to move 'f' object

[3] pass R-value, version with move

Foo f(20);
bar1 (move(f));
cout << (f.i) << endl; // 0, ok, we wanted to move 'f' object

[4] pass L-value, version with move in your push_back method

Foo f(20);
bar1 (f);
cout << (f.i) << endl; // 0 !!! is it OK ? WRONG

in last case, we passed f as L-value, but this object was moved in bar1 function, for me this is strange behavior and is incorrect.

Thanks for the answer. So, inside my `push_back` function, why couldn't I simply do `new(data + size) T(std::move(value));` instead of using `std::move` only when calling `push_back`? — AndraSol, Dec 05 '17 at 14:48
You cannot use `move` inside your _push_back_ method bacuse `move` always returns R-reference. Return type of `forward` depends on a type of `value`. When you call _push_back_ with L-value (you pass `a` object using `.push_back(a)` ) you will get L-reference as type of `value`, and it is ok, because you don't want to move `value` object inside your _push_back_ method. When you call `push_back` with R-value ( you pass 'a' using `.push_back(move(a))`), `value` will have R-reference type and move constructor could be invoked. — rafix07, Dec 05 '17 at 15:33
It's odd, I have changed just that, i.e. `std::move()` where before I had `std::forward()` and works perfectly now. I have been using the whole day without any noticeable issues. — AndraSol, Dec 06 '17 at 05:28

Using move semantics to avoid copying when pushing_back to a custom container does not avoid copying

2 Answers2