public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Fix std::ranges::iota is not included in numeric [PR108760]
@ 2024-04-17 18:24 Michael Levine (BLOOMBERG/ 919 3RD A)
  2024-04-18 21:58 ` Patrick Palka
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Levine (BLOOMBERG/ 919 3RD A) @ 2024-04-17 18:24 UTC (permalink / raw)
  To: libstdc++, gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 750 bytes --]

This patch fixes GCC Bug 108760:  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108760

Before this patch, using std::ranges::iota required including <algorithm> when it should have been sufficient to only include <numeric>.

When the patch is applied, the following code will compile:  https://godbolt.org/z/33EPeqd1b

I added a test case for this change as well.

I built my local version of gcc using the following configuration:  $ ../gcc/configure --disable-bootstrap --prefix="$(pwd)/_pfx/" --enable-languages=c,c++,lto

and I tested my changes by running:  $ make check-c++ -jN -k

I ran this on the following OS:

Virtualization: wsl
Operating System: Ubuntu 20.04.6 LTS
Kernel: Linux 5.15.146.1-microsoft-standard-WSL2
Architecture: x86-64



[-- Attachment #2: 108760.patch --]
[-- Type: application/octet-stream, Size: 6656 bytes --]

From bd04070c281572ed7a3b48e3d33543e25b8c8afe Mon Sep 17 00:00:00 2001
From: Michael Levine <mlevine55@bloomberg.net>
Date: Fri, 23 Feb 2024 14:13:13 -0500
Subject: [PATCH 1/2] Fix the bug

Signed-off-by: Michael Levine <mlevine55@bloomberg.net>
---
 libstdc++-v3/include/bits/ranges_algo.h | 52 ----------------------
 libstdc++-v3/include/bits/stl_numeric.h | 57 ++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 53 deletions(-)

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/stl_numeric.h b/libstdc++-v3/include/bits/stl_numeric.h
index fe911154ab7..1b06c26dc02 100644
--- a/libstdc++-v3/include/bits/stl_numeric.h
+++ b/libstdc++-v3/include/bits/stl_numeric.h
@@ -59,7 +59,7 @@
 #include <bits/concept_check.h>
 #include <debug/debug.h>
 #include <bits/move.h> // For _GLIBCXX_MOVE
-
+#include <bits/ranges_base.h> // For _Range as used by std::ranges::iota
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
@@ -102,6 +102,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 #endif
 
+namespace ranges
+{
+#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
+}
+
 _GLIBCXX_END_NAMESPACE_VERSION
 
 _GLIBCXX_BEGIN_NAMESPACE_ALGO
-- 
2.25.1


From c52c8d79fb3c7dc9d2512d1635712ffcd3dea07c Mon Sep 17 00:00:00 2001
From: Michael Levine <mlevine55@bloomberg.net>
Date: Tue, 16 Apr 2024 16:45:37 -0400
Subject: [PATCH 2/2] Added a test to verify that the bug has been fixed

Signed-off-by: Michael Levine <mlevine55@bloomberg.net>
---
 .../testsuite/std/ranges/iota/108760.cc       | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 libstdc++-v3/testsuite/std/ranges/iota/108760.cc

diff --git a/libstdc++-v3/testsuite/std/ranges/iota/108760.cc b/libstdc++-v3/testsuite/std/ranges/iota/108760.cc
new file mode 100644
index 00000000000..4f71383687c
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/iota/108760.cc
@@ -0,0 +1,41 @@
+// Copyright (C) 2020-2024 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// Fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108760
+// { dg-do run { target c++23 } }
+
+#include <numeric>
+#include <testsuite_hooks.h>
+
+const int ARR_SIZE = 4;
+
+void
+test01()
+{
+    int expected_arr[ARR_SIZE] = {0, 1, 2, 3};
+    int input_arr[ARR_SIZE] = {0, 0, 0, 0};
+    std::ranges::iota(input_arr, 0);
+    for (int i = 0; i < ARR_SIZE; i++) {
+        VERIFY( input_arr[i] == expected_arr[i]);
+    }
+}
+
+int
+main()
+{
+    test01();
+}
-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libstdc++: Fix std::ranges::iota is not included in numeric [PR108760]
  2024-04-17 18:24 [PATCH] libstdc++: Fix std::ranges::iota is not included in numeric [PR108760] Michael Levine (BLOOMBERG/ 919 3RD A)
@ 2024-04-18 21:58 ` Patrick Palka
  2024-04-19  9:18   ` Jonathan Wakely
  0 siblings, 1 reply; 3+ messages in thread
From: Patrick Palka @ 2024-04-18 21:58 UTC (permalink / raw)
  To: Michael Levine (BLOOMBERG/ 919 3RD A); +Cc: libstdc++, gcc-patches

On Wed, 17 Apr 2024, Michael Levine (BLOOMBERG/ 919 3RD A) wrote:

> This patch fixes GCC Bug 108760: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108760
> Before this patch, using std::ranges::iota required including <algorithm> when it should have been sufficient to only include <numeric>.
> 
> When the patch is applied, the following code will compile: https://godbolt.org/z/33EPeqd1b
> 
> I added a test case for this change as well.
> 
> I built my local version of gcc using the following configuration: $ ../gcc/configure --disable-bootstrap --prefix="$(pwd)/_pfx/" --enable-languages=c,c++,lto
> 
> and I tested my changes by running: $ make check-c++ -jN -k

Nice, thanks for the patch!

> 
> I ran this on the following OS:
> 
> Virtualization: wsl
> Operating System: Ubuntu 20.04.6 LTS
> Kernel: Linux 5.15.146.1-microsoft-standard-WSL2
> Architecture: x86-64

> From bd04070c281572ed7a3b48e3d33543e25b8c8afe Mon Sep 17 00:00:00 2001
> From: Michael Levine <mlevine55@bloomberg.net>
> Date: Fri, 23 Feb 2024 14:13:13 -0500
> Subject: [PATCH 1/2] Fix the bug
> 
> Signed-off-by: Michael Levine <mlevine55@bloomberg.net>
> ---
>  libstdc++-v3/include/bits/ranges_algo.h | 52 ----------------------
>  libstdc++-v3/include/bits/stl_numeric.h | 57 ++++++++++++++++++++++++-
>  2 files changed, 56 insertions(+), 53 deletions(-)
> 
> 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/stl_numeric.h b/libstdc++-v3/include/bits/stl_numeric.h
> index fe911154ab7..1b06c26dc02 100644
> --- a/libstdc++-v3/include/bits/stl_numeric.h
> +++ b/libstdc++-v3/include/bits/stl_numeric.h
> @@ -59,7 +59,7 @@
>  #include <bits/concept_check.h>
>  #include <debug/debug.h>
>  #include <bits/move.h> // For _GLIBCXX_MOVE
> -
> +#include <bits/ranges_base.h> // For _Range as used by std::ranges::iota
>  
>  namespace std _GLIBCXX_VISIBILITY(default)
>  {
> @@ -102,6 +102,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      }
>  #endif
>  
> +namespace ranges
> +{
> +#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)}; }
> +    };

IIUC out_value_result should continue to be available from <algorithm>, so we
probably don't want to move it to <numeric> (or one of its internal headers).
Better would be to move it to <bits/ranges_algobase.h> I think.

> +
> +  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

And it might be cleaner to move ranges::iota directly into <numeric>
instead (and then include <bits/ranges_algobase.h> directly from <numeric>
to bring in <bits/ranges_base.h> and out_value_result) since
<bits/stl_numeric.h> has other users besides <numeric> which would
otherwise get unnecessarily bloated.

> +}
> +
>  _GLIBCXX_END_NAMESPACE_VERSION
>  
>  _GLIBCXX_BEGIN_NAMESPACE_ALGO
> -- 
> 2.25.1
> 
> 
> From c52c8d79fb3c7dc9d2512d1635712ffcd3dea07c Mon Sep 17 00:00:00 2001
> From: Michael Levine <mlevine55@bloomberg.net>
> Date: Tue, 16 Apr 2024 16:45:37 -0400
> Subject: [PATCH 2/2] Added a test to verify that the bug has been fixed
> 
> Signed-off-by: Michael Levine <mlevine55@bloomberg.net>
> ---
>  .../testsuite/std/ranges/iota/108760.cc       | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 libstdc++-v3/testsuite/std/ranges/iota/108760.cc
> 
> diff --git a/libstdc++-v3/testsuite/std/ranges/iota/108760.cc b/libstdc++-v3/testsuite/std/ranges/iota/108760.cc
> new file mode 100644
> index 00000000000..4f71383687c
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/ranges/iota/108760.cc
> @@ -0,0 +1,41 @@
> +// Copyright (C) 2020-2024 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library.  This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +
> +// You should have received a copy of the GNU General Public License along
> +// with this library; see the file COPYING3.  If not see
> +// <http://www.gnu.org/licenses/>.
> +
> +// Fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108760
> +// { dg-do run { target c++23 } }
> +
> +#include <numeric>
> +#include <testsuite_hooks.h>
> +
> +const int ARR_SIZE = 4;
> +
> +void
> +test01()
> +{
> +    int expected_arr[ARR_SIZE] = {0, 1, 2, 3};
> +    int input_arr[ARR_SIZE] = {0, 0, 0, 0};
> +    std::ranges::iota(input_arr, 0);
> +    for (int i = 0; i < ARR_SIZE; i++) {
> +        VERIFY( input_arr[i] == expected_arr[i]);
> +    }
> +}
> +
> +int
> +main()
> +{
> +    test01();
> +}

Instead of adding a new test, perhaps we should just move the
existing test libstdc++-v3/testsuite/25_algorithms/iota/1.cc
to libstdc++-v3/testsuite/26_numerics/iota/1.cc and change it
to include <numeric> instead of <algorithm>?

> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libstdc++: Fix std::ranges::iota is not included in numeric [PR108760]
  2024-04-18 21:58 ` Patrick Palka
