| Message ID | 20201021201400.GA1048300@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [committed] libstdc++: Simplify std::shared_ptr construction from std::weak_ptr | expand |
On 21/10/2020 22:14, Jonathan Wakely via Gcc-patches wrote: > The _M_add_ref_lock() and _M_add_ref_lock_nothrow() members of > _Sp_counted_base are very similar, except that the former throws an > exception when the use count is zero and the latter returns false. The > former (and its callers) can be implemented in terms of the latter. > This results in a small reduction in code size, because throwing an > exception now only happens in one place. > > libstdc++-v3/ChangeLog: > > * include/bits/shared_ptr.h (shared_ptr(const weak_ptr&, nothrow_t)): > Add noexcept. > * include/bits/shared_ptr_base.h (_Sp_counted_base::_M_add_ref_lock): > Remove specializations and just call _M_add_ref_lock_nothrow. > (__shared_count, __shared_ptr): Use nullptr for null pointer > constants. > (__shared_count(const __weak_count&)): Use _M_add_ref_lock_nothrow > instead of _M_add_ref_lock. > (__shared_count(const __weak_count&, nothrow_t)): Add noexcept. > (__shared_ptr::operator bool()): Add noexcept. > (__shared_ptr(const __weak_ptr&, nothrow_t)): Add noexcept. > > Tested powerpc64le-linux. Committed to trunk. Clang now complains about > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:230:5: error: '_M_add_ref_lock_nothrow' is missing exception specification 'noexcept' > _M_add_ref_lock_nothrow() > ^ > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:158:7: note: previous declaration is here > _M_add_ref_lock_nothrow() noexcept; > ^ > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:241:5: error: '_M_add_ref_lock_nothrow' is missing exception specification 'noexcept' > _M_add_ref_lock_nothrow() > ^ > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:158:7: note: previous declaration is here > _M_add_ref_lock_nothrow() noexcept; > ^ > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:255:5: error: '_M_add_ref_lock_nothrow' is missing exception specification 'noexcept' > _M_add_ref_lock_nothrow() > ^ > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:158:7: note: previous declaration is here > _M_add_ref_lock_nothrow() noexcept; > ^ > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:876:5: error: exception specification in declaration does not match previous declaration > __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t) noexcept > ^ > ~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:696:16: note: previous declaration is here > explicit __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t); > ^ > 4 errors generated. which would be fixed with > diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h > index a9e1c9bb1d5..10c9c831411 100644 > --- a/libstdc++-v3/include/bits/shared_ptr_base.h > +++ b/libstdc++-v3/include/bits/shared_ptr_base.h > @@ -227,7 +227,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > template<> > inline bool > _Sp_counted_base<_S_single>:: > - _M_add_ref_lock_nothrow() > + _M_add_ref_lock_nothrow() noexcept > { > if (_M_use_count == 0) > return false; > @@ -238,7 +238,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > template<> > inline bool > _Sp_counted_base<_S_mutex>:: > - _M_add_ref_lock_nothrow() > + _M_add_ref_lock_nothrow() noexcept > { > __gnu_cxx::__scoped_lock sentry(*this); > if (__gnu_cxx::__exchange_and_add_dispatch(&_M_use_count, 1) == 0) > @@ -252,7 +252,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > template<> > inline bool > _Sp_counted_base<_S_atomic>:: > - _M_add_ref_lock_nothrow() > + _M_add_ref_lock_nothrow() noexcept > { > // Perform lock-free add-if-not-zero operation. > _Atomic_word __count = _M_get_use_count(); > @@ -693,7 +693,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > explicit __shared_count(const __weak_count<_Lp>& __r); > > // Does not throw if __r._M_get_use_count() == 0, caller must check. > - explicit __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t); > + explicit __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t) noexcept; > > ~__shared_count() noexcept > {
On 26/10/20 08:07 +0100, Stephan Bergmann wrote: >On 21/10/2020 22:14, Jonathan Wakely via Gcc-patches wrote: >>The _M_add_ref_lock() and _M_add_ref_lock_nothrow() members of >>_Sp_counted_base are very similar, except that the former throws an >>exception when the use count is zero and the latter returns false. The >>former (and its callers) can be implemented in terms of the latter. >>This results in a small reduction in code size, because throwing an >>exception now only happens in one place. >> >>libstdc++-v3/ChangeLog: >> >> * include/bits/shared_ptr.h (shared_ptr(const weak_ptr&, nothrow_t)): >> Add noexcept. >> * include/bits/shared_ptr_base.h (_Sp_counted_base::_M_add_ref_lock): >> Remove specializations and just call _M_add_ref_lock_nothrow. >> (__shared_count, __shared_ptr): Use nullptr for null pointer >> constants. >> (__shared_count(const __weak_count&)): Use _M_add_ref_lock_nothrow >> instead of _M_add_ref_lock. >> (__shared_count(const __weak_count&, nothrow_t)): Add noexcept. >> (__shared_ptr::operator bool()): Add noexcept. >> (__shared_ptr(const __weak_ptr&, nothrow_t)): Add noexcept. >> >>Tested powerpc64le-linux. Committed to trunk. > >Clang now complains about > >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:230:5: error: '_M_add_ref_lock_nothrow' is missing exception specification 'noexcept' >> _M_add_ref_lock_nothrow() >> ^ >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:158:7: note: previous declaration is here >> _M_add_ref_lock_nothrow() noexcept; >> ^ >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:241:5: error: '_M_add_ref_lock_nothrow' is missing exception specification 'noexcept' >> _M_add_ref_lock_nothrow() >> ^ >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:158:7: note: previous declaration is here >> _M_add_ref_lock_nothrow() noexcept; >> ^ >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:255:5: error: '_M_add_ref_lock_nothrow' is missing exception specification 'noexcept' >> _M_add_ref_lock_nothrow() >> ^ >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:158:7: note: previous declaration is here >> _M_add_ref_lock_nothrow() noexcept; >> ^ >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:876:5: error: exception specification in declaration does not match previous declaration >> __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t) noexcept >> ^ >>~gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/11.0.0/../../../../include/c++/11.0.0/bits/shared_ptr_base.h:696:16: note: previous declaration is here >> explicit __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t); >> ^ >>4 errors generated. > >which would be fixed with Committed, thanks. >>diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h >>index a9e1c9bb1d5..10c9c831411 100644 >>--- a/libstdc++-v3/include/bits/shared_ptr_base.h >>+++ b/libstdc++-v3/include/bits/shared_ptr_base.h >>@@ -227,7 +227,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> template<> >> inline bool >> _Sp_counted_base<_S_single>:: >>- _M_add_ref_lock_nothrow() >>+ _M_add_ref_lock_nothrow() noexcept >> { >> if (_M_use_count == 0) >> return false; >>@@ -238,7 +238,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> template<> >> inline bool >> _Sp_counted_base<_S_mutex>:: >>- _M_add_ref_lock_nothrow() >>+ _M_add_ref_lock_nothrow() noexcept >> { >> __gnu_cxx::__scoped_lock sentry(*this); >> if (__gnu_cxx::__exchange_and_add_dispatch(&_M_use_count, 1) == 0) >>@@ -252,7 +252,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> template<> >> inline bool >> _Sp_counted_base<_S_atomic>:: >>- _M_add_ref_lock_nothrow() >>+ _M_add_ref_lock_nothrow() noexcept >> { >> // Perform lock-free add-if-not-zero operation. >> _Atomic_word __count = _M_get_use_count(); >>@@ -693,7 +693,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> explicit __shared_count(const __weak_count<_Lp>& __r); >> // Does not throw if __r._M_get_use_count() == 0, caller must check. >>- explicit __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t); >>+ explicit __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t) noexcept; >> ~__shared_count() noexcept >> { >
diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h index 0c393e23132..0bfb525aae7 100644 --- a/libstdc++-v3/include/bits/shared_ptr.h +++ b/libstdc++-v3/include/bits/shared_ptr.h @@ -413,7 +413,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION allocate_shared(const _Alloc& __a, _Args&&... __args); // This constructor is non-standard, it is used by weak_ptr::lock(). - shared_ptr(const weak_ptr<_Tp>& __r, std::nothrow_t) + shared_ptr(const weak_ptr<_Tp>& __r, std::nothrow_t) noexcept : __shared_ptr<_Tp>(__r, std::nothrow) { } friend class weak_ptr<_Tp>; diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h index ff578e66117..ca37f2bebd6 100644 --- a/libstdc++-v3/include/bits/shared_ptr_base.h +++ b/libstdc++-v3/include/bits/shared_ptr_base.h @@ -142,10 +142,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { __gnu_cxx::__atomic_add_dispatch(&_M_use_count, 1); } void - _M_add_ref_lock(); + _M_add_ref_lock() + { + if (!_M_add_ref_lock_nothrow()) + __throw_bad_weak_ptr(); + } bool - _M_add_ref_lock_nothrow(); + _M_add_ref_lock_nothrow() noexcept; void _M_release() noexcept @@ -214,48 +218,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Atomic_word _M_weak_count; // #weak + (#shared != 0) }; - template<> - inline void - _Sp_counted_base<_S_single>:: - _M_add_ref_lock() - { - if (_M_use_count == 0) - __throw_bad_weak_ptr(); - ++_M_use_count; - } - - template<> - inline void - _Sp_counted_base<_S_mutex>:: - _M_add_ref_lock() - { - __gnu_cxx::__scoped_lock sentry(*this); - if (__gnu_cxx::__exchange_and_add_dispatch(&_M_use_count, 1) == 0) - { - _M_use_count = 0; - __throw_bad_weak_ptr(); - } - } - - template<> - inline void - _Sp_counted_base<_S_atomic>:: - _M_add_ref_lock() - { - // Perform lock-free add-if-not-zero operation. - _Atomic_word __count = _M_get_use_count(); - do - { - if (__count == 0) - __throw_bad_weak_ptr(); - // Replace the current counter value with the old value + 1, as - // long as it's not changed meanwhile. - } - while (!__atomic_compare_exchange_n(&_M_use_count, &__count, __count + 1, - true, __ATOMIC_ACQ_REL, - __ATOMIC_RELAXED)); - } - template<> inline bool _Sp_counted_base<_S_single>:: @@ -736,7 +698,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __shared_count(const __shared_count& __r) noexcept : _M_pi(__r._M_pi) { - if (_M_pi != 0) + if (_M_pi != nullptr) _M_pi->_M_add_ref_copy(); } @@ -746,9 +708,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Sp_counted_base<_Lp>* __tmp = __r._M_pi; if (__tmp != _M_pi) { - if (__tmp != 0) + if (__tmp != nullptr) __tmp->_M_add_ref_copy(); - if (_M_pi != 0) + if (_M_pi != nullptr) _M_pi->_M_release(); _M_pi = __tmp; } @@ -765,7 +727,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION long _M_get_use_count() const noexcept - { return _M_pi != 0 ? _M_pi->_M_get_use_count() : 0; } + { return _M_pi ? _M_pi->_M_get_use_count() : 0; } bool _M_unique() const noexcept @@ -897,9 +859,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __shared_count<_Lp>::__shared_count(const __weak_count<_Lp>& __r) : _M_pi(__r._M_pi) { - if (_M_pi != nullptr) - _M_pi->_M_add_ref_lock(); - else + if (_M_pi == nullptr || !_M_pi->_M_add_ref_lock_nothrow()) __throw_bad_weak_ptr(); } @@ -907,12 +867,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<_Lock_policy _Lp> inline __shared_count<_Lp>:: - __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t) + __shared_count(const __weak_count<_Lp>& __r, std::nothrow_t) noexcept : _M_pi(__r._M_pi) { - if (_M_pi != nullptr) - if (!_M_pi->_M_add_ref_lock_nothrow()) - _M_pi = nullptr; + if (_M_pi && !_M_pi->_M_add_ref_lock_nothrow()) + _M_pi = nullptr; } #define __cpp_lib_shared_ptr_arrays 201611L @@ -1175,7 +1134,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_ptr(__p), _M_refcount() { _M_refcount._M_swap(__r._M_refcount); - __r._M_ptr = 0; + __r._M_ptr = nullptr; } __shared_ptr(const __shared_ptr&) noexcept = default; @@ -1191,7 +1150,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_ptr(__r._M_ptr), _M_refcount() { _M_refcount._M_swap(__r._M_refcount); - __r._M_ptr = 0; + __r._M_ptr = nullptr; } template<typename _Yp, typename = _Compatible<_Yp>> @@ -1199,7 +1158,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_ptr(__r._M_ptr), _M_refcount() { _M_refcount._M_swap(__r._M_refcount); - __r._M_ptr = 0; + __r._M_ptr = nullptr; } template<typename _Yp, typename = _Compatible<_Yp>> @@ -1305,7 +1264,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION reset(_Yp* __p) // _Yp must be complete. { // Catch self-reset errors. - __glibcxx_assert(__p == 0 || __p != _M_ptr); + __glibcxx_assert(__p == nullptr || __p != _M_ptr); __shared_ptr(__p).swap(*this); } @@ -1325,8 +1284,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return _M_ptr; } /// Return true if the stored pointer is not null. - explicit operator bool() const // never throws - { return _M_ptr == 0 ? false : true; } + explicit operator bool() const noexcept + { return _M_ptr != nullptr; } /// Return true if use_count() == 1. bool @@ -1378,7 +1337,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // This constructor is used by __weak_ptr::lock() and // shared_ptr::shared_ptr(const weak_ptr&, std::nothrow_t). - __shared_ptr(const __weak_ptr<_Tp, _Lp>& __r, std::nothrow_t) + __shared_ptr(const __weak_ptr<_Tp, _Lp>& __r, std::nothrow_t) noexcept : _M_refcount(__r._M_refcount, std::nothrow) { _M_ptr = _M_refcount._M_get_use_count() ? __r._M_ptr : nullptr;
The _M_add_ref_lock() and _M_add_ref_lock_nothrow() members of _Sp_counted_base are very similar, except that the former throws an exception when the use count is zero and the latter returns false. The former (and its callers) can be implemented in terms of the latter. This results in a small reduction in code size, because throwing an exception now only happens in one place. libstdc++-v3/ChangeLog: * include/bits/shared_ptr.h (shared_ptr(const weak_ptr&, nothrow_t)): Add noexcept. * include/bits/shared_ptr_base.h (_Sp_counted_base::_M_add_ref_lock): Remove specializations and just call _M_add_ref_lock_nothrow. (__shared_count, __shared_ptr): Use nullptr for null pointer constants. (__shared_count(const __weak_count&)): Use _M_add_ref_lock_nothrow instead of _M_add_ref_lock. (__shared_count(const __weak_count&, nothrow_t)): Add noexcept. (__shared_ptr::operator bool()): Add noexcept. (__shared_ptr(const __weak_ptr&, nothrow_t)): Add noexcept. Tested powerpc64le-linux. Committed to trunk. commit 945151b7f14c5d105abd8117f208ae9e3db91fb4 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed Oct 21 21:13:41 2020 libstdc++: Simplify std::shared_ptr construction from std::weak_ptr The _M_add_ref_lock() and _M_add_ref_lock_nothrow() members of _Sp_counted_base are very similar, except that the former throws an exception when the use count is zero and the latter returns false. The former (and its callers) can be implemented in terms of the latter. This results in a small reduction in code size, because throwing an exception now only happens in one place. libstdc++-v3/ChangeLog: * include/bits/shared_ptr.h (shared_ptr(const weak_ptr&, nothrow_t)): Add noexcept. * include/bits/shared_ptr_base.h (_Sp_counted_base::_M_add_ref_lock): Remove specializations and just call _M_add_ref_lock_nothrow. (__shared_count, __shared_ptr): Use nullptr for null pointer constants. (__shared_count(const __weak_count&)): Use _M_add_ref_lock_nothrow instead of _M_add_ref_lock. (__shared_count(const __weak_count&, nothrow_t)): Add noexcept. (__shared_ptr::operator bool()): Add noexcept. (__shared_ptr(const __weak_ptr&, nothrow_t)): Add noexcept.