Skip to content

Commit d5778a7

Browse files
H-G-HristovH-G-Hristov
authored
[libc++] Applied [[nodiscard]] to concurrency (partially) (#169463)
`[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue. - https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant The following utilities have been annotated in this patch: - [x] `<barrier>` - [x] `<condition_variable>` - [x] `<latch>` - [x] `<mutex>` - [x] `<semaphore>` - [x] `<thread>` N.B. Some classes don't provide all specified methods, which were not annotated.
1 parent 3d596ad commit d5778a7

File tree

9 files changed

+173
-53
lines changed

9 files changed

+173
-53
lines changed

‎libcxx/include/__condition_variable/condition_variable.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
170170
wait_for(unique_lock<mutex>& __lk, const chrono::duration<_Rep, _Period>& __d, _Predicate __pred);
171171

172172
typedef __libcpp_condvar_t* native_handle_type;
173-
_LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__cv_; }
173+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__cv_; }
174174

175175
private:
176176
void

‎libcxx/include/__mutex/mutex.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ class _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_CAPABILITY("mutex") mutex {
3737
# endif
3838

3939
_LIBCPP_ACQUIRE_CAPABILITY() void lock();
40-
_LIBCPP_TRY_ACQUIRE_CAPABILITY(true) bool try_lock() _NOEXCEPT;
40+
[[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) bool try_lock() _NOEXCEPT;
4141
_LIBCPP_RELEASE_CAPABILITY void unlock() _NOEXCEPT;
4242

4343
typedef __libcpp_mutex_t* native_handle_type;
44-
_LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
44+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
4545
};
4646

4747
static_assert(is_nothrow_default_constructible<mutex>::value, "the default constructor for std::mutex must be nothrow");

‎libcxx/include/__thread/thread.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,13 +242,13 @@ class _LIBCPP_EXPORTED_FROM_ABI thread {
242242

243243
_LIBCPP_HIDE_FROM_ABI void swap(thread& __t) _NOEXCEPT { std::swap(__t_, __t.__t_); }
244244

245-
_LIBCPP_HIDE_FROM_ABI bool joinable() const _NOEXCEPT { return !__libcpp_thread_isnull(&__t_); }
245+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool joinable() const _NOEXCEPT { return !__libcpp_thread_isnull(&__t_); }
246246
void join();
247247
void detach();
248-
_LIBCPP_HIDE_FROM_ABI id get_id() const _NOEXCEPT { return __libcpp_thread_get_id(&__t_); }
249-
_LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() _NOEXCEPT { return __t_; }
248+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI id get_id() const _NOEXCEPT { return __libcpp_thread_get_id(&__t_); }
249+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() _NOEXCEPT { return __t_; }
250250

251-
static unsigned hardware_concurrency() _NOEXCEPT;
251+
[[__nodiscard__]] static unsigned hardware_concurrency() _NOEXCEPT;
252252
};
253253

254254
inline _LIBCPP_HIDE_FROM_ABI void swap(thread& __x, thread& __y) _NOEXCEPT { __x.swap(__y); }

‎libcxx/include/barrier‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,9 @@ class barrier {
158158
public:
159159
using arrival_token = typename __barrier_base<_CompletionF>::arrival_token;
160160

161-
static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept { return __barrier_base<_CompletionF>::max(); }
161+
[[nodiscard]] static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept {
162+
return __barrier_base<_CompletionF>::max();
163+
}
162164

163165
_LIBCPP_HIDE_FROM_ABI explicit barrier(ptrdiff_t __count, _CompletionF __completion = _CompletionF())
164166
: __b_(__count, std::move(__completion)) {

‎libcxx/include/latch‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ class latch {
7070
atomic<ptrdiff_t> __a_;
7171

7272
public:
73-
static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept { return numeric_limits<ptrdiff_t>::max(); }
73+
[[nodiscard]] static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept {
74+
return numeric_limits<ptrdiff_t>::max();
75+
}
7476

7577
inline _LIBCPP_HIDE_FROM_ABI constexpr explicit latch(ptrdiff_t __expected) : __a_(__expected) {
7678
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
@@ -97,7 +99,7 @@ public:
9799
if (__old == __update)
98100
__a_.notify_all();
99101
}
100-
inline _LIBCPP_HIDE_FROM_ABI bool try_wait() const noexcept {
102+
[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI bool try_wait() const noexcept {
101103
auto __value = __a_.load(memory_order_acquire);
102104
return try_wait_impl(__value);
103105
}

‎libcxx/include/mutex‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,12 @@ public:
229229
recursive_mutex& operator=(const recursive_mutex&) = delete;
230230

231231
void lock();
232-
bool try_lock() _NOEXCEPT;
232+
[[__nodiscard__]] bool try_lock() _NOEXCEPT;
233233
void unlock() _NOEXCEPT;
234234

235235
typedef __libcpp_recursive_mutex_t* native_handle_type;
236236

237-
_LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
237+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
238238
};
239239

240240
class _LIBCPP_EXPORTED_FROM_ABI timed_mutex {
@@ -251,14 +251,14 @@ public:
251251

252252
public:
253253
void lock();
254-
bool try_lock() _NOEXCEPT;
254+
[[__nodiscard__]] bool try_lock() _NOEXCEPT;
255255
template <class _Rep, class _Period>
256-
_LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __d) {
256+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __d) {
257257
return try_lock_until(chrono::steady_clock::now() + __d);
258258
}
259259

260260
template <class _Clock, class _Duration>
261-
_LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t) {
261+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t) {
262262
using namespace chrono;
263263
unique_lock<mutex> __lk(__m_);
264264
bool __no_timeout = _Clock::now() < __t;
@@ -288,14 +288,14 @@ public:
288288
recursive_timed_mutex& operator=(const recursive_timed_mutex&) = delete;
289289

290290
void lock();
291-
bool try_lock() _NOEXCEPT;
291+
[[__nodiscard__]] bool try_lock() _NOEXCEPT;
292292
template <class _Rep, class _Period>
293-
_LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __d) {
293+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __d) {
294294
return try_lock_until(chrono::steady_clock::now() + __d);
295295
}
296296

297297
template <class _Clock, class _Duration>
298-
_LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t) {
298+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t) {
299299
using namespace chrono;
300300
__thread_id __id = this_thread::get_id();
301301
unique_lock<mutex> __lk(__m_);
@@ -320,7 +320,7 @@ public:
320320
};
321321

322322
template <class _L0, class _L1>
323-
_LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1) {
323+
[[__nodiscard__]] _LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1) {
324324
unique_lock<_L0> __u0(__l0, try_to_lock_t());
325325
if (__u0.owns_lock()) {
326326
if (__l1.try_lock()) {
@@ -335,7 +335,8 @@ _LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0,
335335
# ifndef _LIBCPP_CXX03_LANG
336336

337337
template <class _L0, class _L1, class _L2, class... _L3>
338-
_LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3) {
338+
[[__nodiscard__]] _LIBCPP_NO_THREAD_SAFETY_ANALYSIS
339+
_LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3) {
339340
int __r = 0;
340341
unique_lock<_L0> __u0(__l0, try_to_lock);
341342
if (__u0.owns_lock()) {

‎libcxx/include/semaphore‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ class counting_semaphore {
133133
public:
134134
static_assert(__least_max_value >= 0, "The least maximum value must be a positive number");
135135

136-
static constexpr ptrdiff_t max() noexcept { return __least_max_value; }
136+
[[nodiscard]] static constexpr ptrdiff_t max() noexcept { return __least_max_value; }
137137

138138
_LIBCPP_HIDE_FROM_ABI constexpr explicit counting_semaphore(ptrdiff_t __count) : __semaphore_(__count) {
139139
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
@@ -156,12 +156,12 @@ public:
156156
}
157157
_LIBCPP_HIDE_FROM_ABI void acquire() { __semaphore_.acquire(); }
158158
template <class _Rep, class _Period>
159-
_LIBCPP_HIDE_FROM_ABI bool try_acquire_for(chrono::duration<_Rep, _Period> const& __rel_time) {
159+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI bool try_acquire_for(chrono::duration<_Rep, _Period> const& __rel_time) {
160160
return __semaphore_.try_acquire_for(chrono::duration_cast<chrono::nanoseconds>(__rel_time));
161161
}
162-
_LIBCPP_HIDE_FROM_ABI bool try_acquire() { return __semaphore_.try_acquire(); }
162+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI bool try_acquire() { return __semaphore_.try_acquire(); }
163163
template <class _Clock, class _Duration>
164-
_LIBCPP_HIDE_FROM_ABI bool try_acquire_until(chrono::time_point<_Clock, _Duration> const& __abs_time) {
164+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI bool try_acquire_until(chrono::time_point<_Clock, _Duration> const& __abs_time) {
165165
auto const __current = _Clock::now();
166166
if (__current >= __abs_time)
167167
return try_acquire();

‎libcxx/test/libcxx/thread/nodiscard.verify.cpp‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// UNSUPPORTED: c++03
10+
// UNSUPPORTED: no-threads
11+
12+
// Check that functions are marked [[nodiscard]]
13+
14+
#include <chrono>
15+
#include <barrier>
16+
#include <latch>
17+
#include <mutex>
18+
#include <semaphore>
19+
#include <thread>
20+
21+
#include "test_macros.h"
22+
23+
const auto timePoint = std::chrono::steady_clock::now();
24+
25+
void test() {
26+
// Threads
27+
{
28+
std::thread th;
29+
30+
th.joinable(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
31+
th.get_id(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
32+
th.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
33+
th.hardware_concurrency(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
34+
}
35+
#if TEST_STD_VER >= 20
36+
{
37+
std::jthread jt;
38+
39+
jt.joinable(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
40+
jt.get_id(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
41+
jt.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
42+
jt.get_stop_source(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
43+
jt.get_stop_token(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
44+
jt.hardware_concurrency(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
45+
}
46+
#endif
47+
48+
// Mutual exclusion
49+
50+
{ // <mutex>
51+
std::mutex m;
52+
53+
m.try_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
54+
m.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
55+
}
56+
{
57+
std::recursive_mutex m;
58+
59+
m.try_lock(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
60+
m.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
61+
}
62+
{
63+
std::timed_mutex m;
64+
65+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
66+
m.try_lock();
67+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
68+
m.try_lock_for(std::chrono::nanoseconds{82});
69+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
70+
m.try_lock_until(timePoint);
71+
}
72+
{
73+
std::recursive_timed_mutex m;
74+
75+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
76+
m.try_lock();
77+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
78+
m.try_lock_for(std::chrono::nanoseconds{82});
79+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
80+
m.try_lock_until(timePoint);
81+
}
82+
{
83+
std::mutex m1;
84+
std::mutex m2;
85+
std::mutex m3;
86+
87+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
88+
std::try_lock(m1, m2);
89+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
90+
std::try_lock(m1, m2, m3);
91+
}
92+
93+
// Condition variables
94+
95+
{ // <condition_variable>
96+
std::condition_variable cv;
97+
98+
cv.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
99+
}
100+
101+
#if TEST_STD_VER >= 20
102+
103+
// Semaphores
104+
105+
{ // <semaphore>
106+
std::counting_semaphore<> cs{0};
107+
108+
cs.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
109+
110+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
111+
cs.try_acquire_for(std::chrono::nanoseconds{82});
112+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
113+
cs.try_acquire();
114+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
115+
cs.try_acquire_until(timePoint);
116+
117+
std::binary_semaphore bs{0};
118+
119+
bs.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
120+
121+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
122+
bs.try_acquire_for(std::chrono::nanoseconds{82});
123+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
124+
bs.try_acquire();
125+
// expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
126+
bs.try_acquire_until(timePoint);
127+
}
128+
129+
// Latches and barriers
130+
131+
{ // <barrier>
132+
std::barrier<> b{94};
133+
134+
b.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
135+
}
136+
{ // <latch>
137+
std::latch l{94};
138+
139+
l.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
140+
l.try_wait(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
141+
}
142+
143+
#endif
144+
}

‎libcxx/test/std/thread/thread.jthread/nodiscard.verify.cpp‎

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

0 commit comments

Comments
 (0)