Question

I have a struct that i want to be non copyable, only movable, but as it contains a lot of POD, writing move constructor would be long and forgetting a variable would be hard to debug. Example:

struct myStruct{
    int a,b,c,d;
    double e,f,g,h;
    std::complex<double> value1,value2;

    std::unique_ptr<Calculator> calc;

    myStruct(){}
    myStruct(const myStruct &)=delete;
    myStruct(myStruct && other);
};

What would be the problems with this kind of move constructor:

myStruct::myStruct(myStruct && other){
    std::memcpy(this,&other,sizeof(myStruct));
    other.calc.release();
    calc->rebind(this);
}

What problems could I face and is this well defined?

Était-ce utile?

La solution

The minimal change is just to group the trivially-initialized members together, so you can memcpy them easily:

struct myStruct{
    struct {
        int a,b,c,d;
        double e,f,g,h;
        std::complex<double> value1,value2;
    } pod;

    std::unique_ptr<Calculator> calc;

    myStruct(){}
    myStruct(const myStruct &)=delete;
    myStruct(myStruct && other);
};

myStruct::myStruct(myStruct && other){
    std::memcpy(&pod,&other.pod,sizeof(pod));
    other.calc.release();
    calc->rebind(this);
}

Note std::complex is a literal type, which should be safe to put into the pod member. If you add any other member objects of class type, you'll have to verify yourself that they're safe to memcpy.


A better implementation would, as Jonathan Wakely pointed out, sidestep the concerns about pod and non-pod (or literal, and trivially-initialized) members. Instead, group members by whether you want them copied or moved:

struct myStruct{
    struct {
        int a,b,c,d;
        double e,f,g,h;
        std::complex<double> value1,value2;
    } val;

    std::unique_ptr<Calculator> calc;

    myStruct(){}
    myStruct(const myStruct &)=delete;
    myStruct(myStruct && other);
};

myStruct::myStruct(myStruct && other)
  : val(other.val)              // copy the value types
  , calc(std::move(other.calc)) // and move the reference types
{
    calc->rebind(this);
}

Autres conseils

You can use the default move Ctor:

myStruct(myStruct&& other) = default;

Edit: After the recent edit I prefer the accepted answer from @Useless. The following is left to demonstrate an alternative approach that allows the move constructor to be defined as defaulted, but which is over-engineered compared to the accepted answer.

Using memcpy is a bad idea. I might consider creating a helper type that does the non-trival work (moving the unique_ptr and rebinding the calculator) and use that in myStruct, so that its move operations can be defaulted:

struct RebindableCalc
{
  RebindableCalc();

  RebindableCalc(RebindableCalc&& r) noexcept : calc(std::move(r.calc))
  { calc->rebind(self()); }

  RebindableCalc& operator=(RebindableCalc&& r) noexcept
  {
    calc = std::move(r.calc);
    calc->rebind(self());
    return *this;
  }

  std::unique_ptr<Calculator> calc;

  myStruct* self();
};

struct myStruct : RebindableCalc
{
    int a,b,c,d;
    double e,f,g,h;
    std::complex<double> value1,value2;

    myStruct() = default;
    myStruct(myStruct&& other) = default;
    myStruct& operator=(myStruct&& other) = default;
};

inline myStruct* RebindableCalc::self()
{
  return static_cast<myStruct*>(this);
}

This will let the compiler generate the optimal code for the POD members, and still do the right thing for the unique_ptr<Calculator> member. No memcpy needed. If you add more members to myStruct the move ops will still do the right thing.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top