[libcxx-commits] [libcxx] [libc++] Mark scoped_lock constructors as [[nodiscard]] (PR #89397)

[Louis Dionne via libcxx-commits]

https://github.com/ldionne created https://github.com/llvm/llvm-project/pull/89397

It's basically always a bug to discard a scoped_lock.

Fixes #89388

>From 57729dd1aa7f56386693e08d342ec53cebd29aed 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                          | 16 +++++----
 .../thread.lock.scoped/nodiscard.verify.cpp   | 34 +++++++++++++++++++
 2 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 libcxx/test/libcxx/thread/thread.lock/thread.lock.scoped/nodiscard.verify.cpp

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/thread/thread.lock/thread.lock.scoped/nodiscard.verify.cpp b/libcxx/test/libcxx/thread/thread.lock/thread.lock.scoped/nodiscard.verify.cpp
new file mode 100644
index 00000000000000..86c1953457837f
--- /dev/null
+++ b/libcxx/test/libcxx/thread/thread.lock/thread.lock.scoped/nodiscard.verify.cpp
@@ -0,0 +1,34 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// <mutex>
+
+// template <class ...Mutex> class scoped_lock;
+
+// Test that we properly apply [[nodiscard]] to scoped_lock's constructors.
+
+#include <mutex>
+
+void f() {
+  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
+}