[libcxx-commits] [libcxx] [libc++] Apply `[[nodiscard]]` to `unique_lock` and `<shared_mutex>` (PR #200986)
via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jun 1 22:23:12 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: A. Jiang (frederick-vs-ja)
<details>
<summary>Changes</summary>
`[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue.
- https://libcxx.llvm.org/CodingGuidelines.html
- https://wg21.link/thread.sharedmutex.class
- https://wg21.link/thread.sharedtimedmutex.class
- https://wg21.link/thread.lock.unique
- https://wg21.link/thread.lock.shared
Remarks:
- All constructors of `shared_lock` are marked `[[nodiscard]]`, which is consistent with handling for constructors of `unique_lock`.
---
Full diff: https://github.com/llvm/llvm-project/pull/200986.diff
3 Files Affected:
- (modified) libcxx/include/__mutex/unique_lock.h (+5-5)
- (modified) libcxx/include/shared_mutex (+29-21)
- (modified) libcxx/test/libcxx/thread/nodiscard.verify.cpp (+75-2)
``````````diff
diff --git a/libcxx/include/__mutex/unique_lock.h b/libcxx/include/__mutex/unique_lock.h
index 6968922639673..055ece847b3ce 100644
--- a/libcxx/include/__mutex/unique_lock.h
+++ b/libcxx/include/__mutex/unique_lock.h
@@ -84,13 +84,13 @@ class unique_lock {
}
_LIBCPP_HIDE_FROM_ABI void lock();
- _LIBCPP_HIDE_FROM_ABI bool try_lock();
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock();
template <class _Rep, class _Period>
- _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __d);
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __d);
template <class _Clock, class _Duration>
- _LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t);
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t);
_LIBCPP_HIDE_FROM_ABI void unlock();
@@ -106,9 +106,9 @@ class unique_lock {
return __m;
}
- _LIBCPP_HIDE_FROM_ABI bool owns_lock() const _NOEXCEPT { return __owns_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool owns_lock() const _NOEXCEPT { return __owns_; }
_LIBCPP_HIDE_FROM_ABI explicit operator bool() const _NOEXCEPT { return __owns_; }
- _LIBCPP_HIDE_FROM_ABI mutex_type* mutex() const _NOEXCEPT { return __m_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI mutex_type* mutex() const _NOEXCEPT { return __m_; }
};
_LIBCPP_CTAD_SUPPORTED_FOR_TYPE(unique_lock);
diff --git a/libcxx/include/shared_mutex b/libcxx/include/shared_mutex
index c1ba1c3b7a77b..2d0ed11309bd6 100644
--- a/libcxx/include/shared_mutex
+++ b/libcxx/include/shared_mutex
@@ -197,12 +197,14 @@ public:
// Exclusive ownership
_LIBCPP_ACQUIRE_CAPABILITY() _LIBCPP_HIDE_FROM_ABI void lock() { return __base_.lock(); }
- _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool try_lock() { return __base_.try_lock(); }
+ [[nodiscard]] _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool try_lock() {
+ return __base_.try_lock();
+ }
_LIBCPP_RELEASE_CAPABILITY _LIBCPP_HIDE_FROM_ABI void unlock() { return __base_.unlock(); }
// Shared ownership
_LIBCPP_ACQUIRE_SHARED_CAPABILITY _LIBCPP_HIDE_FROM_ABI void lock_shared() { return __base_.lock_shared(); }
- _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool try_lock_shared() {
+ [[nodiscard]] _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool try_lock_shared() {
return __base_.try_lock_shared();
}
_LIBCPP_RELEASE_SHARED_CAPABILITY _LIBCPP_HIDE_FROM_ABI void unlock_shared() { return __base_.unlock_shared(); }
@@ -224,15 +226,15 @@ public:
// Exclusive ownership
void lock() _LIBCPP_ACQUIRE_CAPABILITY();
- _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) bool try_lock();
+ [[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) bool try_lock();
template <class _Rep, class _Period>
- _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
+ [[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
try_lock_for(const chrono::duration<_Rep, _Period>& __rel_time) {
return try_lock_until(chrono::steady_clock::now() + __rel_time);
}
template <class _Clock, class _Duration>
- _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
+ [[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
try_lock_until(const chrono::time_point<_Clock, _Duration>& __abs_time) {
unique_lock<mutex> __lk(__base_.__mut_);
if (__base_.__state_ & __base_.__write_entered_) {
@@ -264,15 +266,15 @@ public:
// Shared ownership
_LIBCPP_ACQUIRE_SHARED_CAPABILITY void lock_shared();
- _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) bool try_lock_shared();
+ [[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) bool try_lock_shared();
template <class _Rep, class _Period>
- _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
+ [[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
try_lock_shared_for(const chrono::duration<_Rep, _Period>& __rel_time) {
return try_lock_shared_until(chrono::steady_clock::now() + __rel_time);
}
template <class _Clock, class _Duration>
- _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
+ [[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool
try_lock_shared_until(const chrono::time_point<_Clock, _Duration>& __abs_time) {
unique_lock<mutex> __lk(__base_.__mut_);
if ((__base_.__state_ & __base_.__write_entered_) ||
@@ -305,27 +307,31 @@ private:
bool __owns_;
public:
- _LIBCPP_HIDE_FROM_ABI shared_lock() _NOEXCEPT : __m_(nullptr), __owns_(false) {}
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_lock() _NOEXCEPT : __m_(nullptr), __owns_(false) {}
- _LIBCPP_HIDE_FROM_ABI explicit shared_lock(mutex_type& __m) : __m_(std::addressof(__m)), __owns_(true) {
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI explicit shared_lock(mutex_type& __m)
+ : __m_(std::addressof(__m)), __owns_(true) {
__m_->lock_shared();
}
- _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
: __m_(std::addressof(__m)),
__owns_(false) {}
- _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, try_to_lock_t)
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, try_to_lock_t)
: __m_(std::addressof(__m)), __owns_(__m.try_lock_shared()) {}
- _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, adopt_lock_t) : __m_(std::addressof(__m)), __owns_(true) {}
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, adopt_lock_t)
+ : __m_(std::addressof(__m)), __owns_(true) {}
template <class _Clock, class _Duration>
- _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __abs_time)
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI
+ shared_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __abs_time)
: __m_(std::addressof(__m)), __owns_(__m.try_lock_shared_until(__abs_time)) {}
template <class _Rep, class _Period>
- _LIBCPP_HIDE_FROM_ABI shared_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __rel_time)
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI
+ shared_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __rel_time)
: __m_(std::addressof(__m)), __owns_(__m.try_lock_shared_for(__rel_time)) {}
_LIBCPP_HIDE_FROM_ABI ~shared_lock() {
@@ -336,7 +342,9 @@ public:
shared_lock(shared_lock const&) = delete;
shared_lock& operator=(shared_lock const&) = delete;
- _LIBCPP_HIDE_FROM_ABI shared_lock(shared_lock&& __u) _NOEXCEPT : __m_(__u.__m_), __owns_(__u.__owns_) {
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_lock(shared_lock&& __u) _NOEXCEPT
+ : __m_(__u.__m_),
+ __owns_(__u.__owns_) {
__u.__m_ = nullptr;
__u.__owns_ = false;
}
@@ -348,11 +356,11 @@ public:
}
_LIBCPP_HIDE_FROM_ABI void lock();
- _LIBCPP_HIDE_FROM_ABI bool try_lock();
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock();
template <class _Rep, class _Period>
- _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __rel_time);
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __rel_time);
template <class _Clock, class _Duration>
- _LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __abs_time);
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __abs_time);
_LIBCPP_HIDE_FROM_ABI void unlock();
// Setters
@@ -369,11 +377,11 @@ public:
}
// Getters
- _LIBCPP_HIDE_FROM_ABI bool owns_lock() const _NOEXCEPT { return __owns_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool owns_lock() const _NOEXCEPT { return __owns_; }
_LIBCPP_HIDE_FROM_ABI explicit operator bool() const _NOEXCEPT { return __owns_; }
- _LIBCPP_HIDE_FROM_ABI mutex_type* mutex() const _NOEXCEPT { return __m_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI mutex_type* mutex() const _NOEXCEPT { return __m_; }
};
_LIBCPP_CTAD_SUPPORTED_FOR_TYPE(shared_lock);
diff --git a/libcxx/test/libcxx/thread/nodiscard.verify.cpp b/libcxx/test/libcxx/thread/nodiscard.verify.cpp
index 23a7554d3095a..8af921494e26b 100644
--- a/libcxx/test/libcxx/thread/nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/thread/nodiscard.verify.cpp
@@ -16,6 +16,7 @@
#include <latch>
#include <mutex>
#include <semaphore>
+#include <shared_mutex>
#include <thread>
#include "test_macros.h"
@@ -23,6 +24,9 @@
const auto timePoint = std::chrono::steady_clock::now();
void test() {
+ std::chrono::time_point<std::chrono::steady_clock> time_point;
+ std::chrono::milliseconds duration;
+
// [futures]
{
{
@@ -155,8 +159,6 @@ void test() {
{
using M = std::timed_mutex; // necessary for the time_point and duration constructors
M m;
- std::chrono::time_point<std::chrono::steady_clock> time_point;
- std::chrono::milliseconds duration;
std::unique_lock<M> other;
// clang-format off
@@ -169,6 +171,16 @@ void test() {
std::unique_lock<M>(m, duration); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
std::unique_lock<M>(std::move(other)); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
// clang-format on
+
+ std::unique_lock<M> lock;
+
+ lock.try_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ lock.try_lock_for(duration);
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ lock.try_lock_until(time_point);
+ lock.owns_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ lock.mutex(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
}
// std::lock_guard
@@ -180,6 +192,67 @@ void test() {
// clang-format on
}
+#if TEST_STD_VER >= 17
+ // std::shared_mutex
+ {
+ std::shared_mutex m;
+
+ m.try_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ m.try_lock_shared(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+#endif
+
+#if TEST_STD_VER >= 14
+ // std::shared_timed_mutex
+ {
+ std::shared_timed_mutex m;
+
+ m.try_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ m.try_lock_for(duration);
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ m.try_lock_until(time_point);
+ m.try_lock_shared(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ m.try_lock_shared_for(duration);
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ m.try_lock_shared_until(time_point);
+ }
+ // std::shared_lock
+ {
+ using M = std::shared_timed_mutex;
+ M m;
+ std::shared_lock<M> other;
+
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{};
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{m};
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{m, std::defer_lock};
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{m, std::try_to_lock};
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{m, std::adopt_lock};
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{m, time_point};
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{m, duration};
+ // expected-warning at +1 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::shared_lock<M>{std::move(other)};
+
+ std::shared_lock<M> lock;
+
+ lock.try_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ lock.try_lock_for(duration);
+ // expected-warning at +1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ lock.try_lock_until(time_point);
+ lock.owns_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ lock.mutex(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+#endif
+
// Threads
{
std::thread th;
``````````
</details>
https://github.com/llvm/llvm-project/pull/200986
More information about the libcxx-commits
mailing list