-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc++] Applied [[nodiscard]] to concurrency (partially)
#169463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc++] Applied [[nodiscard]] to concurrency (partially)
#169463
Conversation
|
@llvm/pr-subscribers-libcxx Author: Hristo Hristov (H-G-Hristov) Changes
The following utilities have been anotated in this patch:
Full diff: https://github.com/llvm/llvm-project/pull/169463.diff 9 Files Affected:
diff --git a/libcxx/include/__condition_variable/condition_variable.h b/libcxx/include/__condition_variable/condition_variable.h
index 1e8edd5dcb009..b7151930e9226 100644
--- a/libcxx/include/__condition_variable/condition_variable.h
+++ b/libcxx/include/__condition_variable/condition_variable.h
@@ -170,7 +170,7 @@ class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
wait_for(unique_lock<mutex>& __lk, const chrono::duration<_Rep, _Period>& __d, _Predicate __pred);
typedef __libcpp_condvar_t* native_handle_type;
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__cv_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__cv_; }
private:
void
diff --git a/libcxx/include/__mutex/mutex.h b/libcxx/include/__mutex/mutex.h
index 68c8842b35eda..1fbe09f266832 100644
--- a/libcxx/include/__mutex/mutex.h
+++ b/libcxx/include/__mutex/mutex.h
@@ -41,7 +41,7 @@ class _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_CAPABILITY("mutex") mutex {
_LIBCPP_RELEASE_CAPABILITY void unlock() _NOEXCEPT;
typedef __libcpp_mutex_t* native_handle_type;
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
};
static_assert(is_nothrow_default_constructible<mutex>::value, "the default constructor for std::mutex must be nothrow");
diff --git a/libcxx/include/__thread/thread.h b/libcxx/include/__thread/thread.h
index a3b672bc0f0e7..561f092ddb7c0 100644
--- a/libcxx/include/__thread/thread.h
+++ b/libcxx/include/__thread/thread.h
@@ -242,13 +242,13 @@ class _LIBCPP_EXPORTED_FROM_ABI thread {
_LIBCPP_HIDE_FROM_ABI void swap(thread& __t) _NOEXCEPT { std::swap(__t_, __t.__t_); }
- _LIBCPP_HIDE_FROM_ABI bool joinable() const _NOEXCEPT { return !__libcpp_thread_isnull(&__t_); }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool joinable() const _NOEXCEPT { return !__libcpp_thread_isnull(&__t_); }
void join();
void detach();
- _LIBCPP_HIDE_FROM_ABI id get_id() const _NOEXCEPT { return __libcpp_thread_get_id(&__t_); }
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() _NOEXCEPT { return __t_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI id get_id() const _NOEXCEPT { return __libcpp_thread_get_id(&__t_); }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() _NOEXCEPT { return __t_; }
- static unsigned hardware_concurrency() _NOEXCEPT;
+ [[__nodiscard__]] static unsigned hardware_concurrency() _NOEXCEPT;
};
inline _LIBCPP_HIDE_FROM_ABI void swap(thread& __x, thread& __y) _NOEXCEPT { __x.swap(__y); }
diff --git a/libcxx/include/barrier b/libcxx/include/barrier
index 41fbfb3e8fb7b..66f1012ccf4a9 100644
--- a/libcxx/include/barrier
+++ b/libcxx/include/barrier
@@ -158,7 +158,7 @@ class barrier {
public:
using arrival_token = typename __barrier_base<_CompletionF>::arrival_token;
- static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept { return __barrier_base<_CompletionF>::max(); }
+ [[nodiscard]] static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept { return __barrier_base<_CompletionF>::max(); }
_LIBCPP_HIDE_FROM_ABI explicit barrier(ptrdiff_t __count, _CompletionF __completion = _CompletionF())
: __b_(__count, std::move(__completion)) {
diff --git a/libcxx/include/latch b/libcxx/include/latch
index c3b8f62e9b50e..d5ff63a5fcfd2 100644
--- a/libcxx/include/latch
+++ b/libcxx/include/latch
@@ -70,7 +70,7 @@ class latch {
atomic<ptrdiff_t> __a_;
public:
- static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept { return numeric_limits<ptrdiff_t>::max(); }
+ [[nodiscard]] static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept { return numeric_limits<ptrdiff_t>::max(); }
inline _LIBCPP_HIDE_FROM_ABI constexpr explicit latch(ptrdiff_t __expected) : __a_(__expected) {
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
diff --git a/libcxx/include/mutex b/libcxx/include/mutex
index 0b81f1bb1c8a6..1c31bafc0da6b 100644
--- a/libcxx/include/mutex
+++ b/libcxx/include/mutex
@@ -234,7 +234,7 @@ public:
typedef __libcpp_recursive_mutex_t* native_handle_type;
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; }
};
class _LIBCPP_EXPORTED_FROM_ABI timed_mutex {
diff --git a/libcxx/include/semaphore b/libcxx/include/semaphore
index 99c4ad24b35ec..d490d4f4abfb3 100644
--- a/libcxx/include/semaphore
+++ b/libcxx/include/semaphore
@@ -133,7 +133,7 @@ class counting_semaphore {
public:
static_assert(__least_max_value >= 0, "The least maximum value must be a positive number");
- static constexpr ptrdiff_t max() noexcept { return __least_max_value; }
+ [[nodiscard]] static constexpr ptrdiff_t max() noexcept { return __least_max_value; }
_LIBCPP_HIDE_FROM_ABI constexpr explicit counting_semaphore(ptrdiff_t __count) : __semaphore_(__count) {
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
diff --git a/libcxx/test/libcxx/thread/nodiscard.verify.cpp b/libcxx/test/libcxx/thread/nodiscard.verify.cpp
new file mode 100644
index 0000000000000..47ede0321ea77
--- /dev/null
+++ b/libcxx/test/libcxx/thread/nodiscard.verify.cpp
@@ -0,0 +1,94 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+// UNSUPPORTED: no-threads
+
+// Check that functions are marked [[nodiscard]]
+
+#include <barrier>
+#include <latch>
+#include <mutex>
+#include <semaphore>
+#include <thread>
+
+#include "test_macros.h"
+
+void test() {
+ // Threads
+ {
+ std::thread th;
+
+ th.joinable(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ th.get_id(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ th.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ th.hardware_concurrency(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+#if TEST_STD_VER >= 20
+ {
+ std::jthread jt;
+
+ jt.joinable(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ jt.get_id(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ jt.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ jt.get_stop_source(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ jt.get_stop_token(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ jt.hardware_concurrency(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+#endif
+
+ // Mutual exclusion
+
+ { // <mutex>
+ std::mutex m;
+
+ m.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ {
+ std::recursive_mutex m;
+
+ m.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+
+ // Condition variables
+
+ { // <condition_variable>
+ std::condition_variable cv;
+
+ cv.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+
+#if TEST_STD_VER >= 20
+
+ // Semaphores
+
+ { // <semaphor>
+ std::counting_semaphore<> cs{0};
+
+ cs.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+ std::binary_semaphore bs{0};
+
+ bs.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+
+ // Latches and barriers
+
+ { // <barrier>
+ std::barrier<> b{94};
+
+ b.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ { // <latch>
+ std::latch l{94};
+
+ l.max(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+
+#endif
+}
\ No newline at end of file
diff --git a/libcxx/test/std/thread/thread.jthread/nodiscard.verify.cpp b/libcxx/test/std/thread/thread.jthread/nodiscard.verify.cpp
deleted file mode 100644
index 2ef5cf874da90..0000000000000
--- a/libcxx/test/std/thread/thread.jthread/nodiscard.verify.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// UNSUPPORTED: no-threads
-// UNSUPPORTED: c++03, c++11, c++14, c++17
-
-// [[nodiscard]] bool joinable() const noexcept;
-// [[nodiscard]] id get_id() const noexcept;
-// [[nodiscard]] native_handle_type native_handle();
-// [[nodiscard]] stop_source get_stop_source() noexcept;
-// [[nodiscard]] stop_token get_stop_token() const noexcept;
-// [[nodiscard]] static unsigned int hardware_concurrency() noexcept;
-
-#include <thread>
-
-void test() {
- std::jthread jt;
- jt.joinable(); // expected-warning {{ignoring return value of function}}
- jt.get_id(); // expected-warning {{ignoring return value of function}}
- jt.native_handle(); // expected-warning {{ignoring return value of function}}
- jt.get_stop_source(); // expected-warning {{ignoring return value of function}}
- jt.get_stop_token(); // expected-warning {{ignoring return value of function}}
- jt.hardware_concurrency(); // expected-warning {{ignoring return value of function}}
-}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
`[[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 anotated in this patch: - [x] `<barrier>` - [x] `<condition_variable>` - [x] `<latch>` - [x] `<mutex>` - [x] `<semaphore>` - [x] `<thread>`
8267f0d to
059c7f0
Compare
[[nodiscard]] to concurrency[[nodiscard]] to concurrency (partially)
libcxx/include/__mutex/mutex.h
Outdated
| # endif | ||
|
|
||
| _LIBCPP_ACQUIRE_CAPABILITY() void lock(); | ||
| _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) bool try_lock() _NOEXCEPT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a prime candidate for [[nodiscard]] as well.
libcxx/include/latch
Outdated
| if (__old == __update) | ||
| __a_.notify_all(); | ||
| } | ||
| inline _LIBCPP_HIDE_FROM_ABI bool try_wait() const noexcept { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one as well.
libcxx/include/mutex
Outdated
| recursive_mutex& operator=(const recursive_mutex&) = delete; | ||
|
|
||
| void lock(); | ||
| bool try_lock() _NOEXCEPT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here. Same for the ones defined below.
| static_assert(__least_max_value >= 0, "The least maximum value must be a positive number"); | ||
|
|
||
| static constexpr ptrdiff_t max() noexcept { return __least_max_value; } | ||
| [[nodiscard]] static constexpr ptrdiff_t max() noexcept { return __least_max_value; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, the try_* functions below.
d126f49 to
4ad45f4
Compare
|
Thank you! |
) `[[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.
) `[[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.
[[nodiscard]]should be applied to functions where discarding the return value is most likely a correctness issue.The following utilities have been annotated in this patch:
<barrier><condition_variable><latch><mutex><semaphore><thread>N.B. Some classes don't provide all specified methods, which were not annotated.