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 5B4193858D1E for ; Fri, 11 Feb 2022 17:09:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5B4193858D1E Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-616-tBjY30tDNdul2IQS4eO0tA-1; Fri, 11 Feb 2022 12:09:41 -0500 X-MC-Unique: tBjY30tDNdul2IQS4eO0tA-1 Received: by mail-qt1-f199.google.com with SMTP id z1-20020ac87ca1000000b002d11bc8d795so7169848qtv.17 for ; Fri, 11 Feb 2022 09:09:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=Mvd1a2+HJ6KnUWsjUcv/2InyhwRgpXZZlfADS9567BU=; b=vhrimXtWhPqP4vMF63FlB86AAcs9bRRKCi308jJPkGAnf0WcouA5n7/iQJSqcqbNXS dzoSFDF6cGC9XtRHqHFxTvgoQI4hcjv8mOSmaG+fUDwk3Pk+wPwZeiJ5e73p8P0QEEEl FUml36ydQdVjNeHGxaQi8MTGEjpyFm5lUUpXT2DPpsi33RJ+ZLZGKGjTx0lnc07Eh1wA iknrjU9qX9CDmAEKNB41x0nsX5S7rBxIkOmvraLIxAOhpTBgwtxAa0sh5T+oWbrVGOdH ZzuTruqNg/ubfqFHvhhGLRdqgTQTMOqfVMT4hB+Z0PoMk7oJRWa5/7p5axypeyjofUDT URRA== X-Gm-Message-State: AOAM530xXMZtlkH7gXIInZFG69MmNEQtrYVIW8mzN3nSyRAmvg9I3XA3 ng/KrvYDm0BQAR1ENViqayzbC+ixzbD8UfO5F7GhXEL/SD8m/Hb7mm5FBkUOa19phZ9oY2eIBAR JZeMLMJNRz3usaj4= X-Received: by 2002:ad4:5aa6:: with SMTP id u6mr1647337qvg.124.1644599381221; Fri, 11 Feb 2022 09:09:41 -0800 (PST) X-Google-Smtp-Source: ABdhPJxrsnasxGJLvG2ysyIGB3gRRVHuMX54IWqlpFZrsec1GyqAVyJ+R16Cm3faZ5GZinb/aU1o8Q== X-Received: by 2002:ad4:5aa6:: with SMTP id u6mr1647315qvg.124.1644599380917; Fri, 11 Feb 2022 09:09:40 -0800 (PST) Received: from [192.168.1.130] (ool-18e40894.dyn.optonline.net. [24.228.8.148]) by smtp.gmail.com with ESMTPSA id 16sm13082435qty.86.2022.02.11.09.09.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Feb 2022 09:09:40 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Fri, 11 Feb 2022 12:09:39 -0500 (EST) To: Patrick Palka cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org Subject: Re: [PATCH] libstdc++: Back out some changes in P2325R3 backport [PR103904] In-Reply-To: <20220211165157.3159659-1-ppalka@redhat.com> Message-ID: References: <20220211165157.3159659-1-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-14.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Feb 2022 17:09:45 -0000 On Fri, 11 Feb 2022, Patrick Palka wrote: > In the P2325R3 backport r11-9555 the relaxation of the constraints on > the partial specialization of __box (which is semantically equivalent to > the primary template, only more space efficient) means some > specializations of __box will now use the partial specialization instead > of the primary template, which (IIUC) constitutes an ABI change unsuitable > for a release branch. This patch reverts this constraint change, which > isn't needed for correctness anyway. > > Similarly the change to use __non_propagating_cache for the data member > split_view::_M_current (so that it's always default-initializable) also > constitutes an unsuitable ABI change. This patch reverts this change > too, and instead further constrains split_view's default constructor to > require that we can default-initialize _M_current. Forgot to clarify that this is for the 11 branch, tested on x86_64-pc-linux-gnu. Does this look reasonable? I noticed these issues while backporting r11-9555 to the 10 branch, which doesn't have __non_propagating_cache or the partial specialization of __box. > > PR libstdc++/103904 > > libstdc++-v3/ChangeLog: > > * include/std/ranges (__detail::__box): Revert r11-9555 changes > to the constraints on the partial specialization and the > now-unnecessary member additions. > (__detail::__non_propagating_cache::operator=): Remove > now-unused overload added by r11-9555. > (split_view::_OuterIter::__current): Adjust after reverting the > r11-9555 change to the type of _M_current. > (split_view::_M_current): Revert r11-9555 change to its type. > (split_view::split_view): Constrain the default ctor further. > * testsuite/std/ranges/adaptors/detail/copyable_box.cc: Disable > now-irrelevant test for the r11-9555 changes to the partial > specialization of __box. > --- > libstdc++-v3/include/std/ranges | 54 +++---------------- > .../ranges/adaptors/detail/copyable_box.cc | 4 ++ > 2 files changed, 10 insertions(+), 48 deletions(-) > > diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges > index 03c6275801f..bf31e4be500 100644 > --- a/libstdc++-v3/include/std/ranges > +++ b/libstdc++-v3/include/std/ranges > @@ -144,8 +144,7 @@ namespace ranges > // std::optional. It provides just the subset of the primary template's > // API that we currently use. > template<__boxable _Tp> > - requires copyable<_Tp> || (is_nothrow_move_constructible_v<_Tp> > - && is_nothrow_copy_constructible_v<_Tp>) > + requires copyable<_Tp> > struct __box<_Tp> > { > private: > @@ -174,38 +173,6 @@ namespace ranges > : _M_value(std::forward<_Args>(__args)...) > { } > > - __box(const __box&) = default; > - __box(__box&&) = default; > - __box& operator=(const __box&) requires copyable<_Tp> = default; > - __box& operator=(__box&&) requires copyable<_Tp> = default; > - > - // When _Tp is nothrow_copy_constructible but not copy_assignable, > - // copy assignment is implemented via destroy-then-copy-construct. > - constexpr __box& > - operator=(const __box& __that) noexcept > - { > - static_assert(is_nothrow_copy_constructible_v<_Tp>); > - if (this != std::__addressof(__that)) > - { > - _M_value.~_Tp(); > - std::construct_at(std::__addressof(_M_value), *__that); > - } > - return *this; > - } > - > - // Likewise for move assignment. > - constexpr __box& > - operator=(__box&& __that) noexcept > - { > - static_assert(is_nothrow_move_constructible_v<_Tp>); > - if (this != std::__addressof(__that)) > - { > - _M_value.~_Tp(); > - std::construct_at(std::__addressof(_M_value), std::move(*__that)); > - } > - return *this; > - } > - > constexpr bool > has_value() const noexcept > { return true; }; > @@ -1180,16 +1147,6 @@ namespace views::__adaptor > return *this; > } > > - constexpr __non_propagating_cache& > - operator=(_Tp __val) > - { > - this->_M_reset(); > - std::construct_at(std::__addressof(this->_M_payload._M_payload), > - std::in_place, std::move(__val)); > - this->_M_payload._M_engaged = true; > - return *this; > - } > - > constexpr _Tp& > operator*() noexcept > { return this->_M_get(); } > @@ -2886,7 +2843,7 @@ namespace views::__adaptor > if constexpr (forward_range<_Vp>) > return _M_current; > else > - return *_M_parent->_M_current; > + return _M_parent->_M_current; > } > > constexpr auto& > @@ -2895,7 +2852,7 @@ namespace views::__adaptor > if constexpr (forward_range<_Vp>) > return _M_current; > else > - return *_M_parent->_M_current; > + return _M_parent->_M_current; > } > > _Parent* _M_parent = nullptr; > @@ -3143,13 +3100,14 @@ namespace views::__adaptor > // XXX: _M_current is "present only if !forward_range" > [[no_unique_address]] > __detail::__maybe_present_t, > - __detail::__non_propagating_cache>> _M_current; > + iterator_t<_Vp>> _M_current; > _Vp _M_base = _Vp(); > > > public: > split_view() requires (default_initializable<_Vp> > - && default_initializable<_Pattern>) > + && default_initializable<_Pattern> > + && default_initializable>) > = default; > > constexpr > diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/detail/copyable_box.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/detail/copyable_box.cc > index fa6d4d56816..2078d442447 100644 > --- a/libstdc++-v3/testsuite/std/ranges/adaptors/detail/copyable_box.cc > +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/detail/copyable_box.cc > @@ -101,6 +101,9 @@ test02() > __box> x1(std::in_place, 0, 0); > } > > +#if 0 > +// On the 11 branch, the partial specialization of __box admits only copyable types > +// so this test doesn't apply. > constexpr bool > test03() > { > @@ -142,3 +145,4 @@ test03() > return true; > } > static_assert(test03()); > +#endif > -- > 2.35.1.102.g2b9c120970 > >