user2623008
user2623008

Reputation: 321

Reference to member variable broken after adding another instance to the same vector

Consider the following code:

struct Point {
    int x;
    int& xref = x;
};

int main() {
    std::vector<Point> points;
    for (int i = 0; i < 2; i++) {
        Point p;
        p.x = i;
        points.push_back(p);
        assert (points[0].x == points[0].xref);
    }

    return 0;
}

The assertion fails on the second iteration. Why?

I'm using the GNU C++ compiler. The problem occurs with all of the standards I've tested:

Every element's xref seems to reference the last added element's x instead of its own, as can be seen here: https://pastebin.com/H4wCszxp

Upvotes: 2

Views: 130

Answers (3)

Disillusioned
Disillusioned

Reputation: 14832

I suspect you might have a small misunderstanding of how the declaration int& xref = x; works. It's simply a default initialisation.

  • It does not guarantee that xref it will always be constructed to reference x. The default copy constructor will copy the source instance's xref.
  • Furthermore you can change the xref binding with an initialiser list.

I'll first illustrate with a simplified version of what you're seeing, and follow-up with a modified version of your code. I'll wrap up with a tweak to your Point object that will get it to behave as you expect.

int main()
{
  Point p;
  p.x = 1;
  std::cout << &p.x << "(" << p.x << ") " << &p.xref << "(" << p.xref << ")\n";

  Point p2 = p;
  std::cout << &p2.x << "(" << p2.x << ") " << &p2.xref << "(" << p2.xref << ")\n";
  p2.x = 2;
  std::cout << &p2.x << "(" << p2.x << ") " << &p2.xref << "(" << p2.xref << ")\n";

  Point p3 {3, p2.x};
  std::cout << &p3.x << "(" << p3.x << ") " << &p3.xref << "(" << p3.xref << ")\n";
  return 0;
}

Sample output:

//The in value of p.x and p.xref are in the same location
0x7ffde0450670(1) 0x7ffde0450670(1)
//p2 is initialised as a copy of p
//So p2.x has its own location but value is copied
//and p2.xref refers to the same location as p.xref (and hence p.x)
0x7ffde0450680(1) 0x7ffde0450670(1)
//Therefore updating p2.x doesn't affect p2.xref
0x7ffde0450680(2) 0x7ffde0450670(1)
//All members of p3 are initialised with initialiser list
//p3.x has its own location
//p3.xref is assigned location of p2.x
0x7ffde0450690(3) 0x7ffde0450680(2)

Graphically this looks like:

p-----> +-------------------------+
     +->| x = 1                   |
     |  | xref = 0x7ffde0450670 --|--+
     |  +-------------------------+  |
     |-------------------------------+
     |
     +--------------------------------+
p2-----> +-------------------------+  |
     +-> | x = 2                   |  |
     |   | xref = 0x7ffde0450670 --|--+
     |   +-------------------------+  
     +--------------------------------+
p3-----> +-------------------------+  |
         | x = 3                   |  |
         | xref = 0x7ffde0450680 --|--+
         +-------------------------+  

Modifying your code slightly:

std::vector<Point> points;
for (int i = 0; i < 2; i++) {
    Point p;
    p.x = i;
    points.push_back(p);
    std::cout << i << " " << &p << "\n";
    std::cout << &p.x << "(" << p.x << ") " << &p.xref << "(" << p.xref << ")\n";
    std::cout << &points[0].x << "(" << points[0].x << ") " << &points[0].xref << "(" << points[0].xref << ")\n";
}

Sample output:

0 0x7fffa80e0840
0x7fffa80e0840(0) 0x7fffa80e0840(0)
0x55bd4ff52c30(0) 0x7fffa80e0840(0)
//On each iteration p is in the same location
1 0x7fffa80e0840
//So p.x is in the same location
0x7fffa80e0840(1) 0x7fffa80e0840(1)
//And the vector elements are initialised with p
//So each element's xref is the same as p.x and p.xref
0x55bd4ff52c50(0) 0x7fffa80e0840(1)

Finally, by exercising a little control of the Point constructors, you can get it to behave as you expect.

struct Point {
  int x;
  int& xref = x;
  Point() = default;
  Point(const Point& p): x(p.x) {}
};

The above code does the following:

  • It replaces the default copy constructor with one that copies only the value of x from source. Therefore xref retains its default initialisation.
  • As a consequence of defining your own constructor, the default constructor is removed. So now you need to explicitly declare it; fortunately you can specify that default implementation suffices.

Upvotes: 2

eerorika
eerorika

Reputation: 238311

When you push an object into a vector, you make a copy †.

When you copy a reference (that is the member), the new reference will refer to the same object as the one it was copied from. So, if you copy a Point p into a Point copy, then copy::xref will refer to p.x, because that's what p.xref refers to.

So, all Point objects that are within the vector are copies of the Point p automatic variables that were constructed within the scope of the loop. And therefore all of those objects in the vector refer to the int objects that are the member of the automatic variable p. None of them refer to their own x member.

During first iteration this is fine, because points[0].xref refers to an existing p.x, which also has the same value as points[0].x. But at the end of that iteration, the automatic variable p (whose member the points[0].xref refers to) is destroyed. At this point points[0].xref is a dangling reference that no longer refers to a valid object. In the next iteration the reference is used. Using a dangling reference has undefined behaviour.


If you wish to access this object, then use this pointer. If you want to store a reference to an object, then don't expect a copy of that reference to refer to another object. Avoid storing a reference (or a pointer) to an object that has shorter lifetime than the object that holds the reference.


† ... or make a move when you push an rvalue. You don't push an rvalue, and a copy is exactly the same thing as move for Point), so this is an irrelevant detail.

Upvotes: 5

Qitelia
Qitelia

Reputation: 21

This is happening because when the point is inserted into the vector it get copied, and the reference points to the original value. Eg. this work fine:

int main() {
    Point p;
    p.x = 100;
    assert (p.x == p.xref);

  return 0;
}

Upvotes: 2

Related Questions