Problem
As I understand it, when a std::unique_ptr is returned from a function into an rvalue, its lifetime should encompass the statement that consumes that rvalue. But compiling with gcc 6.4.1, the return value from Foo::iterator() goes out of scope before the start of the C++11 foreach statement in function crashing_version(). As shown in the output below, the destructor is called just after the containing expression is evaluated. Is this a bug in gcc, or bad programming practice?
Use Case
The goal of this pattern is to make iteration available without exposing the private vectors. This seems to require some object like Foo::Iterator because there are two separate lists to iterate.
#include <iostream>
#include <memory>
#include <vector>
class Foo {
/* Goal: allow iteration without exposing the vector objects. */
std::vector<int> _list0;
std::vector<int> _list1;
public:
class Iterator {
int _list_id;
Foo& _foo;
public:
Iterator(int list_id, Foo& foo) : _list_id(list_id), _foo(foo) {}
~Iterator() {
std::cout << "~Iterator(): Destroying iterator of the "
<< (_list_id == 0 ? "even" : "odd") << " list\n";
}
std::vector<int>::iterator begin() {
if (_list_id == 0)
return _foo._list0.begin();
else
return _foo._list1.begin();
}
std::vector<int>::iterator end() {
if (_list_id == 0)
return _foo._list0.end();
else
return _foo._list1.end();
}
};
void add(int i) {
if ((i % 2) == 0)
_list0.push_back(i);
else
_list1.push_back(i);
}
std::unique_ptr<Iterator> iterator(int list_id) {
return std::make_unique<Iterator>(list_id, *this);
}
};
void working_version() {
Foo foo;
for (int i = 0; i < 10; i++)
foo.add(i);
/* This works because the unique_ptr stays in scope through the loop. */
std::cout << "Valid iterator usage: \n";
std::unique_ptr<Foo::Iterator> evens = foo.iterator(0);
for (int i : *evens)
std::cout << i << "\n";
}
void crashing_version() {
Foo foo;
for (int i = 0; i < 10; i++)
foo.add(i);
/* Crash! The unique_ptr goes out of scope before the loop starts. */
std::cout << "Corrupt iterator usage: \n";
for (int i : *foo.iterator(1))
std::cout << i << "\n";
}
int main() {
working_version();
crashing_version();
return 0;
}
Program output:
Valid iterator usage:
0
2
4
6
8
~Iterator(): Destroying iterator of the even list
Corrupt iterator usage:
~Iterator(): Destroying iterator of the odd list
1
3
5
7
9
A for(range_declaration:range_expression) expression is equivalent (in c++11 and c++14) to:
{
auto && __range = range_expression ;
for (
auto __begin = begin_expr, __end = end_expr;
__begin != __end;
++__begin)
{
range_declaration = *__begin;
loop_statement
}
}
source, with variables starting with __ existing only as exponsition.
We substitute:
for (int i : *evens)
std::cout << i << "\n";
and we get:
{
auto && __range = *evens;
for (
auto __begin = begin_expr, __end = end_expr;
__begin != __end;
++__begin)
{
int i = *__begin;
std::cout << i << "\n";
}
}
we can now clearly see your bug. Your unique ptr lasts as long as the __range line, but after dereferencing the unique ptr goes away, and we have a dangling reference in __range.
You can fix this with a little helper:
template<class Ptr>
struct range_ptr_t {
Ptr p;
auto begin() const {
using std::begin;
return begin(*p);
}
auto end() const {
using std::end;
return end(*p);
}
};
template<class Ptr>
range_ptr_t<std::decay_t<Ptr>> range_ptr( Ptr&& ptr ) {
return {std::forward<Ptr>(ptr)};
}
now we do:
for (int i : range_ptr(evens))
std::cout << i << "\n";
and we no longer have the unique ptr dying on us.
It might be a good idea to extend the lifetime of range_expression to the body of the for(:) loop, as this problem leads to other issues (like when chaining range adapters) that end up with similarly annoying workarounds.
Minimal testcase:
std::unique_ptr<std::vector<int>> foo() {
return std::make_unique<std::vector<int>>( std::vector<int>{ 1, 2, 3} );
}
int main() {
for (int x : range_ptr(foo())) {
std::cout << x << '\n';
}
}