Does std::vector

Question

I'm trying to reserve some spots into an std::vector, and having an error I don't understand:

#include <iostream>
#include <string>
#include <vector>

using namespace std;

class Foo{
public:    
    std::string str;
    int i;

    Foo(){
        this->str = "";
        this->i = 0;
    }

    Foo(Foo &copyFoo){
        this->str = copyFoo.str; //Or get methods if private members
        this->i = copyFoo.i;
    }

    Foo(std::string str, int i){
        this->str = str;
        this->i = i;
    }
};

int main()
{
   std::vector<Foo> fooVector;
   fooVector.reserve(20); // Error

   for(int i=0; i<20; i++){
       fooVector[i] = Foo("Test", i); // Or should I use operator new?
       // Or should I even stick to the push_back method?
   }

   return 0;
}

Of course I could just not reserve, and it would probably work. But now I'm interested in why it is not working now. I added the copy constructor because it looked that could be the problem I was having back then. But after adding the copy constructor, it doesn't work either.

The error says:

In file included from

/usr/local/gcc-4.8.1/include/c++/4.8.1/vector:62:0,
             from main.cpp:3: /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_construct.h: In
instantiation of 'void std::_Construct(_T1*, _Args&& ...) [with _T1 =

Foo; _Args = {Foo}]':

/usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_uninitialized.h:75:53:

required from 'static _ForwardIterator

std::__uninitialized_copy<TrivialValueTypes>::_uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::move_iterator; _ForwardIterator = Foo*; bool

_TrivialValueTypes = false]' /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_uninitialized.h:117:41:

required from '_ForwardIterator

std::uninitialized_copy(_InputIterator, _InputIterator,

_ForwardIterator) [with _InputIterator = std::move_iterator; _ForwardIterator = Foo*]' /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_uninitialized.h:258:63:

required from '_ForwardIterator

std::__uninitialized_copy_a(_InputIterator, _InputIterator,

_ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = std::move_iterator; _ForwardIterator = Foo*; _Tp = Foo]'

/usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_vector.h:1142:29:

required from 'std::vector<_Tp, _Alloc>::pointer std::vector<_Tp,

_Alloc>::_M_allocate_and_copy(std::vector<_Tp, _Alloc>::size_type, _ForwardIterator, _ForwardIterator) [with _ForwardIterator = std::move_iterator; _Tp = Foo; _Alloc = std::allocator;

std::vector<_Tp, _Alloc>::pointer = Foo*; std::vector<_Tp,

_Alloc>::size_type = long unsigned int]' /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/vector.tcc:75:70:

required from 'void std::vector<_Tp, _Alloc>::reserve(std::vector<_Tp,

_Alloc>::size_type) [with _Tp = Foo; _Alloc = std::allocator; std::vector<_Tp, _Alloc>::size_type = long unsigned int]'

main.cpp:31:24: required from here

/usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_construct.h:75:7:

error: no matching function for call to 'Foo::Foo(Foo)'
 { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }

   ^ /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_construct.h:75:7:
