Reference to element of vector returned by a function in C++
Asked Answered
D

2

8

Can someone verify that the following is a BUG, and explain why? I think I know, but am unclear about the details. (My actual problem involved a vector of enums, not ints, but I don't think it should matter.) Suppose I have the following code:

std::vector<int> f (void) {
  std::vector<int> v;
  v.push_back(5);
  return v;
}

void g (void) {
  const int &myIntRef = f()[0];
  std::cout << myIntRef << std::endl;
}

Am I correct that myIntRef is immediately a dangling reference, because the return value of f is saved nowhere on the stack?

Also, is the following a valid fix, or is it still a bug?

  const int myIntCopy = f()[0];  // copy, not a reference

In other words, is the return result of f() thrown away before the 0th element can be copied?

Disintegration answered 14/3, 2013 at 13:39 Comment(2)
I'm just curious, but is there ever any reason to use a int const& instead of an int as a local variable?Dualpurpose
@JamesKanze: Maybe to mark the variable as const?Cardin
S
3

Yes, it is wrong thing to do indeed. When you call:

return v;

temporary copy of object v is being created and

const int &myIntRef = f()[0];

initializes your reference with the first element of this temporary copy. After this line, the temporary copy no longer exists, meaning that myIntRef is an invalid reference, using of which produces undefined behavior.

What you should do is:

std::vector<int> myVector = f();
const int &myIntRef = myVector[0];
std::cout << myIntRef << std::endl;

which (thanks to copy elision) uses an assignment operator to initialize myVector object by using v without copy of v being created. In this case the lifetime of your reference is equal to the lifetime of myVector object, making it perfectly valid code.


And to your second question:

"Also, is the following a valid fix, or is it still a bug?"

 const int myIntCopy = f()[0];  // copy, not a reference

Yes, this is another possible solution. f()[0] will access the first element of the temporary copy and use its value to initialize myIntCopy variable. It is guaranteed that the copy of v returned by f() exists at least until the whole expression is executed, see C++03 Standard 12.2 Temporary objects §3:

Temporary objects are destroyed as the last step in evaluating the full-expression (1.9) that (lexically) contains the point where they were created.

Sapsago answered 14/3, 2013 at 13:46 Comment(0)
P
11

That is a bug. At the end of the complete expression const int &myIntRef = f()[0]; the temporary vector will be destroyed and the memory released. Any later use of myIntRef is undefined behavior.

Under some circumstances, binding a reference to a temporary can extend the lifetime of the temporary. This is not one of such cases, the compiler does not know whether the reference returned by std::vector<int>::operator[] is part of the temporary or a reference to an int with static storage duration or any other thing, and it won't extend the lifetime.

Punjabi answered 14/3, 2013 at 13:42 Comment(3)
By the way, I added a follow-up question, if you feel so inclined :-).Disintegration
@user2170028: Answer to your follow-up question is: Yes. See my answer :)Sapsago
@user2170028: This answer already has the solution to your update: at the end of the complete expression, meaning after the assignment. If you make a copy, the copy will get the correct value. Only after the complete expression completes (;) the temporary is destroyedOpisthognathous
S
3

Yes, it is wrong thing to do indeed. When you call:

return v;

temporary copy of object v is being created and

const int &myIntRef = f()[0];

initializes your reference with the first element of this temporary copy. After this line, the temporary copy no longer exists, meaning that myIntRef is an invalid reference, using of which produces undefined behavior.

What you should do is:

std::vector<int> myVector = f();
const int &myIntRef = myVector[0];
std::cout << myIntRef << std::endl;

which (thanks to copy elision) uses an assignment operator to initialize myVector object by using v without copy of v being created. In this case the lifetime of your reference is equal to the lifetime of myVector object, making it perfectly valid code.


And to your second question:

"Also, is the following a valid fix, or is it still a bug?"

 const int myIntCopy = f()[0];  // copy, not a reference

Yes, this is another possible solution. f()[0] will access the first element of the temporary copy and use its value to initialize myIntCopy variable. It is guaranteed that the copy of v returned by f() exists at least until the whole expression is executed, see C++03 Standard 12.2 Temporary objects §3:

Temporary objects are destroyed as the last step in evaluating the full-expression (1.9) that (lexically) contains the point where they were created.

Sapsago answered 14/3, 2013 at 13:46 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.