boost::adaptors::reverse and temporary values

Question

So I've been using boost adaptors to concisely choose iteration order, and I've run into a problematic case:

#include <iostream>
#include <vector>
#include <boost/range/adaptor/reversed.hpp>

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

int main() {
    for(auto v : fn()) {
        std::cout << ' ' << v;
    }
    std::cout << std::endl;

    for(auto v : boost::adaptors::reverse(fn())) {
        std::cout << ' ' << v;
    }
    std::cout << std::endl;
}

output:

1 2 3
3 0 0

So it seems that using reverse on a value returned from a function call like this doesn't work. Is this a bug is boost? Or something else?

Looks like a limitation with the adaptor. If you look at the [source](https://www.boost.org/doc/libs/release/boost/range/adaptor/reversed.hpp), both overloads are constructing an `iterator_range` from the argument, there's no rvalue reference overload that stores the container too. — Praetorian, Apr 17 '20 at 23:39
This is how *for range-based loop* works. Only lifetime of `reverse` returned value is prolonged, lifetime of any temporary expression within range-expression is not extended, vector returned by `fn`() is just destroyed, because `reverse` stores only reference to vector, you have dangling reference. — rafix07, Apr 18 '20 at 07:38

score 0 · Answer 1 · answered Apr 19 '20 at 21:18

0

Yes, this is by design. Like the comments say, the adaptors store a reference to a temporary. This means the temporary is destructed after creating the adaptor, and the adaptor contains a stale reference.

To make it easier to understand, here's a similar adaptor (without using boost):

template <typename R> struct demo_adaptor {
    R const& r;
    auto begin() const { return r.begin(); }
    auto end() const { return r.end(); }
};

Given a helper function to adapt a range r:

template <typename R>
demo_adaptor<R> demo_adapt(R const& r) {
    return { r };
}

The following program invokes Undefined Behaviour:

for(auto v : demo_adapt(fn())) {
    std::cout << ' ' << v;
}

Note: Use -fsanitize=undefined,address on GCC/Clang compilers to find such errors:

Because it gets interpreted as the following equivalent loop (Live On CppInsights):

{
    demo_adaptor<std::vector<int, std::allocator<int>>>&& __range1 = demo_adapt(fn());
    __gnu_cxx::__normal_iterator<const int*, std::vector<int, std::allocator<int>>> __begin1 = __range1.begin();
    __gnu_cxx::__normal_iterator<const int*, std::vector<int, std::allocator<int>>> __end1 = __range1.end();
    for (; __gnu_cxx::operator!=(__begin1, __end1); __begin1.operator++()) {
        int v = __begin1.operator*();
        std::operator<<(std::cout, ' ').operator<<(v);
    }
}

Now the easy fix is to ensure that the range lives longer than the adaptor will:

{
    auto r = fn();
    for (auto v : demo_adapt(r)) {
        std::cout << ' ' << v;
    }
}

Which prints

1 2 3

answered Apr 19 '20 at 21:18

sehe

374,641
47
450
633

In other words, the adaptor is designed to be mostly useless and dangerous... – Chris Dodd Apr 19 '20 at 21:24
@ChrisDodd That's in the eye of the beholder. How would you do it? Would it be useful if you made it always copy temporaries? I think so, and I suspect rangev3 (Niebler's) **does** but that's in large part because it is a successor to Boost Range, which was designed pre-c++11 and hence move-semantics, perfect forwarding and perfect storage were out of the question. – sehe Apr 19 '20 at 21:29
So, no. The library isn't useless. It's just important to realize its assumptions. And yes, [I'm on record for avoiding Boost Range](https://stackoverflow.com/a/36370974/85371) (link from 2016) unless I have very tight control on the uses and ample testing around it (with ubsan/asan and or valgrind). Being aware of the pitfalls is part of being a C++ programmer. Not to do so is wholly irresponsible. – sehe Apr 19 '20 at 21:32
I guess this is why this never made it into the standard library... – Chris Dodd Apr 19 '20 at 21:34
@ChrisDodd Very much. Though we did get `std::string_view` which has the same foot-gun.¯\\_(ツ)_/¯ And because I don't want to be just negative about an awesome library, I **do** use it in production code. One particular shining example I remember is this answer: https://stackoverflow.com/a/28331382/85371 – sehe Apr 19 '20 at 21:39
It seems like one should be able to use something with rvalue references to define a safer version that can't be constructed from an xvalue to avoid a lot of the dangling problems. – Chris Dodd Apr 19 '20 at 22:03
Indeed. I believe `rangev3` has it (much like Spirit Qi has the same problem for expression templates, but not Spirit X3, which is much more recent) – sehe Apr 19 '20 at 22:08

boost::adaptors::reverse and temporary values

1 Answers1