Inconsecutive implementation of __select_backend

[SergeyKopienko]

The yet another implementation of this task is in #1456

Looks like we have incontinent implementation of __select_backend.
Sometimes we parametrize our dispatch tags with std::true_type or std::false_type types.
But in some cases we parametrize our dispatch tags with some another types like _internal::__is_random_access_iterator<_IteratorTypes...>
In this case it's not the same as std::true_type or std::false_type types.

For example let's analyze the next example of code:

void do_something_for(__serial_tag<std::false_type>) // overload (1)
{
    // use this overload for  __serial_tag<std::false_type>
}

template <typename __Tag>
void do_something_for(__Tag)
{
    // use this overload for  __serial_tag<std::true_type> and for tags of all other types
}

With our previous implementation overload (1) wouldn't work for oneapi::dpl::execution::unsequenced_policy due error in the code :

template <class... _IteratorTypes>
__serial_tag<__internal::__is_random_access_iterator<_IteratorTypes...>>
__select_backend(oneapi::dpl::execution::unsequenced_policy, _IteratorTypes&&...)
{
    return {};
}

because __serial_tag has been specialized here with some another type like __is_random_access_iterator.

This issue has been found by @MikeDvorskiy during works with tag dispatching.

Another example - some code like __pattern_for_loop_n

Let's take a loot at our code :

template <typename _ExecutionPolicy, typename _Ip, typename _Size, typename _Function, typename... _Rest>
void
__pattern_for_loop_n(_ExecutionPolicy&&, _Ip __first, _Size __n, _Function __f, __single_stride_type,
                     /*vector=*/::std::false_type, /*parallel=*/::std::false_type, _Rest&&... __rest) noexcept
{
     // ...
}

The straightforward path too rewrite this function on tags is :

template <typename _ExecutionPolicy, typename _Ip, typename _Size, typename _Function, typename... _Rest>
void
__pattern_for_loop_n(__serial_tag<::std::false_type>,_ExecutionPolicy&&, _Ip __first, _Size __n, _Function __f, __single_stride_type, _Rest&&... __rest) noexcept
{
     // ...
}

But here we have some problem in this param type __serial_tag<::std::false_type> : not always the real type of _IsVector inside __serial_tag is std::true_type or std::false_type.
So we have the question - what is correct way to write overload of this concrete function on tags?

In this PR I am trying to resolve this point.

[SergeyKopienko]

FYI: looks like our upstream implementation has the same inconsecutive implementation here:

template <typename _IteratorTag, typename... _IteratorTypes>
using __are_iterators_of = std::conjunction<
    std::is_base_of<_IteratorTag, typename std::iterator_traits<std::decay_t<_IteratorTypes>>::iterator_category>...>;
 
template <typename... _IteratorTypes>
using __are_random_access_iterators = __are_iterators_of<std::random_access_iterator_tag, _IteratorTypes...>;

[SergeyKopienko]

Here the returning type is ::std::conjunction - looks like an error!

[SergeyKopienko]

Additionally, this implementation is incompletable with the code below:

template <typename... _IteratorTypes>
auto
__is_iterator_of(...) -> ::std::false_type;

[SergeyKopienko]

Here the returning type is the resulting type of std::conjunction which is std::true_type or std::false_type (according to https://en.cppreference.com/w/cpp/types/conjunction).

[danhoeflinger]

If I'm understanding the problem correctly, std::conjunction falls into the same issue as doing struct __is_random_access_iterator. There may be a simpler way of doing this, but to end up with exactly std::true_type or std::false_type, I think this may need to be:

std::integral_constant<bool, std::conjunction_v<...>>

where ... is the list of std::true_type, std::false_type for the individual iterators provided.

[danhoeflinger]

Never mind. I missed the ::type at the end of your formulation which is, in fact, the more direct / simpler way I thought might exist. :)

You can ignore my previous message.

[SergeyKopienko]

Here __is_random_access_iterator is the separate type (structure) which derived from result type of decltype(__is_iterator_of<::std::random_access_iterator_tag, _IteratorTypes...>(0))

[SergeyKopienko]

Here __is_random_access_iterator is template alias which is exactly the same type as decltype(__is_iterator_of<::std::random_access_iterator_tag, _IteratorTypes...>(0))

[SergeyKopienko]

Here __is_forward_iterator is the separate type (structure) which derived from result type of decltype(__is_iterator_of<::std::forward_iterator_tag, _IteratorTypes...>(0))

[SergeyKopienko]

Here __is_forward_iterator is template alias which is exactly the same type as decltype(__is_iterator_of<::std::forward_iterator_tag, _IteratorTypes...>(0))

[SergeyKopienko]

@MikeDvorskiy I believe this change should fix your errors. Please try to apply these changes into your code.

[danhoeflinger]

Based on the explanation and some experimentation in godbolt, this looks like a good fix to me.

Of course, it should wait for confirmation of fixing the error from @MikeDvorskiy

The base branch was changed.

[SergeyKopienko]

Discussed offline: _IsVector may have something different type, not only std::false_type or std::true_type.