If A std::vector is Returned-By-Value when Is It Freed?

Question

I am having trouble understanding how the C++ std::vector container would get freed in the following scenario

void DoSomething()
{
   cv::Point ** curves;
   int * curve_sizes;
   num_curves = m_curves.size(); //member variable holdings curves
   curves = new cv::Point*[num_curves];
   curve_size = new int[num_curves];

   std::vector<cv::Point> cur_points;
   for(int i = 0; i < num_curves; ++i)
   {
     cur_points = CreatePolyPoints(m_curves[i]);
     curves[i] = &cur_points[0];
     curve_sizes = cur_points.size();
   }

   cv::fillPoly(m_roi, curves, curve_sizes, num_curves, ... );

   //Clear the dynamic data
   // Do i do aything here?   
   delete [] curves;
   delete [] curve_sizes;
}

std::vector<cv::Point> CreatePolyPoints(Curve curve)
{
   std::vector<cv::Point> points;

   //Do work here here
   while(something)
   {
     cv::Point cur_point;
     points.push_back(cur_point);
   }

   return points;
}

Thanks in advance. If anyone is interested in my goal: Generate an ROI given polygons defined by "n" curves.

What is `curve_polygons`? Did you `new[]` it? – Jonathan Grynspan Dec 04 '12 at 20:53 — Jonathan Grynspan, Dec 04 '12 at 20:53

mathematician1975 · Accepted Answer · 2012-12-04T21:23:09.453

4

It is not dynamically allocated so it will be destroyed as soon as it goes out of scope. In your case, your variable will go out of scope after the DoSomething() function exits. Your local variable cur_points gets the return value of CreatePolyPoints() assigned to it within your loop. When this happens, the old contents of cur_points will no longer exist - you do not need to worry about this, as the instances of Curve objects within that vector will get destroyed each time the vector gets reassigned. Similarly when cur_points goes out of scope.

EDIT: You do seem to have a problem here

std::vector<cv::Point> cur_points;
for(int i = 0; i < num_curves; ++i)
{
   cur_points = CreatePolyPoints(m_curves[i]);
   curves[i] = &cur_points[0];    // <-- problem
   curve_sizes = cur_points.size();
}

You assign the address of the vector element 0 to your ith element of curves at each iteration - The problem here is that that will be the address of an object that no longer exists. This is because the contents of the vector will no longer exist once cur_points gets reassigned next time in the loop iteration, therefore the pointers you are storing do not point to valid objects - they point to objects that have been destroyed already. When your loop finished, the vector will contain the contents returned in the last call to CreatePolyPoints. If you want to make sure that all the cv::Point objects still exist at the point you call cv::fillPoly you should consider passing the vector by reference to the CreatePolyPoints() function rather than return a new vector by value each time. This will ensure that ALL items you add via CreatePolyPoints will still exist once your loop finishes

edited Dec 04 '12 at 21:23

answered Dec 04 '12 at 20:52

mathematician1975

21,161
6
59
101

1

Conceptually, the `points` vector gets copied, then the original vector gets destroyed when the `CreatePolyPoints` function is exited, and the caller receives the copy as the return value. However, due to C++ return-value optimization, the function may just pass that vector back to the caller instead of copying it, so it is the caller which will eventually destroy it. – Kristopher Johnson Dec 04 '12 at 20:56
Okay, two related questions. Am I approaching this correctly? Secondly, how do I ensure out of scope? Is the `delete [] curves` sufficient? Do I need to iterate through `curves` and `delete [] curves[i]`? Who handles that? – Constantin Dec 04 '12 at 20:59
@Constantin Those responses you quote do not imply that your `cur_points` variable will exist once `DoSomething()` exits unless I totally misread them. – mathematician1975 Dec 04 '12 at 21:00
No I agree. Your edit clarified well. The points only need to exist within DoSomething(). I have removed my comment. – Constantin Dec 04 '12 at 21:01
@Constantin One thing in your code I noticed is that you create 2 arrays with `new` one an array of `cv::Point*` and the other an array of `int`. You are only deleting one of them - the `int` array is not getting deleted - is that your intention? – mathematician1975 Dec 04 '12 at 21:03
Great, makes perfect sense. Thank you for helping me work through this mathematician. – Constantin Dec 04 '12 at 21:18
Yep, that's exactly where my secondary confusions stemmed from, and I agree with your understanding. – Constantin Dec 04 '12 at 21:26

John Dibling · Answer 2 · 2012-12-04T21:02:42.643

2

cur_points is a variable with automatic storage duration. It comes in to existence when it is declared, here:

std::vector<cv::Point> cur_points;

...and persists as long as it's enclosing scope persists.

