[libcxx-commits] [PATCH] D128983: [libcxx][ranges] implement `std::ranges::set_difference`

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik accepted this revision.
philnik added a comment.

LGTM % nits.



================
Comment at: libcxx/include/__algorithm/ranges_set_difference.h:67-95
+    template <
+        input_range _Range1,
+        input_range _Range2,
+        weakly_incrementable _OutIter,
+        class _Comp  = less,
+        class _Proj1 = identity,
+        class _Proj2 = identity>
----------------
This formatting is really weird. Hopefully I'll have some time soon to fix clang-format a bit.


================
Comment at: libcxx/include/__algorithm/set_difference.h:68-76
+  auto __ret = std::__set_difference(
+      __first1,
+      __last1,
+      __first2,
+      __last2,
+      __result,
+      __less<typename iterator_traits<_InputIterator1>::value_type,
----------------
Why aren't you returning `std::__set_difference(...).second` here? IIUC explicitly doing the move is either redundant because of implicit move or detrimental because of copy elision.


================
Comment at: libcxx/test/std/library/description/conventions/customization.point.object/niebloid.compile.pass.cpp:127
 //static_assert(test(std::ranges::replace_copy_if, a, a, odd, 43));
 static_assert(test(std::ranges::replace_if, a, odd, 43));
 static_assert(test(std::ranges::reverse, a));
----------------
Could you also uncomment the corresponding line in `ranges_result_alias_declarations.compile.pass.cpp`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128983