jeudi 25 février 2016

How to safely use std::move() on template data

I am working in VS2013 and C++11.

I am implementing a custom templatized collection. When the collection exceeds capacity, it resizes it's storage. At that point the data should move from the old storage to the new storage.

I would really like enforce safe move semantics on the data elements T. If a data element owns a resource, ownership of the resource should be pilfered from the original storage and move to the new storage. A typical case might be strings or pointers to data array or other resources.

I have several data types that have explicit move constructors and move assignment operators. But I get bugs if those types with explicit move constructors (DeepData1) are themselves members other data structs with trivial constructors (DeepData2). According to how I read this article, I expect I should be getting an implicit compiler generated move constructor on DeepData2. http://ift.tt/V6FbVl

But in the below sample, I show that relying on the implicit constructors of DeepData2 crashes due to the double delete on the pointer _IMPORTANT_DATA. If I make the move constructor of DeepData2 explicit, the code runs fine.

I had hoped to not to need to do that and to be able to rely on implicit move constructors. Othewise it seems a burden on user code to have to remember to provide the extra constructor and assignment.. If it is a requirement that DeepData2 will definitely need an explicite move constructor, can I make it an error if the user code forgets to supply one? Is there anyway to detect if template type requiring an explicit move constructor due to a member that has an explicit move constructor? When I use the std type traits they do not seem to give me enough information to write a decent assert like "hey user code, for template arg T you need move semantics and forgot"

This is complicated. Thanks for any advice or help

#include "stdafx.h"

#include <vector>
#include <algorithm>
#include <iostream>


template <typename T>
class DeepVector
{
public:
    DeepVector()
    {
        deepCopyResize(4);
    }

    void push_back(T& v)
    {
        if (_capacity <= _count)
        {
            deepCopyResize(_capacity * 2);
        }

        // !! deep copy desired here !!
        _data[_count++] = std::move(v);
    }

    T& operator[](int i) { return _data[i];  }

    void deepCopyResize(int cap)
    {
        int n = std::min(_count, cap);
        T* d = new T[cap];
        if (_data)
        {
            for (int i = 0; i < n; ++i)
            {
                // !! deep copy desired here !!
                d[i] = std::move(_data[i]);
            }
            delete[] _data;
        }
        _data = d; 
        _capacity = cap; 
        _count = n;
    }
private:

    int _capacity = 0;
    int _count = 0;
    T* _data = nullptr; 
};


struct FlatData1
{
    int x = 0, y = 0; 
};

struct DeepData1
{
    DeepData1()
    {

    }

    DeepData1(int s)
    {
        _size = s;
        _IMPORTANT_DATA = new int[_size];
    }

    // move constructor
    DeepData1(DeepData1&& rhs)
    {
        _size = rhs._size;
        _IMPORTANT_DATA = rhs._IMPORTANT_DATA;  // pilfer
        rhs._size = 0;
        rhs._IMPORTANT_DATA = nullptr;
    }

    // move operator
    DeepData1& operator=(DeepData1&& rhs)
    {
        _size = rhs._size;
        _IMPORTANT_DATA = rhs._IMPORTANT_DATA; // pilfer
        rhs._size = 0;
        rhs._IMPORTANT_DATA = nullptr;
        return *this;
    }

    ~DeepData1()
    {
        if (_IMPORTANT_DATA)
        {
            std::cout << "non-trivial destructor" << std::endl;
            _size = 0; 

            // it is an error to delete important twice
            delete[] _IMPORTANT_DATA;
            _IMPORTANT_DATA = NULL;
        }
    }
    int _size = 0; 
    int* _IMPORTANT_DATA = nullptr; 

    void resize(int s)
    {
        delete[] _IMPORTANT_DATA; 
        _IMPORTANT_DATA = new int[s];
        _size = s;
    }
};

struct DeepData2
{
    int z = 0; 
    DeepData1 problem;     // this data does not deep copy implicitly ?

    //  DeepData2() {} 

//   despite C++ standard forcing default not supported by VS2013
//    DeepData2(DeepData2&&) = default;


//    DeepData2(int s) : problem(s) {}
    //!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    // where are my implicit move constructors?

    // I have to uncomment these for the 
    // DeepData::operator=(DeepData&& rhs)
    // operator to be called 

    /*
     // have to manually implement move constructor?
     DeepData2(DeepData2&& rhs)
     {
     z = std::move(rhs.z);
     problem = std::move(rhs.problem);
     }

     // move operator
     DeepData2& operator=(DeepData2&& rhs)
     {
     z = std::move(rhs.z);
     problem = std::move(rhs.problem);
     return *this;
     }
   */

    // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!
};


int _tmain(int argc, _TCHAR* argv[])
{

    // ----------------------------------------------
    DeepVector<int> v1; 
    for (int i=0; i<5; ++i)
    { 
        v1.push_back(i);
    }

    if (v1[4] == 4)
    {
        std::cout << "resize 1 worked" << std::endl;
    }

    // ----------------------------------------------
    DeepVector<FlatData1> v2;
    for (int i = 0; i < 5; ++i)
    {
        v2.push_back(FlatData1());
        v2[i].x = i;
        v2[i].y = i;
    }

    if (v2[4].x == 4)
    {
        std::cout << "resize 2 worked" << std::endl;
    }

    // ----------------------------------------------
    DeepVector<DeepData1> v3;
    for (int i = 0; i < 5; ++i)
    {
        v3.push_back(DeepData1(10));

    }

    if (v3[4]._size == 10)
    {
        std::cout << "resize 3 worked" << std::endl;
    }


    // ----------------------------------------------


    bool b1 = std::is_move_constructible<DeepData1>();
    bool b2 = std::is_move_assignable<DeepData1>();
    bool b3 = std::is_trivially_move_assignable<DeepData1>();
    bool b4 = std::is_trivially_move_constructible<DeepData1>();

    bool b5 = std::is_move_constructible<DeepData2>();
    bool b6 = std::is_move_assignable<DeepData2>();

    // VS2013 says DeepData2 is trivially moveable with the implicit constructors
    bool b7 = std::is_trivially_move_assignable<DeepData2>();
    bool b8 = std::is_trivially_move_constructible<DeepData2>();

    DeepVector<DeepData2> v4;
    for (int i = 0; i < 5; ++i)
    {
        DeepData2 d2;
        d2.problem.resize(10);
        v4.push_back(d2); 
    }

    if (v4[4].problem._size == 10)
    {
        std::cout << "resize 4 worked" << std::endl;
    }


    return 0;
}

Aucun commentaire:

Enregistrer un commentaire