[libcxx-commits] [PATCH] D117395: [libc++] Adds a test for std::fill_n.

[Mark de Wever via Phabricator via libcxx-commits]

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


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp:55
+    Iter it = std::fill_n(Iter(ca), UDI(n), char(1));
+    assert(equal(it, ca + n));
     assert(ca[0] == 1);
----------------
Quuxplusone wrote:
> `assert(base(it) == ca + n);`
> and likewise below. All of our test iterators provide an ADL `base`, and we also provide one for pointers.
Thanks for the info I didn't know that.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp:79-85
     const unsigned n = 4;
     int ia[n] = {0};
-    assert(std::fill_n(ia, UDI(n), static_cast<char>(1)) == std::next(ia, n));
+    assert(std::fill_n(ia, UDI(n), static_cast<char>(1)) == ia + 4);
     assert(ia[0] == 1);
     assert(ia[1] == 1);
     assert(ia[2] == 1);
     assert(ia[3] == 1);
----------------
Quuxplusone wrote:
> Here and in all cases below, it would be nice to make it just
> ```
>     int a[4] = {};
>     assert(std::fill_n(a, UDI(4), char(1)) == a + 4);
>     assert(a[0] == 1); // etc 
> ```
> i.e. there's no reason for all these arrays to have names different from `a`, or for variable `n` to exist at all. (There also doesn't seem to be a reason for us to be filling //this// int array with `char(1)` instead of just `1`, but I didn't look closely. :))
I've updated the style, but I keep the `char(1)`. Not sure why this was done so I prefer to keep it that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117395