Skip to content

Commit 7eac39f

Browse files
authored
[libc++] Mark scoped_lock and unique_lock constructors as [[nodiscard]] (llvm#89397)
It's basically always a bug to discard a scoped_lock or a unique_lock. Fixes llvm#89388
1 parent 413f6b9 commit 7eac39f

File tree

4 files changed

+71
-49
lines changed

4 files changed

+71
-49
lines changed

libcxx/include/__mutex/unique_lock.h

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,28 @@ class _LIBCPP_TEMPLATE_VIS unique_lock {
3636
bool __owns_;
3737

3838
public:
39-
_LIBCPP_HIDE_FROM_ABI unique_lock() _NOEXCEPT : __m_(nullptr), __owns_(false) {}
40-
_LIBCPP_HIDE_FROM_ABI explicit unique_lock(mutex_type& __m) : __m_(std::addressof(__m)), __owns_(true) {
39+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock() _NOEXCEPT : __m_(nullptr), __owns_(false) {}
40+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI explicit unique_lock(mutex_type& __m)
41+
: __m_(std::addressof(__m)), __owns_(true) {
4142
__m_->lock();
4243
}
4344

44-
_LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
45+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
4546
: __m_(std::addressof(__m)),
4647
__owns_(false) {}
4748

48-
_LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, try_to_lock_t)
49+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, try_to_lock_t)
4950
: __m_(std::addressof(__m)), __owns_(__m.try_lock()) {}
5051

51-
_LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, adopt_lock_t) : __m_(std::addressof(__m)), __owns_(true) {}
52+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, adopt_lock_t)
53+
: __m_(std::addressof(__m)), __owns_(true) {}
5254

5355
template <class _Clock, class _Duration>
54-
_LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __t)
56+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __t)
5557
: __m_(std::addressof(__m)), __owns_(__m.try_lock_until(__t)) {}
5658

5759
template <class _Rep, class _Period>
58-
_LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __d)
60+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __d)
5961
: __m_(std::addressof(__m)), __owns_(__m.try_lock_for(__d)) {}
6062

6163
_LIBCPP_HIDE_FROM_ABI ~unique_lock() {
@@ -66,7 +68,9 @@ class _LIBCPP_TEMPLATE_VIS unique_lock {
6668
unique_lock(unique_lock const&) = delete;
6769
unique_lock& operator=(unique_lock const&) = delete;
6870

69-
_LIBCPP_HIDE_FROM_ABI unique_lock(unique_lock&& __u) _NOEXCEPT : __m_(__u.__m_), __owns_(__u.__owns_) {
71+
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(unique_lock&& __u) _NOEXCEPT
72+
: __m_(__u.__m_),
73+
__owns_(__u.__owns_) {
7074
__u.__m_ = nullptr;
7175
__u.__owns_ = false;
7276
}

libcxx/include/mutex

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -427,10 +427,10 @@ class _LIBCPP_TEMPLATE_VIS scoped_lock;
427427
template <>
428428
class _LIBCPP_TEMPLATE_VIS scoped_lock<> {
429429
public:
430-
explicit scoped_lock() {}
430+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock() {}
431431
~scoped_lock() = default;
432432

433-
_LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t) {}
433+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t) {}
434434

435435
scoped_lock(scoped_lock const&) = delete;
436436
scoped_lock& operator=(scoped_lock const&) = delete;
@@ -445,13 +445,15 @@ private:
445445
mutex_type& __m_;
446446

447447
public:
448-
explicit scoped_lock(mutex_type& __m) _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__m)) : __m_(__m) {
448+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(mutex_type& __m)
449+
_LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__m))
450+
: __m_(__m) {
449451
__m_.lock();
450452
}
451453

452454
~scoped_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability()) { __m_.unlock(); }
453455

454-
_LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t, mutex_type& __m)
456+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t, mutex_type& __m)
455457
_LIBCPP_THREAD_SAFETY_ANNOTATION(requires_capability(__m))
456458
: __m_(__m) {}
457459

@@ -465,9 +467,11 @@ class _LIBCPP_TEMPLATE_VIS scoped_lock {
465467
typedef tuple<_MArgs&...> _MutexTuple;
466468

467469
public:
468-
_LIBCPP_HIDE_FROM_ABI explicit scoped_lock(_MArgs&... __margs) : __t_(__margs...) { std::lock(__margs...); }
470+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(_MArgs&... __margs) : __t_(__margs...) {
471+
std::lock(__margs...);
472+
}
469473

470-
_LIBCPP_HIDE_FROM_ABI scoped_lock(adopt_lock_t, _MArgs&... __margs) : __t_(__margs...) {}
474+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI scoped_lock(adopt_lock_t, _MArgs&... __margs) : __t_(__margs...) {}
471475

472476
_LIBCPP_HIDE_FROM_ABI ~scoped_lock() {
473477
typedef typename __make_tuple_indices<sizeof...(_MArgs)>::type _Indices;

libcxx/test/libcxx/diagnostics/mutex.nodiscard.verify.cpp

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,58 @@
1212

1313
// check that <mutex> functions are marked [[nodiscard]]
1414

15-
// clang-format off
16-
1715
#include <mutex>
16+
#include <chrono>
17+
#include <utility>
1818

1919
#include "test_macros.h"
2020

2121
void test() {
22-
std::mutex mutex;
23-
std::lock_guard<std::mutex>{mutex}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
24-
std::lock_guard<std::mutex>{mutex, std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
22+
// std::scoped_lock
23+
{
24+
#if TEST_STD_VER >= 17
25+
using M = std::mutex;
26+
M m0, m1, m2;
27+
// clang-format off
28+
std::scoped_lock<>{}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
29+
std::scoped_lock<M>{m0}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
30+
std::scoped_lock<M, M>{m0, m1}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
31+
std::scoped_lock<M, M, M>{m0, m1, m2}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
32+
33+
std::scoped_lock<>{std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
34+
std::scoped_lock<M>{std::adopt_lock, m0}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
35+
std::scoped_lock<M, M>{std::adopt_lock, m0, m1}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
36+
std::scoped_lock<M, M, M>{std::adopt_lock, m0, m1, m2}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
37+
// clang-format on
38+
#endif
39+
}
40+
41+
// std::unique_lock
42+
{
43+
using M = std::timed_mutex; // necessary for the time_point and duration constructors
44+
M m;
45+
std::chrono::time_point<std::chrono::steady_clock> time_point;
46+
std::chrono::milliseconds duration;
47+
std::unique_lock<M> other;
48+
49+
// clang-format off
50+
std::unique_lock<M>{}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
51+
std::unique_lock<M>{m}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
52+
std::unique_lock<M>{m, std::defer_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
53+
std::unique_lock<M>{m, std::try_to_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
54+
std::unique_lock<M>{m, std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
55+
std::unique_lock<M>{m, time_point}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
56+
std::unique_lock<M>{m, duration}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
57+
std::unique_lock<M>(std::move(other)); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
58+
// clang-format on
59+
}
60+
61+
// std::lock_guard
62+
{
63+
std::mutex m;
64+
// clang-format off
65+
std::lock_guard<std::mutex>{m}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
66+
std::lock_guard<std::mutex>{m, std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
67+
// clang-format on
68+
}
2569
}

libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.verify.cpp

Lines changed: 0 additions & 30 deletions
This file was deleted.

0 commit comments

Comments
 (0)