1

My task is to create a template type array without calling the T (template) constructor. After that I want to move some value to this array from another with std::move. How can I do this in c++? Here is my code:

void *temp = malloc((end-begin) * sizeof(T));
for (unsigned int i = begin; i < end; ++i){
    temp[i] = std::move(array[i]);
}

But it isn't work. The compiler says the following: Subscript of pointer to incomplete type 'void'.

kbenda
  • 430
  • 5
  • 15
  • 3
    Wow, using raw malloc with the tag of C++11? – Tatsuyuki Ishi Mar 28 '17 at 09:50
  • 1
    Questions seeking debugging help ("**why isn't this code working?**") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it **in the question itself**. Questions without a **clear problem statement** are not useful to other readers. See: [How to create a Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve). – Tatsuyuki Ishi Mar 28 '17 at 09:51
  • 3
    Look at *placement new*. – Jarod42 Mar 28 '17 at 09:53

4 Answers4

2

An array of void makes no sense. void is an incomplete type (and has no size), so temp[i] generates the error you're seeing.

To achieve what you want to do, why not use a std::vector, and push items into it as they become available ?

std::vector<T> temp;
for (unsigned int i = begin; i < end; ++i){
    temp.push_back(std::move(array[i]));
}
Sander De Dycker
  • 16,053
  • 1
  • 35
  • 40
1

If you really want to do it by hand, you're looking for placement new. First you allocate a "raw buffer" of unsigned chars:

unsigned char* buf = new unsigned char(end-begin);

or:

auto buf = std::make_unique<unsigned char[]>(end-begin);

and then you write things like new (buf) T, new (buf+sizeof(T)) T, new (buf+2*sizeof(T)) T to "emplace" your objects into that memory space.

The problem is, in the above example I've completely ignored alignment requirements, which is dangerous.

So, instead of replicating what memory pools and std::vector do, just use std::vector!

std::vector<T> vec;
vec.reserve(end-begin);
for (unsigned int i = begin; i < end; ++i){
   vec.push_back(std::move(array[i]);
}

and you're done.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
0

As others point out void has no defined size so void* can't be indexed as an array.

Where does temp[1] begin?

A naïve fix of:

T *temp = std::malloc((end-begin) * sizeof(T));

Is just as bad when used in conjunction with:

temp[i] = std::move(array[i]);

That code will call the assignment operator and if non-trivial will fail when acting on the garbage that is in temp[i]. For example, if the assignment explicitly or implicitly de-allocates resources in the assignment target it will operate on the uninitialized temp[i] and fail.

If (IF!) you did this the correct code would be:

std::memcpy(temp+i*sizeof(i),array[I],sizeof(T));

That line of code is essentially assuming that T is a trivially-copyable type.

The first line is assuming objects of T have a trivial default constructor. If T has a trivial default constructor most compilers will optimize the element initialization away (which is what I assume the asker is trying to avoid).

So the recommended code (using a for loop) is:

T* temp= new T[end-being];
for(unsigned int i=begin;i<end;++i){
    temp[i-begin]=array[i];
}

NB: This code also fixes a bug in the original code that breaks when begin!=0. The code appears to be copying a sub-section from the middle of array to the start of temp but forgets to start at index 0 of temp.

But the recommended C++ approach to such copying is:

T* temp= new T[end-being];
std::copy(array+begin,array+end,temp);

Good implementations will determine and take advantage of any bulk memory operations where possible. So implicitly this should result in std::memmove(temp,array,(end-begin)*sizeof(T)); if valid.

The only final twist that a compiler might not recognize is that the potentially slightly more efficient

std::memcpy(temp,array,(end-begin)*sizeof(T));

Is actually valid in this case because we know temp cannot overlap with array (let alone the range being copied).

Footnote: As pointed out in the comments the most conventional C++ approach is to use std::vector which can normally be assumed to be using these optimizations behind the scenes. However if for some reason re-architecting the application is not possible or desirable this answer provides the valid efficient way of re-writing the code provided.

Persixty
  • 8,165
  • 2
  • 13
  • 35
  • well, the C++-approach is to use a vector :) – The Techel Mar 28 '17 at 10:29
  • @TheTechel Normally true. Indeed under the hood we'd expect `std::vector` to notice the trivialities and use bulk copies and indeed take advantage of `std::memcpy`! But there are (some slight) overheads of `std::vector` and if the poster has some reason to set those aside this is the right way to achieve the outcome without recoding their application... I will make a note. – Persixty Mar 28 '17 at 10:35
-1

Try

T *temp = (T*) malloc((end-begin) * sizeof(T));
Sergey Slepov
  • 1,861
  • 13
  • 33