Re: [PATCH v3] libstdc++: Fix std::ranges::iota not in numeric [PR108760]

[Jonathan Wakely <jwakely@redhat.com>]

On 24/05/24 13:56 -0000, Michael Levine (BLOOMBERG/ 731 LEX) wrote:
>I've attached the v3 version of the patch as a single, squashed patch containing all of the changes.  I manually prepended my sign off to the patch.


>Signed-off-by: Michael Levine <mlevine55@bloomberg.net>
>---
>diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
>index 62faff173bd..d258be0b93f 100644
>--- a/libstdc++-v3/include/bits/ranges_algo.h
>+++ b/libstdc++-v3/include/bits/ranges_algo.h
>@@ -3521,58 +3521,6 @@ namespace ranges
> 
> #endif // __glibcxx_ranges_contains
> 
>-#if __glibcxx_ranges_iota >= 202202L // C++ >= 23
>-
>-  template<typename _Out, typename _Tp>
>-    struct out_value_result
>-    {
>-      [[no_unique_address]] _Out out;
>-      [[no_unique_address]] _Tp value;
>-
>-      template<typename _Out2, typename _Tp2>
>-	requires convertible_to<const _Out&, _Out2>
>-	  && convertible_to<const _Tp&, _Tp2>
>-	constexpr
>-	operator out_value_result<_Out2, _Tp2>() const &
>-	{ return {out, value}; }
>-
>-      template<typename _Out2, typename _Tp2>
>-	requires convertible_to<_Out, _Out2>
>-	  && convertible_to<_Tp, _Tp2>
>-	constexpr
>-	operator out_value_result<_Out2, _Tp2>() &&
>-	{ return {std::move(out), std::move(value)}; }
>-    };
>-
>-  template<typename _Out, typename _Tp>
>-    using iota_result = out_value_result<_Out, _Tp>;
>-
>-  struct __iota_fn
>-  {
>-    template<input_or_output_iterator _Out, sentinel_for<_Out> _Sent, weakly_incrementable _Tp>
>-      requires indirectly_writable<_Out, const _Tp&>
>-      constexpr iota_result<_Out, _Tp>
>-      operator()(_Out __first, _Sent __last, _Tp __value) const
>-      {
>-	while (__first != __last)
>-	  {
>-	    *__first = static_cast<const _Tp&>(__value);
>-	    ++__first;
>-	    ++__value;
>-	  }
>-	return {std::move(__first), std::move(__value)};
>-      }
>-
>-    template<weakly_incrementable _Tp, output_range<const _Tp&> _Range>
>-      constexpr iota_result<borrowed_iterator_t<_Range>, _Tp>
>-      operator()(_Range&& __r, _Tp __value) const
>-      { return (*this)(ranges::begin(__r), ranges::end(__r), std::move(__value)); }
>-  };
>-
>-  inline constexpr __iota_fn iota{};
>-
>-#endif // __glibcxx_ranges_iota
>-
> #if __glibcxx_ranges_find_last >= 202207L // C++ >= 23
> 
>   struct __find_last_fn
>diff --git a/libstdc++-v3/include/bits/ranges_algobase.h b/libstdc++-v3/include/bits/ranges_algobase.h
>index e26a73a27d6..965b36aed35 100644
>--- a/libstdc++-v3/include/bits/ranges_algobase.h
>+++ b/libstdc++-v3/include/bits/ranges_algobase.h
>@@ -35,6 +35,7 @@
> #include <compare>
> #include <bits/stl_iterator_base_funcs.h>
> #include <bits/stl_iterator.h>
>+#include <bits/stl_algobase.h> // __memcpy

Why is this being added here? What is __memcpy?

I don't think out_value_result requires any new headers to be included
here, does it?