note: candidates are: main.cpp:22:5: note: Foo::Foo(std::string, int)
 Foo(std::string str, int i){

 ^ main.cpp:22:5: note:   candidate expects 2 arguments, 1 provided main.cpp:17:5: note: Foo::Foo(Foo&)

 Foo(Foo &copyFoo){

 ^ main.cpp:17:5: note:   no known conversion for argument 1 from 'Foo' to 'Foo&' main.cpp:12:5: note: Foo::Foo()

 Foo(){

 ^ main.cpp:12:5: note:   candidate expects 0 arguments, 1 provided

Where is the problem? Do I need to initialize the std:vector objects, or just assign each position to an object instance?

EDIT: I'm using C++11. And If I remove the copy constructor, I receive the following error at the reserve method line:

required from 'void std::_Construct(_T1*, _Args&& ---) [with _TI = Foo; _Args = {Foo&}]

That's why I wrote the copy constructor in the first place.

I don't want to use the resize method because I want the size method to return the actual number of Foo objects contained into the vector, not the amount I reserved.

score 8 · Accepted Answer · answered Jan 27 '14 at 15:47

8

   std::vector<Foo> fooVector;
   fooVector.reserve(20); // Error

   for(int i=0; i<20; i++){
       fooVector[i] = Foo("Test", i); // Or should I use operator new?
       // Or should I even stick to the push_back method?
   }

The code above is wrong. You are accessing elements beyond size() in the container. The idiomatic way of doing that would be doing push_back/emplace_back on the container to actually create the objects, rather than only memory:

   for(int i=0; i<20; i++){
       fooVector.emplace_back("Test", i);  // Alternatively 'push_back'
   }

Other than that, in C++11 the type used in the container requires either a copy or move constructor.

answered Jan 27 '14 at 15:47

David Rodríguez - dribeas

204,818
23
294
489

I'm using C++11. Is that why the reserve method is telling me to write a copy constructor? The default one given by the compiler does not work? – Roman Rdgz Jan 27 '14 at 16:05
1

@RomanRdgz: No, the error is telling you that the provided copy constructor is not valid for the use in the container (it cannot take the argument by non-const reference). But after fixing that you would still be causing undefined behavior, you need to fix the *adding elements into the container* problem. – David Rodríguez - dribeas Jan 27 '14 at 16:13
Could you please provide an example of a copy cosntructor? I have tried, but my compiler does not find std::move into – Roman Rdgz Jan 27 '14 at 16:39
1

@RomanRdgz: Option one: don't declare/define it yourself, let the compiler do it. Option two: read the other answers (i.e. add `const` to the argument) – David Rodríguez - dribeas Jan 27 '14 at 16:41
Also, note the difference between `.reserve()` and `.resize()`. The former doesn't really do anything, it certainly doesn't change the `.size()` of the container. It's best to just ignore it for now. `.resize()` will actually change the `.size()` of the container, but I think it will need a copy constructor and default constructor. Anyway, `.emplace_back()` is best, when it can be used. – Aaron McDaid Jul 11 '15 at 17:03

score 5 · Answer 2 · answered Jan 27 '14 at 15:38

5

Change

Foo(Foo &copyFoo) // bah!!! This can't make copies from temporaries

to

Foo(const Foo &copyFoo)  // now that's one good-looking copy constructor

answered Jan 27 '14 at 15:38

Luchian Grigore

253,575
64
457
625

7

Actually, just remove the user-provided copy constructor. – juanchopanza Jan 27 '14 at 15:40
Segmentation fault (both Luchian and juanchopanza suggestions). Anyway that is for my short example; in my real code, the class Foo gives me error: required from 'void std::_Construct(_T1*, _Args&& ---) [with _TI = Foo; _Args = {Foo&}]'. And this is required from the .reserve() line. – Roman Rdgz Jan 27 '14 at 15:44
2

This does not fix the issue of `reserve()`-ing space for the elements, but not adding them into the container and rather just accessing the memory in the buffer. – David Rodríguez - dribeas Jan 27 '14 at 15:48

Mike Seymour · Answer 3 · 2014-01-27T15:57:04.520

The first problem is that your copy constructor takes its argument by non-const reference, which prevents copying from temporaries.

In this case, there's no need to declare a copy constructor at all: if you don't, then one will be implicitly defined to copy each member, which is what you want.

If the class were sufficiently complicated to need a non-default copy constructor, then it should be

Foo(Foo const &);

Note that you don't necessarily need a copy constructor to store a type in a vector; a move constructor is sufficient if you don't want your class to be copyable.

The second problem is that you're accessing uninitialised elements of the vector. reserve just reserves memory for that many elements without initialising them; you need to insert elements (with resize, insert, push_back, emplace_back or whatever) before you can access them.

score 0 · Answer 4 · edited May 23 '17 at 10:31

As noted by Luchian Grigore in his answer, first you should fix your copy constructor. Or just remove it, because the default one will do just fine here. Secondly, to use the code as you have it now you should use resize instead of reserve:

std::vector<Foo> fooVector;
fooVector.resize(20);

The latter only reserves memory, but container's size remains the same - 0 in your case so it's illegal to access elements. resize on the other hand does what you intended.

But in general you shouldn't even do that (as noted here for example). Just use push_back or emplace_back on an empty vector to fill it with contents. resize will call the default constructor for each element you intend to overwrite anyway.

4 Answers4