[PATCH] D41372: [libcxx] Fix transform_reduce mishandling move-only types, and nonstandard macro use in tests.

Billy Robert O'Neal III via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 19 15:28:56 PST 2017

BillyONeal added a comment.

Do you folks want me to port this test case for move-only T into libcxx? (I added this to our test harness) Note that discussion on validity is ongoing on lists.

  #include <stdlib.h>
  #include <algorithm>
  #include <execution>
  #include <memory>
  #include <iterator>
  #include <numeric>
  #include <utility>
  #include <vector>
  #include <parallel_algorithms_utilities.hpp>
  #include <pmretvals.h>
  using namespace std;
  using namespace std::execution;
  void verify(const bool b) {
      if (!b) {
  vector<unique_ptr<vector<unsigned int>>> get_move_only_test_data(const size_t testSize) {
      vector<unique_ptr<vector<unsigned int>>> testData;
      for (size_t idx = 0; idx < testSize; ++idx) {
          testData.emplace_back(make_unique<vector<unsigned int>>(1, static_cast<unsigned int>(idx)));
      return testData;
  template<class ExPo>
  void test_case_move_only(const size_t testSize, ExPo&& exec) {
      auto testData = get_move_only_test_data(testSize);
      unique_ptr<vector<unsigned int>> result = transform_reduce(std::forward<ExPo>(exec),
          make_move_iterator(testData.begin()), make_move_iterator(testData.end()),
          make_unique<vector<unsigned int>>(),
          [](unique_ptr<vector<unsigned int>> lhs, unique_ptr<vector<unsigned int>> rhs) {
              lhs->insert(lhs->end(), rhs->begin(), rhs->end());
              return lhs;
          [](unique_ptr<vector<unsigned int>> target) {
              verify(target->size() == 1); // should only be called directly on the input sequence
              target->back() *= 10;
              return target;
      sort(result->begin(), result->end());
      for (size_t idx = 0; idx < testSize; ++idx) {
          verify((*result)[idx] == idx * 10);
  int main() {
      parallel_test_case(test_case_move_only<const sequenced_policy&>, seq);
      parallel_test_case(test_case_move_only<const parallel_policy&>, par);
      return PM_TEST_PASS;

Comment at: include/numeric:245
     for (; __first1 != __last1; ++__first1, (void) ++__first2)
         __init = __b1(__init, __b2(*__first1, *__first2));
     return __init;
BillyONeal wrote:
> It's arguable that this should be _VSTD::move(__init); I asked SG1 / LEWG about that. (After all, if they're going to make inner_product do that this should too)
Any comment on whether these should have extra moves added @mclow.lists  ?

Comment at: test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp:61
+inline MoveOnly operator+(const MoveOnly& lhs, const MoveOnly& rhs)
mclow.lists wrote:
> BillyONeal wrote:
> > Should these go in MoveOnly.h ?
> I think so. While you're at it, you can remove the `friend` declaration in `MoveOnly` - that's a remnant.
Should I also add op== and op< while I'm in there for testing things like sort?



More information about the cfe-commits mailing list