[libcxx-commits] [PATCH] D130552: [libc++][ranges] Implement `ranges::generate{, _n}`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_generate.h:47
   template <input_or_output_iterator _OutIter, sentinel_for<_OutIter> _Sent, copy_constructible _Func>
   requires invocable<_Func&> && indirectly_writable<_OutIter, invoke_result_t<_Func&>>
   _LIBCPP_HIDE_FROM_ABI constexpr
----------------
This is a weird constraint, I would have expected `_OutIter` to be an `output_iterator` instead.

I don't think we are testing for this, and I'm not asking that you add tests for it. However, it looks like the Standard doesn't want implementations to assume that `*it++ = expr` is valid, and they want us to use `*it = expr; ++it` instead (which we do).


================
Comment at: libcxx/include/__algorithm/ranges_generate_n.h:42
+    for (; __n > 0; --__n) {
+      *__first = __gen();
+      ++__first;
----------------
We should technically be using `std::invoke` here according to the concept requirements specified in the signature (all we have is `invocable<_Func&>`). However, I don't think there is any way to distinguish `__gen()` from `invoke(gen)` given that it's being called with no arguments. I tried finding a case but couldn't.

So, nothing to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130552