9

My issue is that I have a class which contains a std::ifstream, std::ofstream, and std::mutex. None of these objects can be directly copied as shown in the example below.

std::ifstream stream1;
std::ifstream stream2;
stream1 = stream2; //<-Compiler Error!

My problem is not that I want to copy any instances of my class, but that the push_back() function in vectors is trying to call the copy constructor for my class. I have designed an example that replicates my issue and pasted it below.

#include <fstream> //for std::ifstream // std::ofstream
#include <vector> //for std::vector
#include <mutex> //for std::mutex

class MyClass
{
public:
    MyClass(int ID) : ID(ID) { }
    std::ofstream outputstream;
    std::ifstream inputstream;
    std::mutex mymutex;
private:
    int ID;
};

int main()
{
    std::vector<MyClass> MyVector;
    MyVector.push_back(MyClass(1)); //<-- Error C2280 'MyClass::MyClass(const MyClass &)': attempting to reference a deleted function

    return 0;
}

I am having issues trying to figure out how to get around this error. Any help would be greatly appreciated. Keep in mind, I will never be directly calling the copy constructor as I have no need in my actual scenario to ever copy an instance of this class, but it seems that push_back is calling it and I have been unsuccessful in my attempts to override it.

Edit: I think one way to fix it would be to use pointers instead of ifstream,ofstream, and mutex objects but I would prefer to avoid this at all costs.

user2980207
  • 293
  • 1
  • 11

3 Answers3

5

The real problem here is that this class contains a std::mutex member. Which cannot be copied/moved. This means that classes that have mutex members don't like to live inside vectors.

You need a custom copy and/or move constructors, for your class, and implement the appropriate semantics to copy the streams, and figure out what you want to do with the mutex.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • How come `vec.emplace_back(1);` is complaining about the mutex being non-copyable? – qxz Sep 20 '16 at 03:03
  • 1
    Because emplace might need to copy/move the ***existing*** members in the vector. – Sam Varshavchik Sep 20 '16 at 03:03
  • Oh, if it needs to reallocate and expand its capacity, right? – qxz Sep 20 '16 at 03:04
  • Of course. Emplacing is just a minor sideshow. Resizing the vector is the elephant in the room. – Sam Varshavchik Sep 20 '16 at 03:05
  • The explanation as given seems correct. But I don't understand how the error is flagged. Visual C++ complains about the copy constructor, while g++ ([at Coliru](http://coliru.stacked-crooked.com/a/dd1a9417500f41d7)) complains about the move constructor. In the latter case, for placement new expression that shouldn't involve that constructor at all. I don't grok it. – Cheers and hth. - Alf Sep 20 '16 at 03:10
  • The error occurs because `emplace()`, or `push_back()` has to generate the code to resize the array, in the event that the array is full at the time of emplace/push_back. Placement new will be used to move the array values from the old internal buffer to the grown one. Placement new, itself, will not be the issue. The issue is that the placement new will be used to move-construct all values from the old buffer into the new one. g++'s diagnostic is correct. Why VC++ is yammering about a copy-constructor, who knows... – Sam Varshavchik Sep 20 '16 at 03:15
  • Without having dug into `gcc`'s innards: the compiler is going to start generating the move constructor for `MyClass` first. It's going to start cranking out the default one, which will try to move-construct each member, one at a time. It'll barf when it gets to the mutex. So, the diagnostic will be inside a move constructor. – Sam Varshavchik Sep 20 '16 at 03:19
  • 1
    @Cheersandhth.-Alf [A defaulted move constructor that's defined as deleted is ignored by overload resolution](https://timsong-cpp.github.io/cppwp/class.copy#11) (so that it doesn't interfere with a viable copy constructor). Since OP's code implicitly defaults the move constructor, MSVC's overload resolution picks the copy constructor and then complains about that. GCC uses a slightly different strategy to implement the same (it makes such a function worse than any non-deleted function) "for better error messages". – T.C. Sep 20 '16 at 18:56
  • Gets no argument from me: it was definitely a "better error message". – Sam Varshavchik Sep 20 '16 at 21:46
4

You might try using a vector of pointers to your objects. It is better to use some kind of smart pointer, rather than a vector of raw pointers.

i.e. std::vector< std::shared_ptr< MyClass >> MyVector;

now you have a container of pointers to MyClass objects.

  • Further to this: the type element T "must meet the requirements of CopyAssignable and CopyConstructible" (until c++ 11). Since c++ 11 this isn't always the case but "many member functions impose stricter requirements", such as push_back & emplace_back. – Harry de winton Jul 10 '21 at 10:49
3

Since you have access to C++11 features, you could use emplace_back to construct the element in place.

MyVector.emplace_back(1);

However, there is some problem with std::mutex. This member has to be a pointer in order for the above to work. Otherwise, you could change all the elements of the class to be types that are copyable (e.g., pointers).

Robert Prévost
  • 1,667
  • 16
  • 22