[libcxx-commits] [libcxx] [libc++] Mark scoped_lock and unique_lock constructors as [[nodiscard]] (PR #89397)
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Apr 25 07:52:45 PDT 2024
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/89397
>From 5d0cb37c6ec05f45126e8ad46050f8e4701ab612 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 19 Apr 2024 11:05:17 -0400
Subject: [PATCH] [libc++] Mark scoped_lock constructors as [[nodiscard]]
It's basically always a bug to discard a scoped_lock.
Fixes #89388
---
libcxx/include/__mutex/unique_lock.h | 20 ++++---
libcxx/include/mutex | 16 +++---
.../diagnostics/mutex.nodiscard.verify.cpp | 53 +++++++++++++++++--
.../thread.lock.guard/nodiscard.verify.cpp | 30 -----------
4 files changed, 70 insertions(+), 49 deletions(-)
delete mode 100644 libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.verify.cpp
diff --git a/libcxx/include/__mutex/unique_lock.h b/libcxx/include/__mutex/unique_lock.h
index c27ce4b24c1ae8..4a616ba51ee1cf 100644
--- a/libcxx/include/__mutex/unique_lock.h
+++ b/libcxx/include/__mutex/unique_lock.h
@@ -36,26 +36,28 @@ class _LIBCPP_TEMPLATE_VIS unique_lock {
bool __owns_;
public:
- _LIBCPP_HIDE_FROM_ABI unique_lock() _NOEXCEPT : __m_(nullptr), __owns_(false) {}
- _LIBCPP_HIDE_FROM_ABI explicit unique_lock(mutex_type& __m) : __m_(std::addressof(__m)), __owns_(true) {
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock() _NOEXCEPT : __m_(nullptr), __owns_(false) {}
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI explicit unique_lock(mutex_type& __m)
+ : __m_(std::addressof(__m)), __owns_(true) {
__m_->lock();
}
- _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
: __m_(std::addressof(__m)),
__owns_(false) {}
- _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, try_to_lock_t)
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, try_to_lock_t)
: __m_(std::addressof(__m)), __owns_(__m.try_lock()) {}
- _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, adopt_lock_t) : __m_(std::addressof(__m)), __owns_(true) {}
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, adopt_lock_t)
+ : __m_(std::addressof(__m)), __owns_(true) {}
template <class _Clock, class _Duration>
- _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __t)
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __t)
: __m_(std::addressof(__m)), __owns_(__m.try_lock_until(__t)) {}
template <class _Rep, class _Period>
- _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __d)
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __d)
: __m_(std::addressof(__m)), __owns_(__m.try_lock_for(__d)) {}
_LIBCPP_HIDE_FROM_ABI ~unique_lock() {
@@ -66,7 +68,9 @@ class _LIBCPP_TEMPLATE_VIS unique_lock {
unique_lock(unique_lock const&) = delete;
unique_lock& operator=(unique_lock const&) = delete;
- _LIBCPP_HIDE_FROM_ABI unique_lock(unique_lock&& __u) _NOEXCEPT : __m_(__u.__m_), __owns_(__u.__owns_) {
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI unique_lock(unique_lock&& __u) _NOEXCEPT
+ : __m_(__u.__m_),
+ __owns_(__u.__owns_) {
__u.__m_ = nullptr;
__u.__owns_ = false;
}
diff --git a/libcxx/include/mutex b/libcxx/include/mutex
index 12fae9a88b9d7e..0d2b5914bc4fd5 100644
--- a/libcxx/include/mutex
+++ b/libcxx/include/mutex
@@ -427,10 +427,10 @@ class _LIBCPP_TEMPLATE_VIS scoped_lock;
template <>
class _LIBCPP_TEMPLATE_VIS scoped_lock<> {
public:
- explicit scoped_lock() {}
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock() {}
~scoped_lock() = default;
- _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t) {}
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t) {}
scoped_lock(scoped_lock const&) = delete;
scoped_lock& operator=(scoped_lock const&) = delete;
@@ -445,13 +445,15 @@ private:
mutex_type& __m_;
public:
- explicit scoped_lock(mutex_type& __m) _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__m)) : __m_(__m) {
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(mutex_type& __m)
+ _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__m))
+ : __m_(__m) {
__m_.lock();
}
~scoped_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability()) { __m_.unlock(); }
- _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t, mutex_type& __m)
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t, mutex_type& __m)
_LIBCPP_THREAD_SAFETY_ANNOTATION(requires_capability(__m))
: __m_(__m) {}
@@ -465,9 +467,11 @@ class _LIBCPP_TEMPLATE_VIS scoped_lock {
typedef tuple<_MArgs&...> _MutexTuple;
public:
- _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(_MArgs&... __margs) : __t_(__margs...) { std::lock(__margs...); }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(_MArgs&... __margs) : __t_(__margs...) {
+ std::lock(__margs...);
+ }
- _LIBCPP_HIDE_FROM_ABI scoped_lock(adopt_lock_t, _MArgs&... __margs) : __t_(__margs...) {}
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI scoped_lock(adopt_lock_t, _MArgs&... __margs) : __t_(__margs...) {}
_LIBCPP_HIDE_FROM_ABI ~scoped_lock() {
typedef typename __make_tuple_indices<sizeof...(_MArgs)>::type _Indices;
diff --git a/libcxx/test/libcxx/diagnostics/mutex.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/mutex.nodiscard.verify.cpp
index a98eb5f142113b..403514233926bc 100644
--- a/libcxx/test/libcxx/diagnostics/mutex.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/mutex.nodiscard.verify.cpp
@@ -12,14 +12,57 @@
// check that <mutex> functions are marked [[nodiscard]]
-// clang-format off
-
#include <mutex>
+#include <chrono>
#include "test_macros.h"
void test() {
- std::mutex mutex;
- std::lock_guard<std::mutex>{mutex}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
- std::lock_guard<std::mutex>{mutex, std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ // std::scoped_lock
+ {
+#if TEST_STD_VER >= 17
+ using M = std::mutex;
+ M m0, m1, m2;
+ // clang-format off
+ std::scoped_lock<>{}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::scoped_lock<M>{m0}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::scoped_lock<M, M>{m0, m1}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::scoped_lock<M, M, M>{m0, m1, m2}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+
+ std::scoped_lock<>{std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::scoped_lock<M>{std::adopt_lock, m0}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::scoped_lock<M, M>{std::adopt_lock, m0, m1}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::scoped_lock<M, M, M>{std::adopt_lock, m0, m1, m2}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ // clang-format on
+#endif
+ }
+
+ // std::unique_lock
+ {
+ 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
+ std::unique_lock<M>{}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::unique_lock<M>{m}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::unique_lock<M>{m, std::defer_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::unique_lock<M>{m, std::try_to_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::unique_lock<M>{m, std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::unique_lock<M>{m, time_point}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ 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::lock_guard
+ {
+ std::mutex m;
+ // clang-format off
+ std::lock_guard<std::mutex>{m}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ std::lock_guard<std::mutex>{m, std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ // clang-format on
+ }
}
diff --git a/libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.verify.cpp b/libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.verify.cpp
deleted file mode 100644
index 5fe68c83b3d9f6..00000000000000
--- a/libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.verify.cpp
+++ /dev/null
@@ -1,30 +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
-
-// [[nodiscard]] isn't supported in C++03 (not even as an extension)
-// UNSUPPORTED: c++03
-
-// <mutex>
-
-// template <class Mutex> class lock_guard;
-
-// [[nodiscard]] explicit lock_guard(mutex_type& m);
-// [[nodiscard]] lock_guard(mutex_type& m, adopt_lock_t);
-
-// Test that we properly apply [[nodiscard]] to lock_guard's constructors,
-// which is a libc++ extension.
-
-#include <mutex>
-
-void f() {
- std::mutex m;
- std::lock_guard<std::mutex>{m}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
- std::lock_guard<std::mutex>{m, std::adopt_lock}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
-}
More information about the libcxx-commits
mailing list