2

Here's the code I have:

#include <iostream>
#include <functional>

struct Test {
    struct Envelope {
        const int x = 1;
        int y = 2;
        int z = 3;
    };

    Envelope mEnvelope;

    struct Buffer {
        Envelope mEnvelope;
    } mBuffer;
    std::function<Buffer()> func{[this] {
        mBuffer.mEnvelope = mEnvelope;

        return mBuffer;
    }};    
};  

int main() {   
    Test test;
}

it says:

g++ -std=c++17 -Wall -pedantic -pthread main.cpp && ./a.out
main.cpp: In lambda function:
main.cpp:17:29: error: use of deleted function 'Test::Envelope& Test::Envelope::operator=(const Test::Envelope&)'
   17 |         mBuffer.mEnvelope = mEnvelope;
      |                             ^~~~~~~~~
main.cpp:5:12: note: 'Test::Envelope& Test::Envelope::operator=(const Test::Envelope&)' is implicitly deleted because the default definition would be ill-formed:
    5 |     struct Envelope {
      |            ^~~~~~~~
main.cpp:5:12: error: non-static const member 'const int Test::Envelope::x', can't use default assignment operator

I've tried using a Copy Constructor:

Envelope(const Envelope &other) {
    y = other.y;
}

or override the operator =

Envelope &operator=(Envelope &other) {
    // self-assignment guard
    if (this == &other) {
        return *this;
    }            

    y = other.y;
}

But the errors grown even more.

I need to copy only some "part" of the object. This is just a test, of course the real object have lots of members fields, and some need to be ignored.

How to do it within a std::function<Buffer()>?

markzzz
  • 47,390
  • 120
  • 299
  • 507

2 Answers2

2

OK, since it might not be obvious what the problem is:

  1. A copy assignment operator should usually have a const& parameter, not a & parameter.

  2. You should then also provide the copy constructor of Envelope as you showed. First of all it has different behavior than the implicitly-generated one (which would copy over all the elements, not only y) and the generation of the implicit copy constructor when there is a user-defined copy assignment has been deprecated since C++11 and will probably be removed in a future standard iteration.

  3. Then you will need to default the default constructor as well, since you have a user-defined constructor now:

    Envelope() = default;

  4. Furthermore, your copy assignment operator is declared to return a Envelope& (as it should), but you forgot to actually put a return statement in it at its end, so executing it will cause undefined behavior as is:

    return *this;
walnut
  • 21,629
  • 4
  • 23
  • 59
  • But do I need both operator `=` and copy ctor? that's what I don't understand – markzzz Apr 08 '20 at 15:40
  • @markzzz This might help you: https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom and as I mentioned the copy constructor is not stictly required (yet), but you will need to implement it if you want the specific semantics that only `y` is copied and the other two members are defaulted. – walnut Apr 08 '20 at 15:50
  • check this http://coliru.stacked-crooked.com/a/3a8742f179b0baad . It seems that I need to copy the data on both operator and copy ctor for work (remove the comment on `//y = other.y; // THIS SEEMS NEEDED`). The fact is: if I copy 20 variables, I need to copy 20 + 20 on both copy ctor and operator. Is that normal? – markzzz Apr 08 '20 at 15:58
  • @markzzz Yes, obviously you must make the copy constructor actually copy the parts that you want to copy. I assumed that you explicitly do not want to copy certain parts. If you just want the copy constructor to copy all members, you can default it explicitly: `Envelope(const Envelope&) = default;`. The copy assignment must be specified in full, defaulting it will not be enough. – walnut Apr 08 '20 at 16:03
  • General side note: If `x` is supposed to *always* be `1` without a way of initializing it to something else, then you could simply make it `static`, so that you won't need to define any of the special member functions yourself. – walnut Apr 08 '20 at 16:04
  • @markzzz Yes, that is normal. And because that are too many to do manually, I usually factor those where the defaulted copy constructor/assignment is sufficient into a helper `struct` that is then used as base class in the actual class. – j6t Apr 08 '20 at 16:04
  • I just want to copy some prevent to copy all data, which I don't need and code must be faster. Not clear why it used both operator and Copy ctor. Shouldn't It use one or the other? – markzzz Apr 08 '20 at 16:11
  • Ok, got it why: i use both because I do two different copy. I'll try to review my code, to do it once... – markzzz Apr 08 '20 at 16:16
1

You can create your own copy ctor/operator in order to only copy the info you want:

#include <iostream>
#include <functional>

struct Test {
    typedef struct  {
        const int x;
        int y;
        int z;
    } Envelope_t;

  public:
    Test():env({1,2,3}){}

    Test(const Test & copy):env({copy.env.x,5,copy.env.z}) {}

    Test& operator=(const Test& copy){   
      env.y=copy.env.y+7;
      env.z=copy.env.z;
      return *this;
    }

    void printValues() {
        std::cout << "x:" << env.x << "\ny:" << 
                     env.y << "\nz:" <<  env.z << "\n\n";
    }

  private:
    Envelope_t env;
};  

int main() {   
  Test original;
  original.printValues();

  Test copyCtor(original);
  copyCtor.printValues();

  Test copyOp;
  copyOp = copyCtor;
  copyOp.printValues();
}
Jose
  • 3,306
  • 1
  • 17
  • 22