Re: [Boost-users] [range] istream_range from a temporary istream

Subject: Re: [Boost-users] [range] istream_range from a temporary istream
From: Nathan Ridge (zeratul976_at_[hidden])
Date: 2011-07-22 14:06:59

> >> This way one can write things like:
> >>
> >> vector<T> v;
> >> push_back(v, istream_range<T>(ifstream("some_file.txt")));
> >
> > Of course, one must be careful with such things.
> >
> > Can you spot the bug in the following?
> >
> > BOOST_FOREACH(const string& s, istream_range<string>(ifstream("some_file.txt")))
> > cout<< s;
> >
>
> Wouldn't that fail to compile if the istream_range is not copyable, but
> movable?

It's worse than that, I'm afraid. It does compile, but produces a silent 
runtime error.
 
The problem is that the lifetime of the temporary ifstream object does not 
extend to the entire loop.
 
The BOOST_FOREACH expands to something like this:
 
for (const auto& __range = istream_range<string>(ifstream("some_file.txt"));
     // ... conditions that make sure this loop only runs once ...
    )
    // ... other loops that introduce iterators etc. ...
{
    cout << s;
}
 
The temporary istream_range object will live throughout the loop because
a constant reference to it is maintained; however, the temporary ifstream
object which the istream_range object holds a reference to (indirectly via
its istream_iterators), will get destructed as soon as the for loop's 
init-statement finishes executing. As a result, the loop will try to read 
from an ifstream object that's already destructed, which of course is 
undefined behaviour.
 
Note that this problem doesn't occur if you instead do:
 
for_each(istream_range<string>(ifstream("some_file.txt")),
         [](const string& s){ cout << s; });
 
In this case, the loop is contained within the statement that declares
the ifstream temporary, so everything is fine.
 
Regards,
Nate.