[libcxx-commits] [PATCH] D114136: [libc++] Test that our algorithms never copy a user-provided comparator.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__algorithm/min_element.h:49
 template <class _ForwardIterator>
 _LIBCPP_NODISCARD_EXT inline
 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
----------------
In a couple of these cases I drive-by removed `inline` from function templates. I could put it back.
Do we think `inline` has any beneficial effect, these days?
```
$ git grep -L 'inline' ../libcxx/include/__algorithm/*.h
../libcxx/include/__algorithm/comp.h
../libcxx/include/__algorithm/half_positive.h
../libcxx/include/__algorithm/is_partitioned.h
../libcxx/include/__algorithm/partition_copy.h
../libcxx/include/__algorithm/partition_point.h
../libcxx/include/__algorithm/remove.h
../libcxx/include/__algorithm/remove_if.h
../libcxx/include/__algorithm/shuffle.h
../libcxx/include/__algorithm/sift_down.h
```


================
Comment at: libcxx/test/libcxx/algorithms/robust_against_copying_comparators.pass.cpp:104
+    (void)std::find_if_not(first, last, UnaryTrue(&copies)); assert(copies == 0);
+    (void)std::for_each(first, last, UnaryVoid(&copies)); assert(copies == 1); copies = 0;
+#if TEST_STD_VER > 14
----------------
This one is different because `for_each` returns the comparator by move (and for `UnaryVoid` a move //is// a copy).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114136