delete vector inside vector issue

Question

template <typename T>
class dvec {
    size_t n = 0;
    size_t capacity = 0;
    T* data = nullptr;
public:
    dvec() {
        recapacity(21);
    }
    virtual ~dvec() {
        if (data != nullptr) {
            delete[] data;
            data = nullptr;
        }
    }
    T& operator [] (size_t k) {
        if (k < n)
            return data[k];
        throw "Index out of range";
    }
    void recapacity(size_t k = capacity) {
        capacity = k;
        const size_t bytes = sizeof(T) * capacity;
        T* newData = static_cast<T*>(::operator new(bytes));
        memcpy(newData, data, sizeof(T) * n);
        if (data != nullptr)
            delete[] data;
        data = newData;
    }
    void resize(size_t k, T init = T()) {
        if (k > capacity)
            recapacity(2 * k);
        for (size_t i = n; i < k; i++)
            data[i] = init;
        n = k;
    }
    void push_back(const T& a) {
        resize(n + 1);
        data[n - 1] = a;
    }
};

I want to make some custom vector but current my vector doesn't work in vector scoped vector. This throw error when delete (going out scorp) I guess first inside vector delete so outside vector can't delete spaces that have been deleted.

Not directly related to your question, but your class handle dynamic allocation in its default constructor and destructor. According to the [rule of Three](https://stackoverflow.com/q/4172722/3545273) or [Five](https://stackoverflow.com/q/4782757/3545273) it should have non default copy (move?) constructor and assignment operator. — Serge Ballesta, Jun 24 '21 at 13:40
The `memcpy` means you can't handle just any T. You'll need a concept check: `template `. And this will (correctly) block a vector of vectors. — MSalters, Jun 24 '21 at 13:59

Caleth · Accepted Answer · 2021-06-24T13:53:36.827

Your implementation of recapacity is faulty.

First problem is that memcpy requires trivially copyable types, which dvec<T> isn't, so dvec<dvec<T>> is ill-formed.

Second problem is that you use ::operator new to allocate, and delete[] to deallocate, so dvec<T> is ill-formed.

Third problem is that you can't have a data member as the default for a parameter, so size_t k = capacity probably isn't what you think it is

Forth problem is that you lack copy/move constructors/assignments, so it is easy to have leaks or double deletion.

dvec(const dvec &other) : n(other.n) {
    recapacity(other.n);
    std::copy_n(data, n, other.data);
}
dvec(dvec &&other) 
: n(std::exchange(other.n, 0)),
  capacity(std::exchange(other.capacity, 0)),
  data(std::exchange(other.data, nullptr)) {
}
dvec& operator=(const dvec& other) {
    if (capacity < other.n) {
        recapacity(other.n);
    }
    std::copy_n(data, n, other.data);
    return *this;
}
dvec& operator=(dvec&& other) {
    n = std::exchange(other.n, 0);
    capacity = std::exchange(other.capacity, 0);
    data = std::exchange(other.data, nullptr);
    return *this;
}
virtual /*?*/ ~dvec() {
    ::operator delete(static_cast<void*>(data));
}
void recapacity(size_t k) {
    capacity = k;
    T* newData = static_cast<T*>(::operator new(sizeof(T) * capacity));
    if constexpr (std::is_nothrow_move_constructible_v<T>) {
        std::uninitialised_move_n(data, size, newData);
    } else {
        std::uninitialised_copy_n(data, size, newData);
        std::destroy_n(data, size);
    }
    ::operator delete(static_cast<void*>(data));
    data = newData;
}

delete vector inside vector issue

1 Answers1