Why doesn't this create a dangling reference?

Question

I'd think that the VS2019 suggestion would create a dangling reference situation, but I tested it out, and it seems to work. What is happening here?


    template
    class Queue {
      inline static std::vector messages;
    public:
      static bool isEmpty() {
        return messages.size() == 0;
      }

      template 
      static void emplace(Args&&... args) {
        messages.emplace_back(std::forward(args)...);
      }

      static MessageType pop() {
        auto const& val = messages.back();
        messages.pop_back();
        return val;
      }
    };

It looks like the last message stays alive long enough to be copied into the return value. Is this good practice?

user_1294207 · Answer

It looks like the last message stays alive long enough to be copied into the return value. Is this good practice?

Unfortunately, no it doesn't, and no it isn't. The return type of std::vector::back is an lvalue reference. Maybe intellisense thinks it is an rvalue reference, in which case its lifetime would be extended because of the rules here.

But it is not the case for this and the usage here is undefined behaviour. This is because the item in the list to which the reference refers has been destroyed. The reason it may still work is because the memory of the item is still there, and so it can be read correctly. This is just luck (or unluckiness, if you want to be able to find these errors). If the item destroyed by pop_back held other memory then you may see a different result, like a SEGFAULT.

user_225186 · Answer

I think the suggestion is in the right track, and correct but only if you interpret it in a very contrived way.

First, The only thing it is telling you is that .back() returns a reference and it doesn't need to be assigned. Of course that reference can be dangling later and actually it can depend a lot in the specific implementation of std::vector. You don't want that.

To answer your question, yes, the line .pop_back() sometimes creates a dangling reference, depending on the implementation and runtime conditions. Formally, UB in other words.

(from https://en.cppreference.com/w/cpp/container/vector/pop_back)

Iterators and references to the last element, as well as the end() iterator, are invalidated.

Second, you are going to return a copy anyway, so "something" is fishy, in agreement with the IDE suggestion. You don't want a copy (the IDE either), but also you don't want a reference. What you may want is a moved copy, very likely.

This is efficient if the Message can be moved and should be generically correct and the IDE shouldn't suggest any improvement. Worst case it makes a copy, which is the correct behavior.

      static MessageType pop() {
        auto val = std::move(messages.back());
        messages.pop_back();
        return val;
      }

In conclusion, any UB or discussion aside this is the most conceptual expression of what you want I think.

NOTE: I think auto val is better than auto const val because you then allow NRVO. I am not 100% sure.

Why doesn't this create a dangling reference?

2 Answers