[PATCH 0/2] A few minor fixes in <generator>

[PATCH 0/2] A few minor fixes in <generator>

[Arsen Arsenović]

From: Arsen Arsenović <arsen@gcc.gnu.org>

Afternoon!

These couple of patches fix two minor issues in the generator
implementation that were informally reported a while ago.

Tested on x86_64-pc-linux-gnu.

OK for trunk?

TIA, have a lovely day!

Arsen Arsenović (2):
  libstdc++: fix _V badname in <generator>
  libstdc++: fix generator iterator operator* return type

 libstdc++-v3/include/std/generator            | 22 ++++++------
 .../range_generators/iter_deref_return.cc     | 34 +++++++++++++++++++
 2 files changed, 46 insertions(+), 10 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc

-- 
2.44.0

[PATCH 1/2] libstdc++: fix _V badname in <generator>

[Arsen Arsenović]

libstdc++-v3/ChangeLog:

	* include/std/generator: Fix _V badname.
---
 libstdc++-v3/include/std/generator | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/libstdc++-v3/include/std/generator b/libstdc++-v3/include/std/generator
index 87983ee5e7c6..2d1dcced1e57 100644
--- a/libstdc++-v3/include/std/generator
+++ b/libstdc++-v3/include/std/generator
@@ -76,14 +76,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    * @headerfile generator
    * @since C++23
    */
-  template<typename _Ref, typename _V = void, typename _Alloc = void>
+  template<typename _Ref, typename _Val = void, typename _Alloc = void>
     class generator;
 
   /// @cond undocumented
   namespace __gen
   {
     /// _Reference type for a generator whose reference (first argument) and
-    /// value (second argument) types are _Ref and _V.
+    /// value (second argument) types are _Ref and _Val.
     template<typename _Ref, typename _Val>
     using _Reference_t = __conditional_t<is_void_v<_Val>,
 					 _Ref&&, _Ref>;
@@ -642,14 +642,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   } // namespace __gen
   /// @endcond
 
-  template<typename _Ref, typename _V, typename _Alloc>
+  template<typename _Ref, typename _Val, typename _Alloc>
     class generator
