make custom range v3 view pipeable

Question

I'm trying to implement masked range views with range v3. Somehow I ended up in the situation where my implementation of

ranges::view::masker(datarange, mask)

works, but the piped version

ranges::view::all(datarange) | ranges::view::masker(mask)

doesn't, although withing the operators of the internal structures, the masks arrive correctly. (I put my implementation of masker into the ranges::view namespace, although it is not part of range v3).

My test program is relatively trivial, create some widgets and a meaningless mask

class Widget
{
private:
  int   m_int{0};

public:
  Widget() {}
  Widget( int i ) : m_int( i ) {}
  int   the_int() const { return m_int; }
};

inline std::ostream& operator<<( std::ostream& str, const Widget& obj )
{
  str << '\t' << obj.the_int();
  return str;
}

int main()
{
  std::vector<Widget> widgets;
  std::vector<bool>   mask;

  for ( auto i : ranges::view::indices( 24 ) ) {
    widgets.emplace_back( i );
    mask.push_back( i % 3 != 1 );
  }

  std::cout << "wrapped" << std::endl;
  for ( auto& el : ranges::view::masker( widgets, mask ) ) {
    std::cout << el << std::endl;
  }
  std::cout << std::endl;
  std::cout << std::endl;

  std::cout << "piped" << std::endl;
  for ( auto& el : ranges::view::all( widgets ) | ranges::view::masker( mask ) ) {
    std::cout << el << std::endl;
  }

  return 0;
}

Ignoring namespaces and debug printout the masker just zips the data range and the mask together, filters on the mask and returns the widgets as view:

        struct mask_fn
        {
            template<typename Rng, typename Msk>
            auto operator()(Rng&& rng, Msk&& msk) const
            {
                CONCEPT_ASSERT(Range<Rng>());
                CONCEPT_ASSERT(Range<Msk>());

                return ranges::view::zip(std::forward<Rng>(rng),
                                         std::forward<Msk>(msk)) |
                       ranges::view::filter([](auto&& range_item) -> bool {

                           return range_item.second;
                       }) |
                       ranges::view::transform(
                           [](auto&& range_item) -> decltype(auto) {
                               return range_item.first;
                           });
            }                  
            template<typename Msk>
            auto operator()(Msk&& msk) const -> decltype(
                make_pipeable(std::bind(*this, std::placeholders::_1,
                                        protect(std::forward<Msk>(msk)))))
            {   
                CONCEPT_ASSERT(Range<Msk>());
                return make_pipeable(
                    std::bind(*this,
                              std::placeholders::_1,
                              protect(std::forward<Msk>(msk))));
            }                 
        };                    

        RANGES_INLINE_VARIABLE(mask_fn, masker)

The above program is intended to print out the same resulting range twice, yet I only get:

So while using auto operator()(Rng&& rng, Msk&& msk) const the right widgets are looped over, the version with auto operator()(Msk&& msk) const does not return anything.

I tried adding some debug printout to the former (because it gets ultimately called by the latter) and observe that the mask arrives correctly.

        struct mask_fn
        {
            template<typename Rng, typename Msk>
            auto operator()(Rng&& rng, Msk&& msk) const
            {
                CONCEPT_ASSERT(Range<Rng>());
                CONCEPT_ASSERT(Range<Msk>());
                for(auto t :
                    ranges::view::zip(rng, msk) |
                        ranges::view::filter([](auto&& range_item) ->
                        bool {
                            return range_item.second;
                        }) |
                        ranges::view::transform(
                            [](auto&& range_item) -> decltype(auto) {
                                return range_item.first;
                            }))
                    std::cout << "w: " << t << std::endl;
                return ranges::view::zip(std::forward<Rng>(rng),
                                         std::forward<Msk>(msk)) |
                       ranges::view::filter([](auto&& range_item) -> bool {
                           std::cout << "checking widget "
                                     << range_item.first << std::endl;
                           std::cout << "returning " << range_item.second
                                     << std::endl;
                           return range_item.second;
                       }) |
                       ranges::view::transform(
                           [](auto&& range_item) -> decltype(auto) {

                               return range_item.first;
                           });
            }
            template<typename Msk>
            auto operator()(Msk&& msk) const -> decltype(
                make_pipeable(std::bind(*this, std::placeholders::_1,
                                        protect(std::forward<Msk>(msk)))))
            {
                CONCEPT_ASSERT(Range<Msk>());
                return make_pipeable(
                    std::bind(*this,
                              std::placeholders::_1,
                              protect(std::forward<Msk>(msk))));
            }
        };

        RANGES_INLINE_VARIABLE(mask_fn, masker)

