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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver marked 2 inline comments as done.
zoecarver added inline comments.


================
Comment at: include/numeric:425
             typename iterator_traits<_InputIterator>::value_type __t2(*__first);
-            *__result = __t2 - __t1;
+            *__result = _VSTD::move(__t2) - __t1;
             __t1 = _VSTD::move(__t2);
----------------
mclow.lists wrote:
> And here.
> For every iterator i in [first + 1, last) in order, creates an object val whose type is InputIterator’s value type, initializes it with *i, computes val - **std::move(acc)** or binary_op(val, **std::move(acc))**, assigns the result to *(result + (i - first)), and move assigns from val to acc.

Maybe I am misunderstanding what the paper is saying. While the above call to `std::move` is not specified in the paper, I think the one on line 425 is. 


================
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());
+}
----------------
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. 


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D61170