[libcxx-commits] [PATCH] D149599: [libc++][PSTL] Reduce the amount of transitive includes

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM w/ comments



================
Comment at: libcxx/include/__algorithm/pstl_any_all_none_of.h:61
 all_of(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator __last, _Pred __pred) {
-  return !std::any_of(__policy, __first, __last, std::not_fn(__pred));
+  return !std::any_of(__policy, __first, __last, std::__negate_function(__pred));
 }
----------------
I would just use a lambda instead, since this is used only in two places. Otherwise, honestly I'd imagine someone finding `__negate_function` and "refactoring" it to `std::not_fn`.


================
Comment at: libcxx/include/__pstl/internal/parallel_impl.h:36
     _DifferenceType __initial_dist = __b_first ? __n : -1;
-    std::atomic<_DifferenceType> __extremum(__initial_dist);
+    std::__atomic_base<_DifferenceType> __extremum(__initial_dist);
     // TODO: find out what is better here: parallel_for or parallel_reduce
----------------
I think I would still use `std::atomic` here, but I'd include `<__atomic/atomic.h>` instead. `__atomic_base` is arguably an implementation detail of `std::atomic`, and I don't think we should start using it widely in the rest of our code base. And in fact there's a few places that use `std::__atomic_base` right now in the synchronization library, and those could be refactored to use `std::atomic` (although those are IMO less vexing since the synchronization library is so close to `<atomic>`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149599