-      : public ranges::view_interface<generator<_Ref, _V, _Alloc>>
+      : public ranges::view_interface<generator<_Ref, _Val, _Alloc>>
     {
-      using _Value = __conditional_t<is_void_v<_V>, remove_cvref_t<_Ref>, _V>;
+      using _Value = __conditional_t<is_void_v<_Val>,
+				     remove_cvref_t<_Ref>,
+				     _Val>;
       static_assert(__gen::_Cv_unqualified_object<_Value>,
 		    "Generator value must be a cv-unqualified object type");
-      using _Reference = __gen::_Reference_t<_Ref, _V>;
+      using _Reference = __gen::_Reference_t<_Ref, _Val>;
       static_assert(is_reference_v<_Reference>
 		    || (__gen::_Cv_unqualified_object<_Reference>
 			&& copy_constructible<_Reference>),
@@ -737,8 +739,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       bool _M_began = false;
     };
 
-  template<class _Ref, class _V, class _Alloc>
-    struct generator<_Ref, _V, _Alloc>::_Iterator
+  template<class _Ref, class _Val, class _Alloc>
+    struct generator<_Ref, _Val, _Alloc>::_Iterator
     {
       using value_type = _Value;
       using difference_type = ptrdiff_t;
-- 
2.44.0

[PATCH 2/2] libstdc++: fix generator iterator operator* return type

[Arsen Arsenović]

Per the standard, the return type of a generators ranges iterator op*
should be the reference type rather than the yielded type.

The yielded type was used here by mistake.

libstdc++-v3/ChangeLog:

	* include/std/generator (generator::_Iterator::operator*): Fix
	return type.
	* testsuite/24_iterators/range_generators/iter_deref_return.cc:
	New test.
---
 libstdc++-v3/include/std/generator            |  4 +--
 .../range_generators/iter_deref_return.cc     | 34 +++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc

diff --git a/libstdc++-v3/include/std/generator b/libstdc++-v3/include/std/generator
index 2d1dcced1e57..789016b5a883 100644
--- a/libstdc++-v3/include/std/generator
+++ b/libstdc++-v3/include/std/generator
@@ -773,12 +773,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator++(int)
       { this->operator++(); }
 
-      yielded
+      _Reference
       operator*()
 	const noexcept(is_nothrow_move_constructible_v<_Reference>)
       {
 	auto& __p = this->_M_coro.promise();
-	return static_cast<yielded>(*__p._M_value());
+	return static_cast<_Reference>(*__p._M_value());
       }
 
     private:
diff --git a/libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc b/libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc
new file mode 100644
index 000000000000..7bdbf4d489ec
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc
@@ -0,0 +1,34 @@
+// { dg-do compile { target c++23 } }
+// Copyright (C) 2023-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.
+
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+// <http://www.gnu.org/licenses/>.
+
+#include <generator>
+
+// Check that the return type of iterator::operator* is the reference type.
+// Pre-op* return type fix, this'd have resulted in a op* return type of const
+// bool&.
+
+std::generator<bool, bool>
+foo();
+
+static_assert(std::is_same_v<decltype(*foo().begin()), bool>);
+static_assert(std::is_same_v<typename decltype(foo())::yielded, const bool&>);
-- 
2.44.0

Re: [PATCH 2/2] libstdc++: fix generator iterator operator* return type

[Jonathan Wakely]

On Sat, 23 Mar 2024 at 15:47, Arsen Arsenović <arsen@aarsen.me> wrote:
>
> Per the standard, the return type of a generators ranges iterator op*
> should be the reference type rather than the yielded type.
>
> The yielded type was used here by mistake.
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/generator (generator::_Iterator::operator*): Fix
>         return type.
>         * testsuite/24_iterators/range_generators/iter_deref_return.cc:
>         New test.
> ---
>  libstdc++-v3/include/std/generator            |  4 +--
>  .../range_generators/iter_deref_return.cc     | 34 +++++++++++++++++++
>  2 files changed, 36 insertions(+), 2 deletions(-)
>  create mode 100644 libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc
>
> diff --git a/libstdc++-v3/include/std/generator b/libstdc++-v3/include/std/generator
> index 2d1dcced1e57..789016b5a883 100644
> --- a/libstdc++-v3/include/std/generator
> +++ b/libstdc++-v3/include/std/generator
> @@ -773,12 +773,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        operator++(int)
>        { this->operator++(); }
>
> -      yielded
> +      _Reference
>        operator*()
>         const noexcept(is_nothrow_move_constructible_v<_Reference>)
>        {
>         auto& __p = this->_M_coro.promise();
> -       return static_cast<yielded>(*__p._M_value());
> +       return static_cast<_Reference>(*__p._M_value());
>        }
>
>      private:
> diff --git a/libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc b/libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc
> new file mode 100644
> index 000000000000..7bdbf4d489ec
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/24_iterators/range_generators/iter_deref_return.cc
> @@ -0,0 +1,34 @@
> +// { dg-do compile { target c++23 } }
> +// Copyright (C) 2023-2024 Free Software Foundation, Inc.

Just 2024 here.

> +//
> +// 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.
> +
> +// Under Section 7 of GPL version 3, you are granted additional
> +// permissions described in the GCC Runtime Library Exception, version
> +// 3.1, as published by the Free Software Foundation.
> +
> +// You should have received a copy of the GNU General Public License and
> +// a copy of the GCC Runtime Library Exception along with this program;
> +// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> +// <http://www.gnu.org/licenses/>.


I don't think we want the last two paragraphs about the runtime
exception. That's relevant to code being compiled into the users
binary as inline functions or templates, which doesn't matter for
testcases.

I see we have 10 existing tests that use the runtime exception text,
which I'll fix. For comparison, there are 9000 tests with the GPL
text, so the vast majority don't have the exception.

OK for trunk with the copyright date fixed and the exception text
dropped, thanks.

> +
> +#include <generator>
> +
> +// Check that the return type of iterator::operator* is the reference type.
> +// Pre-op* return type fix, this'd have resulted in a op* return type of const
> +// bool&.
> +
> +std::generator<bool, bool>
> +foo();
> +
> +static_assert(std::is_same_v<decltype(*foo().begin()), bool>);
> +static_assert(std::is_same_v<typename decltype(foo())::yielded, const bool&>);
> --
> 2.44.0
>

Re: [PATCH 1/2] libstdc++: fix _V badname in <generator>

[Jonathan Wakely]

On Sat, 23 Mar 2024 at 15:47, Arsen Arsenović wrote:
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/generator: Fix _V badname.

OK for trunk, thanks.


> ---
>  libstdc++-v3/include/std/generator | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/generator b/libstdc++-v3/include/std/generator
> index 87983ee5e7c6..2d1dcced1e57 100644
> --- a/libstdc++-v3/include/std/generator
> +++ b/libstdc++-v3/include/std/generator
> @@ -76,14 +76,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     * @headerfile generator
>     * @since C++23
>     */
> -  template<typename _Ref, typename _V = void, typename _Alloc = void>
> +  template<typename _Ref, typename _Val = void, typename _Alloc = void>
>      class generator;
>
>    /// @cond undocumented
>    namespace __gen
>    {
>      /// _Reference type for a generator whose reference (first argument) and
> -    /// value (second argument) types are _Ref and _V.
> +    /// value (second argument) types are _Ref and _Val.
>      template<typename _Ref, typename _Val>
>      using _Reference_t = __conditional_t<is_void_v<_Val>,
>                                          _Ref&&, _Ref>;
> @@ -642,14 +642,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    } // namespace __gen
>    /// @endcond
>
> -  template<typename _Ref, typename _V, typename _Alloc>
> +  template<typename _Ref, typename _Val, typename _Alloc>
>      class generator
> -      : public ranges::view_interface<generator<_Ref, _V, _Alloc>>
> +      : public ranges::view_interface<generator<_Ref, _Val, _Alloc>>
>      {
> -      using _Value = __conditional_t<is_void_v<_V>, remove_cvref_t<_Ref>, _V>;
> +      using _Value = __conditional_t<is_void_v<_Val>,
> +                                    remove_cvref_t<_Ref>,
> +                                    _Val>;
>        static_assert(__gen::_Cv_unqualified_object<_Value>,
>                     "Generator value must be a cv-unqualified object type");
> -      using _Reference = __gen::_Reference_t<_Ref, _V>;
> +      using _Reference = __gen::_Reference_t<_Ref, _Val>;
>        static_assert(is_reference_v<_Reference>
>                     || (__gen::_Cv_unqualified_object<_Reference>
>                         && copy_constructible<_Reference>),
> @@ -737,8 +739,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        bool _M_began = false;
>      };
>
> -  template<class _Ref, class _V, class _Alloc>
> -    struct generator<_Ref, _V, _Alloc>::_Iterator
> +  template<class _Ref, class _Val, class _Alloc>
> +    struct generator<_Ref, _Val, _Alloc>::_Iterator
>      {
>        using value_type = _Value;
>        using difference_type = ptrdiff_t;
> --
> 2.44.0
>