[libcxx-commits] [PATCH] D61170: Use std::move in numeric algorithms

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added a comment.

I don't understand P0616 at all.

It's expecting users to provide `operator+(T&&, T const&)` and `operator-(T const&, T&&)` that move from the first and second arguments respectively.
Who writes that kind of thing?

But that ship appears to have sailed.

I don't see anything majorly wrong with this (except the paper itself). But I'll leave it up to Marshall to finish this review.



================
Comment at: test/std/numerics/numeric.ops/accumulate/accumulate_op.pass.cpp:40
+    std::accumulate(arr, arr + size, force_move());
+    std::accumulate(arr, arr + size, force_move(), force_move());
+}
----------------
zoecarver wrote:
> mclow.lists wrote:
> > I don't see what this code is testing.  You set no values in the array; nor do you test the results.
> This ensures that the left-hand side of the addition operator is an rvalue. 
A better name would be `ensure_move`, or `rvalue_addable`.

Also, why is the rvalue `const`?




================
Comment at: test/std/numerics/numeric.ops/accumulate/accumulate_op.pass.cpp:73
 
-  return 0;
+    test_use_move();
+
----------------
The new test requires C++11 but this test file runs in C++03. So you need to wrap the new test in `#ifdef TEST_STD_VER >= 11` blocks.




================
Comment at: test/std/numerics/numeric.ops/inner.product/inner_product_comp.pass.cpp:35
+
+force_move operator+(force_move const&& a, force_move const&)
+{ return a; }
----------------
Again, none of the `&&` parameters should be const.


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

https://reviews.llvm.org/D61170