[libcxx-commits] [PATCH] D104492: [libc++][pstl] Implement tag dispatching

[Mikhail Dvorskiy via Phabricator via libcxx-commits]

MikeDvorskiy accepted this revision.
MikeDvorskiy added a comment.

In spite of my minor comments it LGTM.



================
Comment at: pstl/include/pstl/internal/algorithm_fwd.h:402
 template <class _ForwardIterator, class _OutputIterator>
-_OutputIterator
-__brick_swap_ranges(_ForwardIterator __first, _ForwardIterator __last, _OutputIterator __result,
-                    /*vector=*/std::false_type) noexcept;
+_OutputIterator __brick_swap_ranges(_ForwardIterator, _ForwardIterator, _OutputIterator,
+                                    /*vector=*/std::false_type) noexcept;
----------------
I guess we have wrong clang format changes here. (As far as I remember our profile of clang-format options keeps return type on the separate line).


================
Comment at: pstl/include/pstl/internal/algorithm_fwd.h:406
 template <class _RandomAccessIterator, class _OutputIterator>
-_OutputIterator
-__brick_swap_ranges(_RandomAccessIterator __first, _RandomAccessIterator __last, _OutputIterator __result,
-                    /*vector=*/std::true_type) noexcept;
+_OutputIterator __brick_swap_ranges(_RandomAccessIterator, _RandomAccessIterator, _OutputIterator,
+                                    /*vector=*/std::true_type) noexcept;
----------------
See the comment below.


================
Comment at: pstl/include/pstl/internal/algorithm_impl.h:308
 _RandomAccessIterator2
-__pattern_walk2_brick(_ExecutionPolicy&& __exec, _RandomAccessIterator1 __first1, _RandomAccessIterator1 __last1,
-                      _RandomAccessIterator2 __first2, _Brick __brick, /*parallel=*/std::true_type)
+__pattern_walk2_brick(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _RandomAccessIterator1 __first1,
+                      _RandomAccessIterator1 __last1, _RandomAccessIterator2 __first2, _Brick __brick)
----------------
As far as I can see, here and in the other same places  we don't in use parameter name __tag in code, excepting type deduction (via decltype) of type that we already have. So, I would propose to omit this parameter name and define __backend_tag as:

using __backend_tag = typename __parallel_tag<_IsVector>::__backend_tag;

It is not showstopper because it is not public API. So, it is up to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104492/new/

https://reviews.llvm.org/D104492