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

[Jonathan Wakely <jwakely@redhat.com>]

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
>