aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJonathan Wakely <jwakely@redhat.com>2023-08-17 18:58:24 +0100
committerJonathan Wakely <jwakely@redhat.com>2023-11-11 00:41:08 +0000
commit64bcf3f07a211f2ea136d99b3700c172efc08d93 (patch)
treecb061d1af41e4353cc82f2a075eec6eae08ac8d9
parenta92a434024c59f57dc24328d946f97a5e71cee94 (diff)
downloadgcc-64bcf3f07a211f2ea136d99b3700c172efc08d93.zip
gcc-64bcf3f07a211f2ea136d99b3700c172efc08d93.tar.gz
gcc-64bcf3f07a211f2ea136d99b3700c172efc08d93.tar.bz2
libstdc++: Add [[nodiscard]] to lock types
Adding this attribute means users get a warning when they accidentally create a temporary lock instead of creating an automatic variable with block scope. For std::lock_guard both constructors have side effects (they both take a mutex and so both cause it to be unlocked at the end of the full expression when a temporary is constructed). Ideally we would just put the attribute on the class instead of the constructors, but that doesn't work with GCC (PR c++/85973). For std::unique_lock the default constructor and std::defer_lock_t constructor do not cause any locking or unlocking, so do not need to give a warning. It might still be a mistake to create a temporary using those constructors, but it's harmless and seems unlikely anyway. For a lock object created with one of those constructors you would expect the lock object to be referred to later in the function, and that would not even compile if it was constructed as an unnamed temporary. std::scoped_lock gets the same treatment as std::lock_guard, except that the explicit specialization for zero lockables has no side effects so doesn't need to warn. libstdc++-v3/ChangeLog: * include/bits/std_mutex.h (lock_guard): Add [[nodiscard]] attribute to constructors. * include/bits/unique_lock.h (unique_lock): Likewise. * include/std/mutex (scoped_lock, scoped_lock<Mutex>): Likewise. * testsuite/30_threads/lock_guard/cons/nodiscard.cc: New test. * testsuite/30_threads/scoped_lock/cons/nodiscard.cc: New test. * testsuite/30_threads/unique_lock/cons/nodiscard.cc: New test.
-rw-r--r--libstdc++-v3/include/bits/std_mutex.h2
-rw-r--r--libstdc++-v3/include/bits/unique_lock.h5
-rw-r--r--libstdc++-v3/include/std/mutex5
-rw-r--r--libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc20
-rw-r--r--libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc29
-rw-r--r--libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc40
6 files changed, 101 insertions, 0 deletions
diff --git a/libstdc++-v3/include/bits/std_mutex.h b/libstdc++-v3/include/bits/std_mutex.h
index 4693055..9ac8c76 100644
--- a/libstdc++-v3/include/bits/std_mutex.h
+++ b/libstdc++-v3/include/bits/std_mutex.h
@@ -245,9 +245,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
public:
typedef _Mutex mutex_type;
+ [[__nodiscard__]]
explicit lock_guard(mutex_type& __m) : _M_device(__m)
{ _M_device.lock(); }
+ [[__nodiscard__]]
lock_guard(mutex_type& __m, adopt_lock_t) noexcept : _M_device(__m)
{ } // calling thread owns mutex
diff --git a/libstdc++-v3/include/bits/unique_lock.h b/libstdc++-v3/include/bits/unique_lock.h
index c28e645..07474d2 100644
--- a/libstdc++-v3/include/bits/unique_lock.h
+++ b/libstdc++-v3/include/bits/unique_lock.h
@@ -66,6 +66,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _M_device(0), _M_owns(false)
{ }
+ [[__nodiscard__]]
explicit unique_lock(mutex_type& __m)
: _M_device(std::__addressof(__m)), _M_owns(false)
{
@@ -77,10 +78,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _M_device(std::__addressof(__m)), _M_owns(false)
{ }
+ [[__nodiscard__]]
unique_lock(mutex_type& __m, try_to_lock_t)
: _M_device(std::__addressof(__m)), _M_owns(_M_device->try_lock())
{ }
+ [[__nodiscard__]]
unique_lock(mutex_type& __m, adopt_lock_t) noexcept
: _M_device(std::__addressof(__m)), _M_owns(true)
{
@@ -88,6 +91,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Clock, typename _Duration>
+ [[__nodiscard__]]
unique_lock(mutex_type& __m,
const chrono::time_point<_Clock, _Duration>& __atime)
: _M_device(std::__addressof(__m)),
@@ -95,6 +99,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ }
template<typename _Rep, typename _Period>
+ [[__nodiscard__]]
unique_lock(mutex_type& __m,
const chrono::duration<_Rep, _Period>& __rtime)
: _M_device(std::__addressof(__m)),
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index bd3a1cb..9d22ce8 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -744,9 +744,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
class scoped_lock
{
public:
+
+ [[nodiscard]]
explicit scoped_lock(_MutexTypes&... __m) : _M_devices(std::tie(__m...))
{ std::lock(__m...); }
+ [[nodiscard]]
explicit scoped_lock(adopt_lock_t, _MutexTypes&... __m) noexcept
: _M_devices(std::tie(__m...))
{ } // calling thread owns mutex
@@ -779,9 +782,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
public:
using mutex_type = _Mutex;
+ [[nodiscard]]
explicit scoped_lock(mutex_type& __m) : _M_device(__m)
{ _M_device.lock(); }
+ [[nodiscard]]
explicit scoped_lock(adopt_lock_t, mutex_type& __m) noexcept
: _M_device(__m)
{ } // calling thread owns mutex
diff --git a/libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc b/libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc
new file mode 100644
index 0000000..50137f9
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+
+#include <mutex>
+
+struct Mutex
+{
+ void lock();
+ void unlock();
+ bool try_lock();
+};
+
+using namespace std;
+
+void
+test_nodiscard(Mutex& m)
+{
+ lock_guard<Mutex>{m}; // { dg-warning "ignoring return value" }
+ lock_guard<Mutex>{m, adopt_lock}; // { dg-warning "ignoring return value" }
+ lock_guard<Mutex>(m, adopt_lock); // { dg-warning "ignoring return value" }
+}
diff --git a/libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc b/libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc
new file mode 100644
index 0000000..ca673e2
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc
@@ -0,0 +1,29 @@
+// { dg-do compile { target c++17 } }
+// { dg-require-gthreads "" }
+// { dg-additional-options "-pthread" { target pthread } }
+
+#include <mutex>
+
+struct Mutex
+{
+ void lock();
+ void unlock();
+ bool try_lock();
+};
+
+using namespace std;
+
+void
+test_nodiscard(Mutex& m, mutex& m2)
+{
+ scoped_lock<>{}; // no warning
+ scoped_lock<>(); // no warning
+
+ scoped_lock{m}; // { dg-warning "ignoring return value" }
+ scoped_lock{adopt_lock, m}; // { dg-warning "ignoring return value" }
+ scoped_lock(adopt_lock, m); // { dg-warning "ignoring return value" }
+ scoped_lock(m, m2); // { dg-warning "ignoring return value" }
+ scoped_lock{m, m2}; // { dg-warning "ignoring return value" }
+ scoped_lock{adopt_lock, m, m2}; // { dg-warning "ignoring return value" }
+ scoped_lock(adopt_lock, m, m2); // { dg-warning "ignoring return value" }
+}
diff --git a/libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc
new file mode 100644
index 0000000..79e347e
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc
@@ -0,0 +1,40 @@
+// { dg-do compile { target c++11 } }
+
+#include <mutex>
+#include <chrono>
+
+using namespace std;
+
+struct Mutex
+{
+ void lock();
+ void unlock();
+ bool try_lock();
+
+ bool try_lock_for(chrono::seconds);
+ bool try_lock_until(chrono::system_clock::time_point);
+};
+
+
+void
+test_nodiscard(Mutex& m)
+{
+ unique_lock<Mutex>(); // no warning
+ unique_lock<Mutex>{}; // no warning
+ unique_lock<Mutex>(m, defer_lock); // no warning
+ unique_lock<Mutex>{m, defer_lock}; // no warning
+
+ unique_lock<Mutex>{m}; // { dg-warning "ignoring return value" }
+ unique_lock<Mutex>{m, try_to_lock}; // { dg-warning "ignoring return value" }
+ unique_lock<Mutex>(m, try_to_lock); // { dg-warning "ignoring return value" }
+ unique_lock<Mutex>{m, adopt_lock}; // { dg-warning "ignoring return value" }
+ unique_lock<Mutex>(m, adopt_lock); // { dg-warning "ignoring return value" }
+
+ chrono::seconds reltime(1);
+ unique_lock<Mutex>{m, reltime}; // { dg-warning "ignoring return value" }
+ unique_lock<Mutex>(m, reltime); // { dg-warning "ignoring return value" }
+ chrono::system_clock::time_point abstime(reltime);
+ unique_lock<Mutex>{m, abstime}; // { dg-warning "ignoring return value" }
+ unique_lock<Mutex>(m, abstime); // { dg-warning "ignoring return value" }
+}
+