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.133.124]) by sourceware.org (Postfix) with ESMTPS id 93006384AB6F for ; Fri, 19 Apr 2024 09:19:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 93006384AB6F 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 93006384AB6F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.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-f71.google.com (mail-ot1-f71.google.com [209.85.210.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-636-ndAMopuBNf2GqOaq2jD73w-1; Fri, 19 Apr 2024 05:18:59 -0400 X-MC-Unique: ndAMopuBNf2GqOaq2jD73w-1 Received: by mail-ot1-f71.google.com with SMTP id 46e09a7af769-6eb76c24132so2330601a34.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=T7j7/ExzcY1PXkMo7vn2JS9FTsPO0hxUqY+jcWXgCNokb/IijqgJ8eJxkppL02xq1H 2JGFqOfhXFjMFl49SIHPAQxLtbdVMDCLuLjAydRG6vao5vZmlKCVkwaUgGjrABc/DFQl aJBlFNAXBGNfElcKtr6hF7Y+ZeMB92y+mvHu6f22k24ik+zL70Jj4Xg7sJj79fKiIwz1 uMcyHzOf3MJ4zvO1PLvv2NHUj22SeFWcZ5eoAQw7svY/L/5OCeC1mJtv9ofUl1L0umfA MIgYa5wuftFPqYqdTahOWk2GpIfDO+0DzYC5scrr7/LoWbq4lYU6ektR0hkhWWm7RYg+ 3Z+w== X-Forwarded-Encrypted: i=1; AJvYcCWhOUiM7OsguGfiYhUtdyWwOk0IgG2wuGlcfbEba9VHsJi/Gufymma3ePaVnuskLtUmmowJlsR4OHnzoBccSqGIFe3uXcqz2g== X-Gm-Message-State: AOJu0Yxv4jLHT+nX3KnOidlHG/KUH/VQ7C3+vA2UfA6LidtTx+PRXy5L MnWCjW4Svz4TBoUwjsCFbqK3fy8fskAJaoHXKnIt9PqyU1A/fNnadFRExaI4afHEh0Xpgw7Q052 N3TynmwiXelmpCF63Vz024OeohGcp0uHJ7RGTjDcZNHGkxO9u1bw7xYmWFrACPAx+q3/Iz1H+Ik Wg+3un+58qbdh7twPlNK3QsVZFH8lGhQ== X-Received: by 2002:a05:6830:148a:b0:6eb:7abc:99b3 with SMTP id s10-20020a056830148a00b006eb7abc99b3mr1483214otq.11.1713518339046; 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=-12.4 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.