> #include <bits/ranges_base.h> // ranges::begin, ranges::range etc.
> #include <bits/invoke.h>      // __invoke
> #include <bits/cpp_type_traits.h> // __is_byte
>@@ -70,6 +71,32 @@ namespace ranges
> 	__is_move_iterator<move_iterator<_Iterator>> = true;
>   } // namespace __detail
> 
>+#if __glibcxx_ranges_iota >= 202202L // C++ >= 23
>+
>+    template<typename _Out, typename _Tp>
>+    struct out_value_result
>+    {
>+        [[no_unique_address]] _Out out;
>+        [[no_unique_address]] _Tp value;
>+
>+        template<typename _Out2, typename _Tp2>
>+	requires convertible_to<const _Out&, _Out2>
>+        && convertible_to<const _Tp&, _Tp2>
>+	constexpr
>+	operator out_value_result<_Out2, _Tp2>() const &
>+	{ return {out, value}; }
>+
>+        template<typename _Out2, typename _Tp2>
>+	requires convertible_to<_Out, _Out2>
>+        && convertible_to<_Tp, _Tp2>
>+	constexpr
>+	operator out_value_result<_Out2, _Tp2>() &&
>+	{ return {std::move(out), std::move(value)}; }
>+    };
>+
>+#endif // __glibcxx_ranges_iota
>+
>+
>   struct __equal_fn
>   {
>     template<input_iterator _Iter1, sentinel_for<_Iter1> _Sent1,
>diff --git a/libstdc++-v3/include/std/numeric b/libstdc++-v3/include/std/numeric
>index c912db4a519..d88f7f02137 100644
>--- a/libstdc++-v3/include/std/numeric
>+++ b/libstdc++-v3/include/std/numeric
>@@ -65,6 +65,10 @@
> # include <parallel/numeric>
> #endif
> 
>+#if __glibcxx_ranges_iota >= 202202L // C++ >= 23
>+#include <bits/ranges_algobase.h> // for out_value_result as used by std::ranges::iota.  It transitively also brings in <bits/ranges_base.h>, from which _Range is used by std::ranges::iota

We generally try to keep lines below 80 columns, or 120 at a push.
This is unnecessarily long, and I don't know what _Range is meant to
be (that's just a template parameter, not something defined in
<bits/ranges_algobase.h>. Please just use:

#include <bits/ranges_algobase.h> // for ranges::out_value_result

Otherwise this looks ready to go in, thanks.

>+#endif // __glibcxx_ranges_iota
>+
> #if __cplusplus >= 201402L
> # include <type_traits>
> # include <bit>
>@@ -726,6 +730,40 @@ namespace __detail
>   /// @} group numeric_ops
> #endif // C++17
> 
>+namespace ranges
>+{
>+#if __glibcxx_ranges_iota >= 202202L // C++ >= 23
>+
>+  template<typename _Out, typename _Tp>
>+  using iota_result = out_value_result<_Out, _Tp>;
>+
>+  struct __iota_fn
>+  {
>+    template<input_or_output_iterator _Out, sentinel_for<_Out> _Sent, weakly_incrementable _Tp>
>+      requires indirectly_writable<_Out, const _Tp&>
>+      constexpr iota_result<_Out, _Tp>
>+      operator()(_Out __first, _Sent __last, _Tp __value) const
>+      {
>+	while (__first != __last)
>+	  {
>+	    *__first = static_cast<const _Tp&>(__value);
>+	    ++__first;
>+	    ++__value;
>+	  }
>+	return {std::move(__first), std::move(__value)};
>+      }
>+
>+    template<weakly_incrementable _Tp, output_range<const _Tp&> _Range>
>+      constexpr iota_result<borrowed_iterator_t<_Range>, _Tp>
>+      operator()(_Range&& __r, _Tp __value) const
>+      { return (*this)(ranges::begin(__r), ranges::end(__r), std::move(__value)); }
>+  };
>+
>+  inline constexpr __iota_fn iota{};
>+
>+#endif // __glibcxx_ranges_iota
>+} // namespace ranges
>+
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace std
> 
>diff --git a/libstdc++-v3/testsuite/25_algorithms/iota/1.cc b/libstdc++-v3/testsuite/26_numerics/iota/2.cc
>similarity index 96%
>rename from libstdc++-v3/testsuite/25_algorithms/iota/1.cc
>rename to libstdc++-v3/testsuite/26_numerics/iota/2.cc
>index 61bf418b4da..040c48d91ce 100644
>--- a/libstdc++-v3/testsuite/25_algorithms/iota/1.cc
>+++ b/libstdc++-v3/testsuite/26_numerics/iota/2.cc
>@@ -1,6 +1,6 @@
> // { dg-do run { target c++23 } }
> 
>-#include <algorithm>
>+#include <numeric>
> #include <testsuite_hooks.h>
> #include <testsuite_iterators.h>
>