[libcxx-commits] [PATCH] D122074: [libc++] Allow an output_iterator in fill.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Interesting patch! On the one hand I don't really want to commit to this extension (extensions are evil!), however it does seem to me like moving instead of copying the iterator is an improvement, and it's trivial to do. It's also clearly conforming.

This leads me to wonder whether the standard could relax those restrictions itself. Would you have any interest in investigating that with a short paper?

I would suggest one of two things for the time being:

1. Either you add `std::move` to `std::fill` and start using it in `<format>`, but you put the rest of this patch on the ice until we have clarity about relaxing this in the Standard (i.e. you don't add tests or documentation that would commit us to an extension), or
2. You use `ranges::fill` instead in `<format>` and you put this patch on the ice until we have clarity, or you abandon it.

In other words, if you want to keep using `std::fill` instead of `ranges::fill` in `<format>`, I would be fine with relaxing the requirement as an implementation detail in `std::fill`, but not to commit to the extension until we have clarity about doing it in the Standard per se. I hope this makes sense?



================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp:151
     test_char<cpp17_output_iterator<char*> >();
+#if _LIBCPP_VERSION
+	// The iterator statisfies the output_iterator concept but not the
----------------



================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp:162
     test_int<cpp17_output_iterator<int*> >();
+#if _LIBCPP_VERSION
+	// The iterator statisfies the output_iterator concept but not the
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122074