Since the "enclosing scope" in this case is the function void DoSomething() itself, cur_points will go out of scope and be destroyed (automatically) when the function returns.

Edit Now, the above is the simple answer. The complete answer is actually a bit more complex because temporaries are also involved.

The function CreatePolyPoints returns this vector by-value. Strictly adhering to the semantics of this function and the return value means that the points object created locally within that function will be destroyed when CreatePolyPoints returns, and a new temporary vector will be constructed from points. That temporary is what's actually returned from the function. This temporary is then copied to your cur_points variable (via copy-assignemnt), and then the temporary is destroyed.

This is a lot of wasteful copying, but there's a bit more to the story. The C++ Standard also has a rule (referred to as the "AS-IF" rule) which states that a compiler can change your code in any way it sees fit, so long as the behavior of your program is the same as if the compiler had made no changes, considering side-effects.

What the above means is that compilers have a major optimization opportunity to eliminate all these temporaries in many cases, including this case, by copying the return directly to the cur_points variable instead of thru a temporary. This optimization in it's most general form is called "Return Value Optimization," and most (all?) modern compilers do it frequently. When this happens (which it will here usually), the behavior is exactly as I laid out before my edit here.

edited Dec 04 '12 at 21:02

answered Dec 04 '12 at 20:55

John Dibling

99,718
31
186
324

What about these responses? http://stackoverflow.com/questions/3703302/c-vector-return-vs-parameter – Constantin Dec 04 '12 at 20:55
@Constantin: Hold on, I'll read your link. Meantime, read my edit please. – John Dibling Dec 04 '12 at 21:03
@Constantin: I'm not sure how the responses you linked to contradict what I've said? Can you please elaborate? – John Dibling Dec 04 '12 at 21:04
I must have misread your question, originally I thought you mentioned that it gets deleted at the end of `CreatePolyPoints`. I completely understand and agree with your analysis. -- Last question, notice that cur_points gets set every loop iteration. FURTHERMORE, notice I cheat with a `&cur_points[0]`. Will that data be cleared at the end of each loop iteration? That is the scope for that "temporary" vector – Constantin Dec 04 '12 at 21:07
@Constantin: When you say "that data," what do you mean, exactly? Unless I'm misunderstanding you, the asnwer is no -- nothing gets "cleared out" within the for loop. No automatics variables are declared within the scope of the loop, and nothing is erased or `delete`-ed in the loop. – John Dibling Dec 04 '12 at 21:13
Each iteration we create a new `std::vector` in `CreatePolyPoints` and return it to the for loop. Inside the for loop, we take the first element of that new std::vector and store it in one of the curves `curves[i] = &cur_points[0]`. Later we use `curves` which needs to be an array of `cv::Point` arrays, in `cv::fillPoly`. – Constantin Dec 04 '12 at 21:15
@Constantin: Neihter `cur_points` nor `curves` are cleared out at the iteration of the loop. – John Dibling Dec 04 '12 at 21:20

score 0 · Answer 3 · answered Dec 04 '12 at 21:29

0

Here is an example of DoSomething that does not leak, and with bugs removed:

void DoSomething()
{
  std::vector<std::vector<cv::Point>> curves;
  // so long as I do not resize this, or change its length,
  // curves[x].data() will remain valid
  curves.resize( m_curves.size() );

  for(int i = 0; i < num_curves; ++i)
  {
    curves[i] = CreatePolyPoints(m_curves[i]);
  }
  // create buffers of the correct size for the legacy API:
  std::vector<v::Point*> curve_ptrs(curves.size());
  std::vector<int> curve_sizes(curves.size());
  for(int i = 0; i < curves.size(); ++i)
  {
    curve_ptrs[i] = curves[i].data();
    curve_sizes[i] = curves[i].size();
  }

  // call the legacy API:
  cv::fillPoly(m_roi, curve_ptrs.data(), curve_sizes.data(), num_curves, ... );
}

answered Dec 04 '12 at 21:29

Yakk - Adam Nevraumont

262,606
27
330
524

Yakk, thank you so much for the C++ style fillPoly approach. You mention legacy API? Which part is legacy? – Constantin Dec 04 '12 at 21:32
`cv::fillPoly` takes an array of lengths and an array of pointers to buffers. I would describe that as a legacy-style API. – Yakk - Adam Nevraumont Dec 04 '12 at 21:47
AH I understand, is there a cleaner way to approach it? – Constantin Dec 04 '12 at 22:32
Yes. Take a const reference to a std vector of std vector of points for example. – Yakk - Adam Nevraumont Dec 05 '12 at 00:09

If A std::vector is Returned-By-Value when Is It Freed?

3 Answers3