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.

diff --git a/libstdc++-v3/include/bits/std_mutex.h b/libstdc++-v3/include/bits/std_mutex.h
index 4693055269d5e7e04d31d58e073c5578fddb877f..9ac8c76c9fbc41b5a951d2b40c1de1e226650eba 100644 (file)
--- 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 c28e6456ad5e37fe6742e4917b68cb2d48275d9e..07474d26db5306dfce6dcbf937399e8d6bd16e29 100644 (file)
--- 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 bd3a1cbd94de8d86cdff795063d35286302890ff..9d22ce80045f88d95e24412d914d099f24978a8c 100644 (file)
--- 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 (file)
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 (file)
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 (file)
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" }
+}
+