From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id C5057384AB74 for ; Fri, 19 Apr 2024 09:19:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C5057384AB74 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C5057384AB74 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713518344; cv=none; b=SgT10Wr+TUADoo8YSDY/wvEUyaZUISxZNyl6eXIpaGw8htVz6wYHlFfRbydOkU6HUzSYdd7tRznhUCzKkR3ll4F1euuFc3hAJb48v7gvpqdfXlVJCVInNaUs+hdMq5qlkkZcjGPZKKQe0w0OjZFF6ryE60lB/QH7/QN34tl9Y/k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713518344; c=relaxed/simple; bh=1tYYxeOEoaAybZ8uf2oGz2FJlkDj0/WZe4r6rvGbzaI=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=S6LgNaJPm0JhVzcJoNvqzql/a3rgo33RnOC2ecbpLe2PmbssN4BUhDnwnSPTGTXNBDv+FeKSBHam85Er3krbz47fjIu4oYlRpYZGKLlLIngzcobbmtN2jBW4sTU0W43bmwoJU8+2JfNdm09ExTuh+fCtCBs7Q86vvyMl3fg+AsI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713518341; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=77d52r0u/sMtBkzFJ+TqFoKwMfvd2rod+YgUjIqnnZ8=; b=XxDFCYSwr3Lz+qhdhiKN2QjeJjfBnwywui/syOq6WgOBpPHJanP0BiDRJooEb/usHNS00Y 5rM3ff3ADQmF6cWHTh1163LFAe5rU+LKyNNMoSxc5oZXm3XRSDZcaNXgbdlRcAtXbxhTyr 3Jku0QO2nHuyGmqFO2HBJ3dC6LwgizA= Received: from mail-ot1-f72.google.com (mail-ot1-f72.google.com [209.85.210.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-551-vZznqxfeP_CMqDzyPliZqg-1; Fri, 19 Apr 2024 05:18:59 -0400 X-MC-Unique: vZznqxfeP_CMqDzyPliZqg-1 Received: by mail-ot1-f72.google.com with SMTP id 46e09a7af769-6eb76c24132so2330602a34.2 for ; Fri, 19 Apr 2024 02:18:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713518339; x=1714123139; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=77d52r0u/sMtBkzFJ+TqFoKwMfvd2rod+YgUjIqnnZ8=; b=oFNbawXOWsV/yQGQHqJOSqJoiOYexMwz9ZStVk1hzmSrm66FFy9J0XanBktjuHfIFx Jl4SyB9kgBtmkuV5GpV53DIWXFWSkRuMobbVsUGf8WM4btvP6bS8v5cssbqmAmA4nt5P 3bbUuJt3DDRxP1BMWpFKgYMTZYV80d8qkeAOv+/oVdjLVyaWPPtND9VGvBye6Bf8DMIg 1qHIWX0pM4esQn31cWXRo3fSZJm1p1gJPrHNxhAy/prF4Nax0x2aVGMtQfmT1zkwz3IC hQ8K8povMuVnwBSKknLibax9RZAhiu56KtRNuPEPjvfGf5r9wHiMeUior/+ZelkeUKgz SXMw== X-Forwarded-Encrypted: i=1; AJvYcCWI/XoziqFf6bH4RFf5Cx9iyqA1BM0teL4f4g0GUsPirrBZIBkSrrukawk1eCpAF32dkqHWTTgGHpDBWXMqzE9FuTt4TkY= X-Gm-Message-State: AOJu0Yz01zVOUZp7dRcPmXG0jqicUNrzh/8x6Gq9hENgGqk/AjUopP1P aPWas63iGJVh/DRooLZyN8npJplTyDT5458VNCpiey3raM7o19ai921EOcgMGuFV/+N17ZtpcXP Z/Jqz1gRTj10ytkVI1vqPINUnlMiUR1WG4FjyR15KWz5cWt18T1m/oKuWAhmULmC1rIBvKAEt4z HoDlcleGn6Tew3XaRrW2cd3Kh7kBM= X-Received: by 2002:a05:6830:148a:b0:6eb:7abc:99b3 with SMTP id s10-20020a056830148a00b006eb7abc99b3mr1483211otq.11.1713518339032; Fri, 19 Apr 2024 02:18:59 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFcBgjoRLtcckgHCa0PqgyKIwdS67vNY+o1DL0x9GQ0/Rm57iUFDmbaBnN2QLhtoAUAKE0A1Bl59+TrAz9SrhM= X-Received: by 2002:a05:6830:148a:b0:6eb:7abc:99b3 with SMTP id s10-20020a056830148a00b006eb7abc99b3mr1483208otq.11.1713518338804; Fri, 19 Apr 2024 02:18:58 -0700 (PDT) MIME-Version: 1.0 References: <662013D8000158270C340001@message.bloomberg.net> In-Reply-To: From: Jonathan Wakely Date: Fri, 19 Apr 2024 10:18:42 +0100 Message-ID: Subject: Re: [PATCH] libstdc++: Fix std::ranges::iota is not included in numeric [PR108760] To: Patrick Palka Cc: "Michael Levine (BLOOMBERG/ 919 3RD A)" , libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-14.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SORBS_WEB,SPF_HELO_NONE,SPF_NONE,TXREP,URI_HEX autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, 18 Apr 2024 at 22:59, Patrick Palka 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 when it should have been sufficient to only include . > > > > 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 > > Date: Fri, 23 Feb 2024 14:13:13 -0500 > > Subject: [PATCH 1/2] Fix the bug > > > > Signed-off-by: Michael Levine > > --- > > 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 > > - struct out_value_result > > - { > > - [[no_unique_address]] _Out out; > > - [[no_unique_address]] _Tp value; > > - > > - template > > - requires convertible_to > > - && convertible_to > > - constexpr > > - operator out_value_result<_Out2, _Tp2>() const & > > - { return {out, value}; } > > - > > - template > > - requires convertible_to<_Out, _Out2> > > - && convertible_to<_Tp, _Tp2> > > - constexpr > > - operator out_value_result<_Out2, _Tp2>() && > > - { return {std::move(out), std::move(value)}; } > > - }; > > - > > - template > > - using iota_result = out_value_result<_Out, _Tp>; > > - > > - struct __iota_fn > > - { > > - template _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(__value); > > - ++__first; > > - ++__value; > > - } > > - return {std::move(__first), std::move(__value)}; > > - } > > - > > - template _Range> > > - constexpr iota_result, _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 > > #include > > #include // For _GLIBCXX_MOVE > > - > > +#include // 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 > > + struct out_value_result > > + { > > + [[no_unique_address]] _Out out; > > + [[no_unique_address]] _Tp value; > > + > > + template > > + requires convertible_to > > + && convertible_to > > + constexpr > > + operator out_value_result<_Out2, _Tp2>() const & > > + { return {out, value}; } > > + > > + template > > + 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 , so we > probably don't want to move it to (or one of its internal headers). > Better would be to move it to I think. > > > + > > + template > > + using iota_result = out_value_result<_Out, _Tp>; > > + > > + struct __iota_fn > > + { > > + template _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(__value); > > + ++__first; > > + ++__value; > > + } > > + return {std::move(__first), std::move(__value)}; > > + } > > + > > + template _Range> > > + constexpr iota_result, _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 > instead (and then include directly from > to bring in and out_value_result) since > has other users besides 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 > > 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 > > --- > > .../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 > > +// . > > + > > +// Fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108760 > > +// { dg-do run { target c++23 } } > > + > > +#include > > +#include > > + > > +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 instead of ? 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.