Add workaround to parallel merge in TBB backend not to use input iterators default constructor

[kboyarinov]

Add workaround to implementation of __parallel_merge function for the following use case:

#include <oneapi/dpl/algorithm>
#include <oneapi/dpl/execution>
#include <oneapi/dpl/iterator>
#include <vector>

int main() {
    std::vector<int> input1 = {1, 2, 3};
    std::vector<int> input2 = {1, 2, 3};
    std::vector<int> output = {0, 0, 0};

    auto no_transformation = [](int i) { return i; };

    oneapi::dpl::merge(oneapi::dpl::execution::par,
                       oneapi::dpl::make_transform_iterator(input1.begin(), no_transformation),
                       oneapi::dpl::make_transform_iterator(input1.end(), no_transformation),
                       input2.begin(),
                       input2.end(),
                       output.begin());
}

The first input iterator in merge algorithm is dpl::transform_iterator<vector_it, lambda>. Since lambda is not default constructible, this use case results in the compilation error while trying to default construct iterator inside TBB backend.

From the C++ Standard perspective, it is not an issue, because default constructability is part of ForwardIterator requirements, but after the short offline discussion, it was decided to workaround this place in parallel backend TBB to make things better for our fancy iterators.

There is still no 100% guarantee, that no issues would be present for transform iterator with lambda body, because some algorithms we use from the C++ Standard Library are still allowed to call default constructor on the input iterator, e.g. std::upper_bound). This PR was tested together with libstdc++ and no issues was found.

[SergeyKopienko]

I think we are able to fix this issue without changes in transform_iterator and it's more preferable: for that we should duplicate some code in if / else parts inside https://github.com/oneapi-src/oneDPL/pull/1346/files#diff-9908a4470cd112b8a7c7c87c5d3dfa425d873a5e5f9bf353268e6bdf756f5af8.

[SergeyKopienko]

What about the code

        // Using callable object instead of lambda here to ensure transform iterator would be
        // default constructible, that is part of the Forward Iterator requirements in the C++ standard.
        struct NoTransform
        {
            ValueType operator()(const ValueType& val) const
            {
                return val;
            }
        };

Should we use lambda now in

oneDPL/test/parallel_api/iterator/permutation_iterator.h

Line 211 in 5740f58

struct NoTransform

?

[kboyarinov]

What about the code

        // Using callable object instead of lambda here to ensure transform iterator would be
        // default constructible, that is part of the Forward Iterator requirements in the C++ standard.
        struct NoTransform
        {
            ValueType operator()(const ValueType& val) const
            {
                return val;
            }
        };

Should we use lambda now in

oneDPL/test/parallel_api/iterator/permutation_iterator.h

Line 211 in 5740f58

struct NoTransform

?

I would prefer to keep this test as-is because, as I mentioned this fix is not a 100% guarantee that merge with transform_iterator with lambda body would work. std::upper_bound and std::lower_bound implementations can still rely on default constructability of input iterators.
For today, this code works, but formally such an iterator violates the ForwardIterator requirements from the Standard.
This patch is intended to minimize the risk of issues, as we discussed offline.

[SergeyKopienko]

What about the code

        // Using callable object instead of lambda here to ensure transform iterator would be
        // default constructible, that is part of the Forward Iterator requirements in the C++ standard.
        struct NoTransform
        {
            ValueType operator()(const ValueType& val) const
            {
                return val;
            }
        };

Should we use lambda now in

oneDPL/test/parallel_api/iterator/permutation_iterator.h

Line 211 in 5740f58

struct NoTransform

?

I would prefer to keep this test as-is because, as I mentioned this fix is not a 100% guarantee that merge with transform_iterator with lambda body would work. std::upper_bound and std::lower_bound implementations can still rely on default constructability of input iterators. For today, this code works, but formally such an iterator violates the ForwardIterator requirements from the Standard. This patch is intended to minimize the risk of issues, as we discussed offline.

ok

[SergeyKopienko]

LGTM

[akukanov]

Should the patch also be done in PSTL upstream?
@MikeDvorskiy @rarutyun

[MikeDvorskiy]

Recently, we came to conclusion that we should not write :: operator before std:: in new code, at least.

[MikeDvorskiy]

Should the patch also be done in PSTL upstream? @MikeDvorskiy @rarutyun

I guess, yes it should.