(cutting the output a bit) one can see that using the supposed return range within operator() I loop over the correct widgets, but the printout from within the lambdas in the return line shows "false" flags for all items.

wrapped
w:  0
w:  2
w:  3
w:  5
<snap>
w:  20
w:  21
w:  23
checking widget     0
returning 1
    0
checking widget     1
returning 0
checking widget     2
returning 1
    2
checking widget     3
returning 1
    3
<snap>
checking widget     22
returning 0
checking widget     23
returning 1
    23


piped
w:  0
w:  2
w:  3
w:  5
<snap>
w:  20
w:  21
w:  23
checking widget     0
returning 0
checking widget     1
returning 0
checking widget     2
returning 0
checking widget     3
returning 0
<snap>
checking widget     22
returning 0
checking widget     23

My best guess at the moment is that I messed up the protect, std::forward, &&, or std::move somewhere, though I tried to stick as close as possible to filter.hpp (since I thought I had understood it reasonably well) and also tried some random adding/removing of ampersands and forwards without success.

Any suggestion how to fix this? (And ideally explanation of what's going on, along?).

Thanks in advance.

footnotes: I'm at the moment not concerned about c++11 compatibility.

EDIT:

I pushed the mess to github.

score 5 · Accepted Answer · answered Jul 18 '18 at 14:35

std::bind is weird. If you pass a bind_expression - the result of a call to std::bind - to std::bind, it produces an expression tree that is evaluated from the "leaves" down. For example:

auto f = [](int i, int j){ return i * j; };
auto g = [](int i) { return i + 1; };
auto b = std::bind(f, std::bind(g, std::placeholders::_1), std::placeholders::_2);
std::cout << b(0, 3) << '\n'; // prints 3

The call b(0, 3) here is equivalent to f(g(0), 3).

protect is a range-v3 utility used to capture function objects inside the bind object that prevents std::bind from engaging this weirdness if those function objects happen to be bind_expressions. For non-bind_expressions, protect has "capture rvalues by value and lvalues by reference" behavior (most things in range-v3 assume that the caller guarantees lifetime of lvalues but that rvalues may "disappear" before needed and therefore must be stored).

Unfortunately, you use protect with a Range in your "partial application" overload:

template<typename Msk>
auto operator()(Msk&& msk) const -> decltype(
    make_pipeable(std::bind(*this, std::placeholders::_1,
                            protect(std::forward<Msk>(msk)))))
{   
    CONCEPT_ASSERT(Range<Msk>());
    return make_pipeable(
        std::bind(*this,
                  std::placeholders::_1,
                  protect(std::forward<Msk>(msk))));
}

std::bind has a different design from range-v3: it stores copies of whatever you pass inside the returned bind_expression and passes lvalues denoting those stored objects to the wrapped function upon invocation. The end effect is that your overload returns a bind_expression wrapped in a make_pipeable that holds a copy of the vector the caller passed in.

When the test program "pipes" in another range, the make_pipeable invokes your other overload with that range and an lvalue denoting the vector copy stored inside the bind expression. You pass that lvalue to view::zip which (as described above) assumes that its caller will guarantee that lvalue will remain alive as long as the zip_view it produces. This is of course not the case: The make_pipeable temporary - including the vector stored inside the bind_expression it contains - are destroyed after evaluating the initializer in the range-for statement in the test program. When the range-for tries to access that dead vector, UB occurs which manifests in this case as an empty range.

The fix is to not use protect in your "partial application" overload, but to instead pass an all_view of the range to std::bind:

template<typename Msk>
auto operator()(Msk&& msk) const -> decltype(
    make_pipeable(std::bind(*this, std::placeholders::_1,
                            ranges::view::all(std::forward<Msk>(msk)))))
{   
    CONCEPT_ASSERT(Range<Msk>());
    return make_pipeable(
        std::bind(*this,
                  std::placeholders::_1,
                  ranges::view::all(std::forward<Msk>(msk))));
}

(Admittedly, it would be nice if protect would guard against this error by refusing to accept Ranges.)

score 0 · Answer 2 · answered Jul 18 '18 at 14:27

0

After more trying around, we figured out that the std::bind should receive a std::ref to the mask.

            return make_pipeable(
                std::bind(*this,
                          std::placeholders::_1,
                          std::ref(msk));

If not, then - so my understanding - the masker would outlive a temporary copy of the msk.

answered Jul 18 '18 at 14:27

pseyfert

3,263
3
21
47

2

FYI, this won't work when someone passes an rvalue `View` to your "partial application" overload: You'll return a `bind_expression` that stores a `reference_wrapper` to that (possibly temporary) `View` object. – Casey Jul 18 '18 at 14:44

make custom range v3 view pipeable

2 Answers2