[libcxx-commits] [PATCH] D126283: [libc++] Implement ranges::replace{, _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/test/std/algorithms/alg.modifying.operations/alg.replace/ranges.replace.pass.cpp:43
+  // simple test
+  test<Iter, Sent, 4>({1, 2, 3, 4}, 2, 23, {1, 23, 3, 4});
+  // no match
----------------
philnik wrote:
> var-const wrote:
> > Optional: consider adding comments to imitate named arguments, something like:
> > ```
> > test<Iter, Sent, 4>(/*input=*/{1, 2, 3, 4}, /*old=*/2, /*new=*/23, /*expected=*/{1, 23, 3, 4});
> > ```
> I'm using a struct with designated initializers. WDYT?
It looks great, definitely better than comments.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/ranges.replace.pass.cpp:45
+  // no match
+  test<Iter, Sent, 4>({1, 2, 3, 4}, 5, 23, {1, 2, 3, 4});
+  // all match
----------------
philnik wrote:
> var-const wrote:
> > Can this argument be deduced? (in all cases except for `0`, I guess)
> I tried it and couldn't get it to work.
Right -- looks like you would need to pass `std::array{1, 2, 3, 4}`. Let's leave as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126283