On returning references to local objects and subscripting operator.

[VoB]

@helios

In your last comment, you're referring to line 4 of my last snippet, where I used the std::vector, right? Or to my Vector class?

If you push_back() while size() == capacity(), all references into the vector are invalidated

That is because in principle you need to allocate with new (I know that std::vector uses something different), move the old elements to the new one, and reset the "elem" pointer, right?

[helios]

you're referring to line 4 of my last snippet, where I used the std::vector, right? Or to my Vector class?

The snippet in question uses std::vector, so obviously I'm referring to that class.

That is because in principle you need to allocate with new (I know that std::vector uses something different), move the old elements to the new one, and reset the "elem" pointer, right?

Not sure what you mean by "in principle", but yes. Every object in the vector has to be moved into a new memory region. In theory a vector implementation could reallocate the internal buffer and avoid the invalidation, but the standard doesn't require implementations to do this, so in practice none of them do it.

[dhayden]

VoB, I want to stress something in all this: don't just try something and see if it "works." Often times code has undefined behavior but a particular test will happen to generate the expected result. This stuff with a vector is a good example. The vector might reallocate and the old data might be left unchanged. That does not mean that the code is valid or that it will work every time. It just means that you got lucky.

Make sure your code works by design, not by accident.

[jonnin]

^^ I second this. I was taught, in school, by a crusty professor that it was acceptable and just fine to return a reference to a temporary in a function -- and it worked back then, more often than not (I can't recall it ever going wrong in schoolwork type problems). It STILL works just fine in a lot of code ... until it doesn't work and you have a debugging nightmare. Back then, the cpu and os designs made it mostly work if you consumed the temporary immediately after the function was called; so even when it worked you had a fragile design. By the time I was into my first job, it was painfully clear that doing this was high risk and that the professor was flat out wrong. That is neither here nor there, the point is Dhayden's point: things like this can be flat out wrong and still WORK just fine ... until the day it does not work and you can't figure out why. You don't want to be in that situation, trust me.

[seeplus]

Seconded. In C++ just because 'something seems to work' in one/couple of cases doesn't mean it is working correctly and will always work correctly - especially in a 'live' environment instead of a development environment.

[dhayden]

jonnin wrote:

it worked back then, more often than not

Sometimes a lot more often than not. I think there was an intel processor that actually had a bug in one of the assembly instructions whereby it popped the stack and then read the value that used to be on top. This worked most of the time. A lot of the time. Every single time they tested it. But it was still wrong.

They hadn't considered what happened if an interrupt occurred between the "pop" and the "read".

If interrupts use the same stack as user code, then it is never safe to read on the "far side" of the stack pointer unless interrupts are disabled. Otherwise you get a once-in-a-billion bug that's nearly impossible to track down.

[George P]

Undefined behavior can be a real killer at times, such as returning a reference or pointer to a temporary in a function. Run a couple of test cases and "it seems to work." Sign off and you ship the bomb.

Then customer complaints start coming. And coming and coming and coming......

I had a simple test program with a struct containing std::strings instead of C strings, writing to/reading from a binary file. It worked for the test samples I tried, and I believed it was bullet proof.

Until I tried the code sample with a couple of different data sets a couple of years later, and *BLOOMPH!*

I learned the hard way serialization isn't all that simple.

[VoB]

Thanks @dhayden for this comment

Make sure your code works by design, not by accident.

I got the message.

Thanks also to all the others for sharing their experience about undefined behaviour :-)

[George P]

VoB, if you are the person doing the reporting, STOP. If you don't like what others have to say, fine. The Report button is not a "I don't like the answer" button.

[VoB]

@FurryGuy I know what is a report and, as you can see, also my last comment has been reported. I don't know who and why is reporting :/

[MikeyBoy]

helios wrote:

I'm going to disagree with MikeyBoy, here. The reference is invalid. Using it in any way after the push_back() leads to undefined behavior, as per the std::vector specification. Since the behavior is undefined, there's little point in trying to reason about the program's behavior after line 5, since all behaviors are permissible.

To be clear, I said that if the value of the elem pointer is changed, and the memory it was previously pointing to is not freed, then the reference is merely useless; but that if that memory is freed, then it is properly invalid. (EDIT: And I'm not claiming that's a particularly important distinction - in both cases, it causes the class to function incorrectly, and needs to be fixed).

This was in the context of VoB's custom Vector class - where, in the code we'd been shown, there was no freeing of the the old memory (which, incidentally, means it had a memory leak).

In the context of a std::vector, then the reference should always be treated as invalid.

So I believe we're in total agreement here.

If I missed a point where the code I was commenting on changed from using VoB's own Vector to using std::vector, then that was my bad.

[helios]

Fair enough.