Enable binary comparison operations for internal tuple (second approach) #1481
Conversation
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
include/oneapi/dpl/pstl/tuple_impl.h
Outdated
| operator==(Tuple1&& __lhs, Tuple2&& __rhs) | ||
| { | ||
| return static_cast<__get_std_tuple_t<Tuple1>>(std::forward<Tuple1>(__lhs)) == | ||
| static_cast<__get_std_tuple_t<Tuple2>>(std::forward<Tuple2>(__rhs)); |
There was a problem hiding this comment.
I double checked on Godbolt and this does not introduce a copy for the cast of a std::tuple to a std::tuple. I don't have anything to add; I just wanted to verify that there wasn't an unnecessary copy being introduced.
There was a problem hiding this comment.
I'm still working it out. I think it may inject an extra copy for lvalue refs... still a work in progress.
…t... Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
|
From my pointy of view, when we call template <typename U1, typename... U, ::std::size_t... _Ip>
static ::std::tuple<U1, U...>
to_std_tuple(const oneapi::dpl::__internal::tuple<U1, U...>& __t, ::std::index_sequence<_Ip...>)
{
return ::std::tuple<U1, U...>(oneapi::dpl::__internal::get_impl<_Ip>()(__t)...);
}from |
Yes, it seems I've missed a fundamental issue with this PR. I was trying to avoid re-implementing the functionality of I guess the appropriate thing to do is to just reimplement the functionality of the operators here... then no casting is necessary. |
|
Closing this in light of the issue discussed above in favor of changes in the main branch. |
This takes an alternative approach to #1472 to enable binary comparison using SFINAE.
It only needs a single overload per operator, as it uses SFINAE create the overloads to match the incoming types rather than partial specialization.
In this version we are able to use forwarding (universal references) for the inputs, forwarding into the
static_cast.Here we
static_castall inputs (which may bestd::tupleoroneapi::dpl::__internal::tuple) tostd::tuplewith the same element types. I believe this cast should be converted to a noop at compile time in the case where thestatic_castis the existing type of the input.