Does this make any difference to optimized code, or only for -O0?
On Thu, 12 Nov 2026 at 20:23, Jonathan Wakely <[email protected]> wrote: > > On Thu, 12 Nov 2026 at 17:41, Malcolm Parsons <[email protected]> wrote: > > > > The constructor for an immediately-owned unique_lock initializes the > > ownership flag to false and then calls the checked lock() modifier, which > > sets the flag to true. The constructor then sets it to true again. The > > destructor similarly calls the checked unlock() modifier after already > > testing ownership. > > > > Match the implementation pattern used by shared_lock: construct the object > > directly in its final ownership state, call the mutex operations directly > > during construction and destruction, and remove unlock()'s redundant mutex > > pointer check after ownership has been established. This avoids duplicated > > checks and exception paths without changing observable behavior. > > > > No new test is needed because the change has no observable behavior. The > > existing tests cover construction, destruction, explicit unlocking, and > > condition_variable_any waits. > > > > Tested on x86_64-pc-linux-gnu with no regressions. > > > > libstdc++-v3/ChangeLog: > > > > * include/bits/unique_lock.h (unique_lock::unique_lock): Construct > > directly in the owned state and call the mutex directly. > > (unique_lock::~unique_lock): Unlock the mutex directly. > > (unique_lock::unlock): Remove redundant mutex pointer check. > > > > Assisted-by: OpenAI Codex > > --- > > libstdc++-v3/include/bits/unique_lock.h | 16 +++++----------- > > 1 file changed, 5 insertions(+), 11 deletions(-) > > > > diff --git a/libstdc++-v3/include/bits/unique_lock.h > > b/libstdc++-v3/include/bits/unique_lock.h > > index 51596ef31..162be4b62 100644 > > --- a/libstdc++-v3/include/bits/unique_lock.h > > +++ b/libstdc++-v3/include/bits/unique_lock.h > > @@ -70,11 +70,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > [[__nodiscard__]] > > explicit unique_lock(mutex_type& __m) > > - : _M_device(std::__addressof(__m)), _M_owns(false) > > - { > > - lock(); > > - _M_owns = true; > > - } > > + : _M_device(std::__addressof(__m)), _M_owns(true) > > + { __m.lock(); } > > > > unique_lock(mutex_type& __m, defer_lock_t) noexcept > > : _M_device(std::__addressof(__m)), _M_owns(false) > > @@ -111,7 +108,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > ~unique_lock() > > { > > if (_M_owns) > > - unlock(); > > + _M_device->unlock(); > > This removes the check for non-null _M_device, but it should be > impossible for _M_owns to be true and _M_device to be null. > > > > } > > > > unique_lock(const unique_lock&) = delete; > > @@ -198,11 +195,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > { > > if (!_M_owns) > > __throw_system_error(int(errc::operation_not_permitted)); > > - else if (_M_device) > > - { > > - _M_device->unlock(); > > - _M_owns = false; > > - } > > And this does the same. Again, this should be fine in a correct > program, but it could change the behaviour for a program which has > somehow broken the unique_lock invariants. The technically redundant > check for _M_device avoids a segfault, or optimizations that assume > _M_device must be non-null because it's dereferenced. > > How much would it hurt the codegen to do `if (!_M_owns || !_M_device)` > in the condition for the throwing path? > > And similarly, check `if (_M_owns && _M_device)` in the destructor. > > Or just add __glibcxx_assert(_M_device) immediately before the > _M_device->unlock() calls. > > > + _M_device->unlock(); > > + _M_owns = false; > > } > > > > void > > -- > > 2.43.0 > >
