oneapi-src / oneDPL

oneAPI DPC++ Library (oneDPL) https://software.intel.com/content/www/us/en/develop/tools/oneapi/components/dpc-library.html
Apache License 2.0
720 stars 114 forks source link

To do the same fix for output type (by re-using __transform_functor if applicable), to make input type consistently: const T& or T&& #1369

Open MikeDvorskiy opened 7 months ago

MikeDvorskiy commented 7 months ago

1) TODO: to do the same fix for output type (by re-using transform_functor if applicable) for the other functor below: __transform_if_unary_functor, transform_if_binary_functor, __replace_functor, __replace_copy_functor 2) TODO: to make input type consistently: const T& or T&&; to think which way is preferable

_Originally posted by @MikeDvorskiy in https://github.com/oneapi-src/oneDPL/pull/1233#discussion_r1463205525_

Proposed Implementation:

template <typename _UnaryOper, typename _UnaryPred>
class __transform_if_unary_functor
{
    __transform_functor<_UnaryOper> _M_oper;
    mutable _UnaryPred _M_pred;

  public:
    explicit __transform_if_unary_functor(_UnaryOper&& __op, _UnaryPred&& __pred)
        : _M_oper(::std::forward<_UnaryOper>(__op)), _M_pred(::std::forward<_UnaryPred>(__pred))
    {
    }

    template <typename _Input1Type, typename _OutputType>
    void
    operator()(const _Input1Type& __x, _OutputType&& __output) const
    {
        if (_M_pred(__x))
            _M_oper(__x, ::std::forward<_OutputType>(__output));
    }
};

template <typename _BinaryOper, typename _BinaryPred>
class __transform_if_binary_functor
{
    __transform_functor<_BinaryOper> _M_oper;
    mutable _BinaryPred _M_pred;

  public:
    explicit __transform_if_binary_functor(_BinaryOper&& __op, _BinaryPred&& __pred)
        : _M_oper(::std::forward<_BinaryOper>(__op)), _M_pred(::std::forward<_BinaryPred>(__pred))
    {
    }

    template <typename _Input1Type, typename _Input2Type, typename _OutputType>
    void
    operator()(const _Input1Type& __x, const _Input2Type& __y, _OutputType&& __output) const
    {
        if (_M_pred(__x, __y))
            _M_oper(__x, __y, ::std::forward<_OutputType>(__output));
    }
};

template <typename _Tp, typename _Pred>
class __replace_functor
{
    const _Tp _M_value;
    _Pred _M_pred;

  public:
    __replace_functor(const _Tp& __value, _Pred __pred) : _M_value(__value), _M_pred(__pred) {}

    template <typename _OutputType>
    void
    operator()(_OutputType&& __output) const
    {
        if (_M_pred(::std::forward<_OutputType>(__output)))
            ::std::forward<_OutputType>(__output) = _M_value;
    }
};

template <typename _Tp, typename _Pred>
class __replace_copy_functor
{
    const _Tp _M_value;
    _Pred _M_pred;

  public:
    __replace_copy_functor(const _Tp& __value, _Pred __pred) : _M_value(__value), _M_pred(__pred) {}

    template <typename _InputType, typename _OutputType>
    void
    operator()(const _InputType& __x, _OutputType&& __output) const
    {
        ::std::forward<_OutputType>(__output) = _M_pred(__x) ? _M_value : __x;
    }
};

3) Also to investigate whether be any issue with another const tuple_type& (and std::get) place in the code: https://github.com/oneapi-src/oneDPL/blob/main/include/oneapi/dpl/pstl/utils_ranges.h#L140

MikeDvorskiy commented 2 months ago

At least, const T& (instead of T&&) breaks binary_search.pass test.

 error: static assertion failed due to requirement '::std::is_invocable_v<(lambda at /home/sdp/mdvorski/oneDPL/make/../include/oneapi/dpl/internal/binary_search_impl.h:120:35), const unsigned long &>': A predicate cannot be called with the passed arguments
  302 |         static_assert(::std::is_invocable_v<_Pred, _Args...>, "A predicate cannot be called with the passed arguments");
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/sdp/mdvorski/oneDPL/make/../include/oneapi/dpl/pstl/hetero/dpcpp/../../utils.h:293:9: note: in instantiation of function template specialization 'oneapi::dpl::__internal::__transform_functor<(lambda at /home/sdp/mdvorski/oneDPL/make/../include/oneapi/dpl/internal/binary_search_impl.h:120:35)>::__transform_impl<unsigned long &, const unsigned long &>' requested here
  293 |         __transform_impl(::std::forward<_OutputType>(__output), __x);
 error: no matching function for call to object of type '(lambda at /home/sdp/mdvorski/oneDPL/make/../include/oneapi/dpl/internal/binary_search_impl.h:120:35)'
  303 |         ::std::forward<_OutputType>(__output) = _M_pred(::std::forward<_Args>(__args)...);
      |                                                 ^~~~~~~
/home/sdp/mdvorski/oneDPL/make/../include/oneapi/dpl/internal/binary_search_impl.h:120:35: note: candidate function not viable: 1st argument ('const unsigned long') would lose const qualifier
  120 |                                   [=](typename ::std::iterator_traits<InputIterator2>::reference val) {
      |                                   ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~