[oneTBB] Improve named requirements to better deal with value categories

[akukanov]

This patch attempts to clarify how named requirements are defined, especially for dealing with various value categories a function can be applied to.

Using these clarifications, the definition of ParallelForEachBody is changed to be simpler, while the semantics should remain the same (unless I missed some nuances).

[kboyarinov]

The part describing the pseudo-signature looks good.
Need a bit more time to review the correctness of parallel_for_each semantics

[kboyarinov]

May be it makes sense to also explicitly mention that if pseudo-signature declares return type as void, but the real signature returns some type, the returned value should be ignored by the implementation.

[kboyarinov]

Should it be ReferenceType as well?

Suggested change

	.. cpp:function:: void Body::operator()( ItemType&& item, oneapi::tbb::feeder<ItemType>& feeder ) const
	.. cpp:function:: void Body::operator()( ReferenceType item, oneapi::tbb::feeder<ItemType>& feeder ) const

[akukanov]

This question is related to the question below about "removing" a requirement:

Additional elements submitted into oneapi::tbb::parallel_for_each through the feeder::add are passed to the Body as rvalues. In this case, the corresponding execution of the Body is required to be well-formed.

As I understand it, it says that if the feeder is used, the body has to accept rvalues - because the work elements supplied through the feeder will be "moved" to the body for processing. Since parallel_for_each may only use one operator(), I interpret the requirement as "if your body operator takes a feeder, it has to accept rvalues" - and that is exactly what the pseudo-signature describes.

If the idea is that the original sequence elements are passed just as the result of iterator dereferencing (i.e., *it) while new work elements are "moved" - the operator should then support both ReferenceType (which might be an lvalue reference) and ItemType&& rvalue reference - so it seemingly should take arguments either by value or by const reference. Neither allows modification of the original sequence (which can be fine, given the use of feeder); taking by value will have some copy construction overhead, and taking by const reference makes move useless.

Another option is to use a templated operator with universal references or use different overloads for lvalues and rvalues.

Let's figure out what semantics we want, how it could be implemented in practice, and then hopefully it will be more clear how to describe that in the named requirement.

[kboyarinov]

Yes, the original idea was that the elements of the input sequence are passed as the result of dereferencing. and additional items are always provided as rvalues.

The semantics proposed would break the first part, for example if the iterator have non-const lvalue reference, *it will not be accepted by the body. And the opposite is true, if the operator accepts non-const lvalue reference argument, passing of new items through feeder would not work.

I disagree that parallel_for_each may use only one operator(). I think it should be possible to have several overloads with different first argument types for input sequence and feeder items. By the way, current implementation of oneTBB would also work if the input sequence dereference is passed to the overload with feeder and the additional items are passed to the overload without the feeder:

struct body {
    // overload for feeder items
    void operator()(ItemType&& value_from_feeder) const {}
    
    // overload for input sequence
    void operator()(ItemType& input_sequence, oneapi::tbb::feeder<ItemType>& feeder) const {}
};

[akukanov]

I disagree that parallel_for_each may use only one operator(). I think it should be possible to have several overloads with different first argument types for input sequence and feeder items.

I meant. only one of the two specified operators - either with or without the feeder, but not both. Several overloads with the feeder would be fine, if there is a good rationale for that.

By the way, current implementation of oneTBB would also work if the input sequence dereference is passed to the overload with feeder and the additional items are passed to the overload without the feeder:

This is simply an incorrect behavior. The choice of using or not using the feeder should not depend on how an item was obtained or on whether we can move it or not. Yes, we can distinguish between these sources of input, but why do we think it matters for the user, based only on the type of the parameter? After all, the feeder can be ignored when provided, but it cannot be created when absent,

[kboyarinov]

This document specify that the body should meet one of the requirements - either be invocable without a feeder or with it.
Should it be specified/recommended which overload should be preferred if both of them are present.
oneTBB implementation prefers the overload with the feeder.

[akukanov]

Yes, I think it should be specified. I do not see how the implementation could use both overloads; it would need to decide whether to provide the feeder or not, and how would it decide? So using the overload with the feeder all the time is the "safest" choice.

[kboyarinov]

Is it intentional that this requirement is removed?