@ 2024-04-19  9:18   ` Jonathan Wakely
  0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2024-04-19  9:18 UTC (permalink / raw)
  To: Patrick Palka
  Cc: Michael Levine (BLOOMBERG/ 919 3RD A), libstdc++, gcc-patches

On Thu, 18 Apr 2024 at 22:59, Patrick Palka <ppalka@redhat.com> wrote:
>
> On Wed, 17 Apr 2024, Michael Levine (BLOOMBERG/ 919 3RD A) wrote:
>
> > This patch fixes GCC Bug 108760: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108760
> > Before this patch, using std::ranges::iota required including <algorithm> when it should have been sufficient to only include <numeric>.
> >
> > When the patch is applied, the following code will compile: https://godbolt.org/z/33EPeqd1b
> >
> > I added a test case for this change as well.
> >
> > I built my local version of gcc using the following configuration: $ ../gcc/configure --disable-bootstrap --prefix="$(pwd)/_pfx/" --enable-languages=c,c++,lto
> >
> > and I tested my changes by running: $ make check-c++ -jN -k
>
> Nice, thanks for the patch!
>
> >
> > I ran this on the following OS:
> >
> > Virtualization: wsl
> > Operating System: Ubuntu 20.04.6 LTS
> > Kernel: Linux 5.15.146.1-microsoft-standard-WSL2
> > Architecture: x86-64
>
> > From bd04070c281572ed7a3b48e3d33543e25b8c8afe Mon Sep 17 00:00:00 2001
> > From: Michael Levine <mlevine55@bloomberg.net>
> > Date: Fri, 23 Feb 2024 14:13:13 -0500
> > Subject: [PATCH 1/2] Fix the bug
> >
> > Signed-off-by: Michael Levine <mlevine55@bloomberg.net>
> > ---
> >  libstdc++-v3/include/bits/ranges_algo.h | 52 ----------------------
> >  libstdc++-v3/include/bits/stl_numeric.h | 57 ++++++++++++++++++++++++-
> >  2 files changed, 56 insertions(+), 53 deletions(-)
> >
> > 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/stl_numeric.h b/libstdc++-v3/include/bits/stl_numeric.h
> > index fe911154ab7..1b06c26dc02 100644
> > --- a/libstdc++-v3/include/bits/stl_numeric.h
> > +++ b/libstdc++-v3/include/bits/stl_numeric.h
> > @@ -59,7 +59,7 @@
> >  #include <bits/concept_check.h>
> >  #include <debug/debug.h>
> >  #include <bits/move.h> // For _GLIBCXX_MOVE
> > -
> > +#include <bits/ranges_base.h> // For _Range as used by std::ranges::iota
> >
> >  namespace std _GLIBCXX_VISIBILITY(default)
> >  {
> > @@ -102,6 +102,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >      }
> >  #endif
> >
> > +namespace ranges
> > +{
> > +#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)}; }
> > +    };
>
> IIUC out_value_result should continue to be available from <algorithm>, so we
> probably don't want to move it to <numeric> (or one of its internal headers).
> Better would be to move it to <bits/ranges_algobase.h> I think.
>
> > +
> > +  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
>
> And it might be cleaner to move ranges::iota directly into <numeric>
> instead (and then include <bits/ranges_algobase.h> directly from <numeric>
> to bring in <bits/ranges_base.h> and out_value_result) since
> <bits/stl_numeric.h> has other users besides <numeric> which would
> otherwise get unnecessarily bloated.

Good idea, thanks, Patrick.

> > +}
> > +
> >  _GLIBCXX_END_NAMESPACE_VERSION
> >
> >  _GLIBCXX_BEGIN_NAMESPACE_ALGO
> > --
> > 2.25.1
> >
> >
> > From c52c8d79fb3c7dc9d2512d1635712ffcd3dea07c Mon Sep 17 00:00:00 2001
> > From: Michael Levine <mlevine55@bloomberg.net>
> > Date: Tue, 16 Apr 2024 16:45:37 -0400
> > Subject: [PATCH 2/2] Added a test to verify that the bug has been fixed
> >
> > Signed-off-by: Michael Levine <mlevine55@bloomberg.net>
> > ---
> >  .../testsuite/std/ranges/iota/108760.cc       | 41 +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 libstdc++-v3/testsuite/std/ranges/iota/108760.cc
> >
> > diff --git a/libstdc++-v3/testsuite/std/ranges/iota/108760.cc b/libstdc++-v3/testsuite/std/ranges/iota/108760.cc
> > new file mode 100644
> > index 00000000000..4f71383687c
> > --- /dev/null
> > +++ b/libstdc++-v3/testsuite/std/ranges/iota/108760.cc
> > @@ -0,0 +1,41 @@
> > +// Copyright (C) 2020-2024 Free Software Foundation, Inc.
> > +//
> > +// This file is part of the GNU ISO C++ Library.  This library is free
> > +// software; you can redistribute it and/or modify it under the
> > +// terms of the GNU General Public License as published by the
> > +// Free Software Foundation; either version 3, or (at your option)
> > +// any later version.
> > +
> > +// This library is distributed in the hope that it will be useful,
> > +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +// GNU General Public License for more details.
> > +
> > +// You should have received a copy of the GNU General Public License along
> > +// with this library; see the file COPYING3.  If not see
> > +// <http://www.gnu.org/licenses/>.
> > +
> > +// Fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108760
> > +// { dg-do run { target c++23 } }
> > +
> > +#include <numeric>
> > +#include <testsuite_hooks.h>
> > +
> > +const int ARR_SIZE = 4;
> > +
> > +void
> > +test01()
> > +{
> > +    int expected_arr[ARR_SIZE] = {0, 1, 2, 3};
> > +    int input_arr[ARR_SIZE] = {0, 0, 0, 0};
> > +    std::ranges::iota(input_arr, 0);
> > +    for (int i = 0; i < ARR_SIZE; i++) {
> > +        VERIFY( input_arr[i] == expected_arr[i]);
> > +    }
> > +}
> > +
> > +int
> > +main()
> > +{
> > +    test01();
> > +}
>
> Instead of adding a new test, perhaps we should just move the
> existing test libstdc++-v3/testsuite/25_algorithms/iota/1.cc
> to libstdc++-v3/testsuite/26_numerics/iota/1.cc and change it
> to include <numeric> instead of <algorithm>?


Yes please, the existing test is better as it uses an output range
instead of a contiguous range for the result. And the new test has
incorrect copyright/licence info. As a new test, it should not have
2020-2024 as the copyright date, and it should not be copyright FSF
unless Michael (or Bloomberg) has a copyright assignment on file to
assign copyright to the FSF. Since Michael added a DCO Signed-off-by:
tag, I assume this is being contributed under the
https://gcc.gnu.org/dco.html terms, not under copyright assignment, so
the test should not be copyright FSF at all.

I don't bother adding copyright and license info to trivial tests like
this, it's not doing anything meaningful and so I don't consider it
copyrightable. There's no novel implementation or non-trivial logic in
this test code, it's literally just exercising an API for the purpose
of testing it. There is not really any other way to do that, so there
is no "originality" in that work.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-04-19  9:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 18:24 [PATCH] libstdc++: Fix std::ranges::iota is not included in numeric [PR108760] Michael Levine (BLOOMBERG/ 919 3RD A)
2024-04-18 21:58 ` Patrick Palka
2024-04-19  9:18   ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).