[libcxx-commits] [PATCH] D123462: [libc++] Implement ranges::fill{, _n}

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const requested changes to this revision.
var-const added a comment.
This revision now requires changes to proceed.

Almost LGTM (just one remaining comment). Thanks!



================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill_n.pass.cpp:27
+  { // simple test
+    int a[3];
+    std::same_as<It> auto ret = std::ranges::fill_n(It(a), 3, 1);
----------------
philnik wrote:
> var-const wrote:
> > Can you also try with a non-trivially-copyable type? (e.g. to make sure that the implementation doesn't just unconditionally use `memset`)
> I think we have this discussion in another PR as well. Would it be OK for you if I just add that it's testing non-trivially-copyable things to the description of `// check that every element is copied once`?
I'd prefer to have these test cases separate. For the non-trivially-copyable test, I'd use something that contains a pointer to make sure pointers are deep-copied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123462