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<V>" > [[no_unique_address]] > __detail::__maybe_present_t<!forward_range<_Vp>, > - __detail::__non_propagating_cache<iterator_t<_Vp>>> _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<iterator_t<_Vp>>) > = 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<A<false>> 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 > >