[libcxx-commits] [PATCH] D128618: [libc++] Implement ranges::remove{, _if}

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision.
var-const added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/module.modulemap.in:344
       module ranges_none_of                  { private header "__algorithm/ranges_none_of.h" }
+      module ranges_remove                   { private header "__algorithm/ranges_remove.h" }
       module ranges_replace                  { private header "__algorithm/ranges_replace.h" }
----------------
Do you also need to add `remove_if`?


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges.remove_if.pass.cpp:143
+
+   // This is https://llvm.org/PR56382
+   // clang-format off
----------------
Nit: can you add a little bit more context in addition to the link?


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges.remove_if.pass.cpp:65
+constexpr void test(Data<N, M> d) {
+  {
+    auto input = d.input;
----------------
philnik wrote:
> var-const wrote:
> > Please add brief comments to explain what is being tested.
> What exactly are you asking for? Something like `check that the expected outcome is the actual outcome` isn't exactly useful, but I don't think there is anything more specific I could say.
Basically I meant something like `// Iterator overload`, `// Range overload`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128618