2

I have a problem very similar to

How to make a class with a member of unique_ptr work with std::move and std::swap?

and I am using VS2013 so I expect the following to work.

#include <memory>
#include <string>

class AbstractCamera{
public:
    AbstractCamera(const std::string& name) :_name(name){}
    virtual void method() = 0;
    std::string _name;
};

class CameraImpl : public AbstractCamera{
public:
    CameraImpl() :AbstractCamera("CameraImpl"){}
    void method(){}
};

class RenderManager{
public: 
    RenderManager():_currentCamera(std::move(std::make_unique<CameraImpl>())){}
private:
    std::unique_ptr<AbstractCamera> _currentCamera;
};

class Engine{
public: 
    Engine(){}
private:
    RenderManager r;
};

int main(){
    Engine e;
    e = Engine(); // Causes error: C2280 call of deleted function
}

path\engine.cpp(75): error C2280: 'std::unique_ptr> &std::unique_ptr<_Ty,std::default_delete<_Ty>>::operator =(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)' : attempting to reference a deleted function with [ _Ty=AbstractCamera ] other_path\memory(1487) : see declaration of 'std::unique_ptr>::operator =' with [ _Ty=AbstractCamera ] This diagnostic occurred in the compiler generated function 'RenderManager &RenderManager::operator =(const RenderManager &)'

I do get that simple examples like

std::unique_ptr<A> a = std::make_unique<A>();
std::unique_ptr<A> b = a;

are not allowed but my problem is the following code:

Engine e;
e = Engine();

because the assignment operator of unique_ptr is deleted but how does this affect a class hierarchy like those of Engine->RenderManger->unique_ptr member.

I do get, that Engine e uses the default constructor Engine() and e = Engine() calls the default constructor and the operator= of Engine to assign the temporary Engine object to e.

My question therefore is: where does the code try to copy/assign unique_ptr and how do I resolve it?

I tried to strip the original code down as much as possible but was not able to reproduce the error using a simpler example and ideone in form of a SSCCEE as I do not really understand what causes the problem, so sorry for that!

Community
  • 1
  • 1
Kevin Streicher
  • 454
  • 1
  • 6
  • 21
  • Yes. Sorry, this is one of the typos from the minimization of the original code as pointed out by T.C. Thank you for pointing this typo out, so I could correct it, s.t. the original question contains one error less! – Kevin Streicher Feb 20 '15 at 19:31

3 Answers3

3

VS2013 does not automatically generate move special member functions (this is non-conforming, in case it isn't obvious). You'll have to write the move constructor and move assignment operator of Engine and RenderManager yourself, as otherwise the copy special member functions will be used.

As a side note, _currentCamera(std::move(std::make_unique<CameraImpl>)) won't compile; _currentCamera(std::move(std::make_unique<CameraImpl>())) would but the std::move is useless and a pessimization. make_unique already returns an rvalue. There are also several other typos in your code, presumably caused by the minimization process.

T.C.
  • 123,516
  • 14
  • 264
  • 384
  • Thank you very much and yes, sadly you are right there are typos because of the minimization. I will try to correct them in the original post for people who read this later. I will try this now! Thank you for the fast response! Could explain what you mean with: `and a pessimization`, simply "uncessary"? – Kevin Streicher Feb 20 '15 at 19:28
  • 1
    @NoxMortem It inhibits copy elision. – T.C. Feb 20 '15 at 19:41
  • oh, ouch. Thank you very much for pointing that out, this is some small but important information. – Kevin Streicher Feb 20 '15 at 19:53
0

T.C. correctly answered the question, but I wanted to give some further insights as I did actually asked the wrong question :)

The real question therefore should have been

How can I use the following code

Engine e;
e = Engine();

when my intent is the destruction of the default constructed object e and the assignment of the newly constructed object.

First of all one needs to understand the difference between copy and move assignment and construction and the following thread and especially the two long posts by @FredOverflow here can help very much to understand this: What are move semantics?

The problem setting are actually classes with std::vectors<std::unique_ptr<T>> and classes with std::unique_ptr<T> members. See http://www.cplusplus.com/forum/beginner/110610/ and the answer of JLJorges for a longer example.

JLJorges posted:

So don't do anything: by default B is not Copyable or Assignable, but B is Moveable and MoveAssignable

JLJorges pointed out two important things under such circumstances:

  • A class containing a non-copyable member can not be copyable by default.
  • A class containing a non-copyable but moveable/moveAssignable member can still be moveable/moveAssigneable by default.

VS2013 therefore is able to implicitly generate move and move assign operators/constructors as it does exactly this for such a class setup.

The reason I did not select T.C.'s answer altough he was very helpful and close to the answer is the following:

In case one needs copy-able/copy-assignable classes he has to implement copy-constructor and copy-assignment. This would have been the answer if I would have needed exactly what the code in the initial question looked like: a copy/copy-assignable class Engine.

In case one implements one of those methods/operators he might want to implement the others as well, see the following links for explanations:

As I said, I had the asked the wrong question, as I did not needed a copyable/copyassignable class the solution would have been to implement move constructor and move assign operator or like JLJorges pointed out: do nothing and use the implicitly generated version, but when I wanted to move I obviously had to tell the compiler this:

Engine e;
e = std::move(Engine());
Community
  • 1
  • 1
Kevin Streicher
  • 454
  • 1
  • 6
  • 21
-1

e = Engine() calls Engine's move assignment operator. Since you don't have one, it's copy assignment operator will be called. Then RenderManger's copy assignment will be called. Finally it's the copy assignment of unique_ptr, which is deleted.

A move assignment is needed for Engine, as

Engine & operator = (Engine && rhs) { r._currentCamera = std::move(rhs.r._currentCamera); return * this; }

Suppose RenderManager::_currentCamera is visible to Engine, otherwise you need to defind move assignment for RenderManager.

imlyc
  • 84
  • 5