CODEWITHSUNDEEP
c++templatesc++11
Zizheng Tai
Zizheng Tai

Reputation: 6636

Return by universal reference

std::for_each accepts and returns a functor by value:

template< class InputIt, class UnaryFunction >
UnaryFunction for_each( InputIt first, InputIt last, UnaryFunction f );

Although the functor can be moved in and moved out, I'm interested whether there can be no object construction involved at all. If I declare my own my_for_each like this:

template< class InputIt, class UnaryFunction >
UnaryFunction&& my_for_each( InputIt first, InputIt last, UnaryFunction&& f);

And inside my_for_each, call f with std::forward<UnaryFunction>(f)(...), I can avoid the cost to move construct the parameter, and as a bonus be able to respect ref-qualifiers. But I'm not really sure what I should return. If I do:

return std::forward<UnaryFunction>(f);

Can bad things (e.g., dangling references) happen?

(This question comes up when I'm designing the for_each in this post.)

Upvotes: 7

Views: 2572

Answers (2)

Yakk - Adam Nevraumont
Yakk - Adam Nevraumont

Reputation: 275840

As the other fine answer noted, passing const& and rvalue reference parameters through is dangerous, as reference lifetime extension does not commute.

The proper thing to return when you want to pass a forwarding reference T&& through is T. This turns an rvalue into a temporary, and leaves lvalues as references.

So:

template< class InputIt, class UnaryFunction >
UnaryFunction my_for_each( InputIt first, InputIt last, UnaryFunction&& f);

Which for temporary f creates a (moved-to) copy.

If you store this copy, it will be elided into the storage, so zero additional cost. Unless you do not store it, and move is expensive, this is basically optimal.

Upvotes: 8

Nir Friedman
Nir Friedman

Reputation: 17714

Bad things can and will happen.

struct my_functor {
  void operator()(int x) { sum += x; }
  int sum = 0;
}

std::vector<int> v{1,2,3};

const auto& s = my_for_each(v.begin(), v.end(), my_functor{});
auto sum = s.sum;

This code is undefined behavior. You construct a temporary instance of my_functor. This binds to the rvalue reference, extending its lifetime for now, as the function call my_for_each enters. When the function exits, it will return an rvalue reference to the temporary. However, when the function exits, the rvalue reference that the temporary originally bound to will be destroyed, because it's a function local. At that point, the temporary will be destroyed. The net effect of all this is that s is immediately a dangling reference.

Note that with the original for_each, the functor would have been returned by value, and the reference would have bound to it directly, extending its lifetime.

Herb Sutter talks about this in this cppcon talk: https://www.youtube.com/watch?v=hEx5DNLWGgA. Basically, when you return a reference/pointer from a function, there's very few safe options as to what it can actually point to. Function arguments that take by reference are one of them, but functions arguments by const ref and rvalue ref are not because they attract temporary and cause the return of the function to dangle. I've written about this topic here: http://www.nirfriedman.com/2016/01/18/writing-good-cpp-by-default-in-the-stl/.

If you want to take a functor and avoid object construction, I would simply take by forwarding reference and return nothing. Why? Because, if the user doesn't need the functor afterwards, they can just pass an rvalue and not worry about misusing the return. And if they do need it after, they can just construct it, pass it as an lvalue, and then use it after.

template< class InputIt, class UnaryFunction >
void my_for_each( InputIt first, InputIt last, UnaryFunction&& f);

my_functor s{};
my_for_each(v.begin(), v.end(), s);
auto sum = s.sum;

No construction/destruction, no problems. Though I would warn that if you are trying to avoid this for performance reasons, it's probably misguided unless your functor/lambda is large, which is unusual.

Upvotes: 4

Related Questions