1

I have the following setup:

struct Frame {
    Frame(vector<Region> regions_)
        : regions(regions_){}
    const vector<Region> regions;
};

Now, at a different part in my code I want to create a vector<Frame> and created the following for loop:

vector<Frame> shot;
Frame currentFrame = generateRandomFrame();
for (int i = 0; i < length; i++) {
    shot.push_back(currentFrame);
    currentFrame = generateNextFrame(currentFrame); // this line gives the error
}

where generateNextFramehas the following signature: Frame FrameGenerator::generateNextFrame(Frame previousFrame)

Now, this code won't compile and give me the following error:

copy assignment operator of 'Frame' is implicitly 
deleted because field 'regions' has no copy assignment 
operator const vector<Region> regions;

Now, I don't fully understand this error message. I strongly assume that it's related to the fact that currentFrame is allocated on the stack and not on the heap, and thus I can't just reassign the variable. Being a novice to C++ however, I am not familiar with how to handle these kinds of situations. Should I use pointers here instead and try to allocate currentFrame on the heap?

To be clear, my goal is to generate a series of frames (that depend on some previous frame). Can anyone point me into the right direction here?

Update: Thanks a lot for all the hints in the comments, I now understand that the issue comes from the fact that I declare regions as const. I rewrote the code to use pointers instead of heap variables, it now looks like this:

vector<Frame> shot;
Frame currentFrame = generateRandomFrame();
Frame *currentFramePtr = &currentFrame; // the first frame in the shot is random
for (int i = 0; i < length; i++) {
    shot.push_back(*currentFramePtr);
    Frame tmpFrame = generateNextFrame(*currentFramePtr);
    currentFramePtr = &tmpFrame; 
}

This code does compile now, however it still doesn't do what I want. According to my understanding it should now work, because I am storing the currentFrame in a pointer, so I can easily override it with new objects that I create. But it seems that there still is a bug, the frame generation doesn't work as expected (that is any new frame is generated with 0 regions while the number of regions should be identical to the previous frame). Can anyone see what's wrong with this updated version of my code?

trincot
  • 317,000
  • 35
  • 244
  • 286
nburk
  • 22,409
  • 18
  • 87
  • 132
  • 2
    You can't change something that's `const`, so you can't change `currentFrame.regions`. – user253751 Apr 01 '15 at 06:42
  • ah ok! I know in C++ it's common to override operators. would it be an option to override the "copy" operator of `Frame`? does that make sense? – nburk Apr 01 '15 at 06:43
  • It's `operator =` that you're calling here. `currentFrame = ...;` calls `currentFrame.operator=(...);` – user253751 Apr 01 '15 at 06:44
  • 4
    If you try to write your own `operator=`, you should see why it's impossible, and therefore why the compiler is complaining that it can't automatically generate one. – user253751 Apr 01 '15 at 06:46
  • It doesn't make sense to assign to something with a const data member, because you'd expect assignment would change the member. – juanchopanza Apr 01 '15 at 06:46
  • Being more explicit: change `const vector regions;` to `vector regions;`. – Tony Delroy Apr 01 '15 at 06:50
  • all right! thanks a lot for the hints everyone! I would like to keep my `regions` as`const`, so what are the options then? is it possible to circumvent this situation using pointers instead of stack variables? – nburk Apr 01 '15 at 06:55

2 Answers2

1

Your struct declares a const member, which forces compiler to implicitly delete default copy assignment operator due to the fact that const member doesn't have copy assignment operator.

The code below replaces the bits you omitted from the sample and shows how to generate a bunch of Frame *s instead of Frames. Basically, it's a workaround for your compilation issue, but it may not suit your specific need if your code REQUIRES stack usage or you'd have to refactor too much.

#include <iostream>
#include <vector>

struct Region{};

struct Frame {
    Frame(std::vector<Region> regions_)
    : regions(regions_){}
    const std::vector<Region> regions;
};

int main(int argc, const char * argv[]) {

    std::vector<Frame *> shot;
    Frame * currentFrame = new Frame((std::vector<Region>()));
    for (int i = 0; i < 10; i++) {
        shot.push_back(currentFrame);
        currentFrame = new Frame(std::vector<Region>());
    }

    return 0;
}

Also please note: how to use const_cast? - const_cast won't work for you and will cause UB. Just in case ;)

Community
  • 1
  • 1
pdeschain
  • 1,411
  • 12
  • 21
  • thanks a lot for the answer! do you maybe have an advice how I can reach my goal without declaring `regions` as non-`const`? – nburk Apr 01 '15 at 07:04
0

If you absolutely insist on keeping regions const, and having "assignment", you can destroy the existing Frame and create a new one in place.

std::vector<Frame> shot;
Frame currentFrame = generateRandomFrame();
for (int i = 0; i < length; i++) {
    shot.push_back(currentFrame);
    currentFrame.~Frame();
    new (&currentFrame) Frame(generateNextFrame(shot.back()));
}

However an even easier way would be to not reassign anything

std::vector<Frame> shot;
shot.push_back(generateRandomFrame());
for (int i = 1; i < length; i++) {
    shot.push_back(generateNextFrame(shot.back()));
}
Caleth
  • 52,200
  • 2
  • 44
  • 75