thrust icon indicating copy to clipboard operation
thrust copied to clipboard

transform_iterator fails with callables whose arguments are passed by non-const l-value ref

Open jrhemstad opened this issue 3 years ago • 2 comments

Minimal reproducer: https://godbolt.org/z/s4oPhqx3W

This fails because it cannot deduce the return type of the callable.

A workaround is to explicitly define the result_type in the callable: https://godbolt.org/z/MTM8b1zW9

However, I believe a better solution is to fix the result type deduction.

It currently fails because it is ultimately attempting to do:

using result_type = std::invoke_result_t<ref, int>;

This fails due to using int as the argument type instead of int&.

This stems from using thrust::iterator_value on the adapted iterator here: https://github.com/NVIDIA/thrust/blob/d50708f1be248f4a42e81fdce360cf368990582e/thrust/iterator/detail/transform_iterator.inl#L39-L42

Instead of getting the value_type of the iterator, I believe it should use iterator_reference:

    typedef typename thrust::detail::ia_dflt_help<
      Reference,
      thrust::detail::result_of_adaptable_function<UnaryFunc(typename raw_reference<typename thrust::iterator_reference<Iterator>::type>>::type)>
    >::type reference;

Note the use of raw_reference to avoid issues with device_vector::iterator::reference.

Example: https://godbolt.org/z/8eePb5Kvc

jrhemstad avatar Jan 13 '22 17:01 jrhemstad

Can you share a bit more about the usecase here? Using transform_iterator with a functor that modifies the input sequence seems problematic, and I'm not sure this should be allowed.

alliepiper avatar Jan 13 '22 19:01 alliepiper

Can you share a bit more about the usecase here? Using transform_iterator with a functor that modifies the input sequence seems problematic, and I'm not sure this should be allowed.

I've been experimenting with creating a new fancy iterator that allows accessing a data member of an object. There have been several cases in the past where I've wanted to compute the result of something like a scan directly into into existing objects without having to materialize the intermediate scan result and then copy it into the existing objects.

One way I was trying to do this was via a transform iterator that takes an l-value ref of the object and returns an l-value ref of the member:

https://godbolt.org/z/jd1obbbWG

Generally I'd agree with you that taking an l-value ref in a transform function is a bad idea as it almost certainly means side-effects. In this case there aren't any side-effects. It's moreso to create a different kind of "transform_output_iterator".

jrhemstad avatar Jan 13 '22 19:01 jrhemstad