Returning a vector by value into a reference

Question

I have the following code:

std::vector<Info*> filter(int direction)
{
    std::vector<Info*> new_buffer;
    for(std::vector<Info*>::iterator it=m_Buffer.begin();it<m_Buffer.end();it++)
    {
        if(((*it)->direction == direction)
        {
            new_buffer.push_back(*it);
        }
    }

    return new_buffer;
}

std::vector<Info*> &filteredInfo= filter(m_Direction);

Can someone explain what is happening here ? Would the filter method return by value create a temporary and filteredInfo never gets destroyed because its a reference ?

Not sure if I understand correctly. What is the diference between filteredInfo being a reference and not being one in this case ?

OK. Do note that this is non-standard code. MSVS allows this but this will not work on most other compilers. — NathanOliver, May 18 '16 at 21:21

score 6 · Accepted Answer · edited May 23 '17 at 12:31

6

Your compiler should complain of that code.

This statement:

std::vector<Info*> &filteredInfo= filter(m_Direction);

is a bad idea where filter is:

std::vector<Info*> filter(int direction);

You are trying to create a reference to a temporary object. Even if it succeeds with your compiler, its illegal.

You should use:

std::vector<Info*> filteredInfo= filter(m_Direction);

Its as efficient as you want. Either a move operation (C++11) will happen there or Return Value Optimization will kick in. For your implementation of filter, it should be RVO on optimized builds (it depends on your compiler quality though) .

However, you should note that you are copying raw pointers into your vector, I hope you have a correct ownership model? If not, I advice you to use a smart pointer.

edited May 23 '17 at 12:31

Community

1
1

answered May 18 '16 at 21:21

WhiZTiM

21,207
4
43
68

Can this cause a memory leak ? when using the reference to a temporary ? – Adrian May 18 '16 at 21:24
@Kobe Not a memory leak but certainly UB if you use that reference. – Fantastic Mr Fox May 18 '16 at 21:26
MSVC (Visual Studio) does something well define here, but it is indeed compiler dependant and not standards comformant – OMGtechy May 18 '16 at 22:42

score 2 · Answer 2 · answered May 18 '16 at 21:30

Here is what happens:

std::vector<Info*> new_buffer; creates an object locally.
return new_buffer; moves new_buffer to a temporary object when filter(m_Direction) is called.
Now if you call std::vector<Info*> filteredInfo= filter(m_Direction); the temprary object will be moved to filteredInfo so there is no unnecessary copies and it's the most efficient way.
But, if you call std::vector<Info*> &filteredInfo= filter(m_Direction); then filteredInfo is bound to a temporary object, which is a terrible idea and most compilers will complain about this.

score 2 · Answer 3 · edited May 23 '17 at 11:44

Here you're correctly puzzled because there are two independent weird facts mixing in:

Your compiler allows a non-const reference to be bound to a temporary. This historically was a mistake in Microsoft compilers and is not permitted by the standard. That code should not compile.
The standard however, strangely enough, actually allows binding const references to temporaries and has a special rule for that: the temporary object will not be destroyed immediately (like it would happen normally) but its life will be extended to the life of the reference.

In code:

std::vector<int> foo() {
    std::vector<int> x{1,2,3};
    return x;
}

int main() {
    const std::vector<int>& x = foo(); // legal
    for (auto& item : x) {
        std::cout << x << std::endl;
    }
}

The reason for this apparently absurd rule about binding const references to temporaries is that in C++ there is a very common "pattern"(1) of passing const references instead of values for parameters, even when identity is irrelevant. If you combine this (anti)-pattern with implicit conversion what happens is that for example:

void foo(const std::string& x) { ... }

wouldn't be callable with

foo("Hey, you");

without the special rule, because the const char * (literal) is implicitly converted to a temporary std::string and passed as parameter bound to a const reference.

(1) The pattern is indeed quite bad from a philosophical point of view because a value is a value and a reference is a reference: the two are logically distinct concepts. A const reference is not a value and confusing the two can be the source of very subtle bugs. C++ however is performance-obsessed and, especially before move semantics, passing const references was considered a "smart" way of passing values, despite being a problem because of lifetime and aliasing issues and for making things harder for the optimizer. With a modern compiler passing a reference should be used only for "big" objects, especially ones that are not constructed on the fly to be passed or when you're actually interested in object identity and not in just object value.

Returning a vector by value into a reference

3 Answers3