[libc++] Make `std::lock_guard` available with `_LIBCPP_HAS_NO_THREADS` #98717

petrhosek · 2024-07-13T05:54:28Z

This change makes std::lock_guard available when _LIBCPP_HAS_NO_THREADS is set. This class is generic and doesn't require threading support, and is regularly used even in environments where threading isn't available like embedded.

fixes #89891

This change makes std::lock_guard, std::unique_lock, std::scoped_lock, and std::shared_lock available when _LIBCPP_HAS_NO_THREADS is set. These classes are generic and don't require threading support, and are regularly used even in environments where threading isn't available like embedded. fixes llvm#89891

llvmbot · 2024-07-13T05:54:58Z

@llvm/pr-subscribers-libcxx

Author: Petr Hosek (petrhosek)

Changes

This change makes std::lock_guard, std::unique_lock, std::scoped_lock, and std::shared_lock available when _LIBCPP_HAS_NO_THREADS is set. These classes are generic and don't require threading support, and are regularly used even in environments where threading isn't available like embedded.

fixes #89891

Full diff: https://github.com/llvm/llvm-project/pull/98717.diff

4 Files Affected:

(modified) libcxx/include/__mutex/lock_guard.h (-4)
(modified) libcxx/include/__mutex/unique_lock.h (-4)
(modified) libcxx/include/mutex (+3-3)
(modified) libcxx/include/shared_mutex (+7-8)

diff --git a/libcxx/include/__mutex/lock_guard.h b/libcxx/include/__mutex/lock_guard.h
index 8340b9bbd4453..ef56896be9f68 100644
--- a/libcxx/include/__mutex/lock_guard.h
+++ b/libcxx/include/__mutex/lock_guard.h
@@ -16,8 +16,6 @@
 #  pragma GCC system_header
 #endif
 
-#ifndef _LIBCPP_HAS_NO_THREADS
-
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Mutex>
@@ -47,6 +45,4 @@ _LIBCPP_CTAD_SUPPORTED_FOR_TYPE(lock_guard);
 
 _LIBCPP_END_NAMESPACE_STD
 
-#endif // _LIBCPP_HAS_NO_THREADS
-
 #endif // _LIBCPP___MUTEX_LOCK_GUARD_H
diff --git a/libcxx/include/__mutex/unique_lock.h b/libcxx/include/__mutex/unique_lock.h
index 4a616ba51ee1c..5df791de4c742 100644
--- a/libcxx/include/__mutex/unique_lock.h
+++ b/libcxx/include/__mutex/unique_lock.h
@@ -22,8 +22,6 @@
 #  pragma GCC system_header
 #endif
 
-#ifndef _LIBCPP_HAS_NO_THREADS
-
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Mutex>
@@ -172,6 +170,4 @@ inline _LIBCPP_HIDE_FROM_ABI void swap(unique_lock<_Mutex>& __x, unique_lock<_Mu
 
 _LIBCPP_END_NAMESPACE_STD
 
-#endif // _LIBCPP_HAS_NO_THREADS
-
 #endif // _LIBCPP___MUTEX_UNIQUE_LOCK_H
diff --git a/libcxx/include/mutex b/libcxx/include/mutex
index 02c52dd72f02b..d4d7539240094 100644
--- a/libcxx/include/mutex
+++ b/libcxx/include/mutex
@@ -419,8 +419,9 @@ inline _LIBCPP_HIDE_FROM_ABI void lock(_L0& __l0, _L1& __l1, _L2& __l2, _L3&...
 }
 
 #  endif // _LIBCPP_CXX03_LANG
+#endif   // !_LIBCPP_HAS_NO_THREADS
 
-#  if _LIBCPP_STD_VER >= 17
+#if _LIBCPP_STD_VER >= 17
 template <class... _Mutexes>
 class _LIBCPP_TEMPLATE_VIS scoped_lock;
 
@@ -491,8 +492,7 @@ private:
 };
 _LIBCPP_CTAD_SUPPORTED_FOR_TYPE(scoped_lock);
 
-#  endif // _LIBCPP_STD_VER >= 17
-#endif   // !_LIBCPP_HAS_NO_THREADS
+#endif // _LIBCPP_STD_VER >= 17
 
 _LIBCPP_END_NAMESPACE_STD
 
diff --git a/libcxx/include/shared_mutex b/libcxx/include/shared_mutex
index 397ac290d9b2e..5918af4a214c1 100644
--- a/libcxx/include/shared_mutex
+++ b/libcxx/include/shared_mutex
@@ -122,16 +122,11 @@ template <class Mutex>
 
 */
 
-#include <__config>
-
-#ifdef _LIBCPP_HAS_NO_THREADS
-#  error "<shared_mutex> is not supported since libc++ has been configured without support for threads."
-#endif
-
 #include <__chrono/duration.h>
 #include <__chrono/steady_clock.h>
 #include <__chrono/time_point.h>
 #include <__condition_variable/condition_variable.h>
+#include <__config>
 #include <__memory/addressof.h>
 #include <__mutex/mutex.h>
 #include <__mutex/tag_types.h>
@@ -152,6 +147,8 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+#  ifndef _LIBCPP_HAS_NO_THREADS
+
 struct _LIBCPP_EXPORTED_FROM_ABI __shared_mutex_base {
   mutex __mut_;
   condition_variable __gate1_;
@@ -181,7 +178,7 @@ struct _LIBCPP_EXPORTED_FROM_ABI __shared_mutex_base {
   //     native_handle_type native_handle(); // See 30.2.3
 };
 
-#  if _LIBCPP_STD_VER >= 17
+#    if _LIBCPP_STD_VER >= 17
 class _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_THREAD_SAFETY_ANNOTATION(__capability__("shared_mutex")) shared_mutex {
   __shared_mutex_base __base_;
 
@@ -218,7 +215,7 @@ public:
   //     typedef __shared_mutex_base::native_handle_type native_handle_type;
   //     _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return __base::unlock_shared(); }
 };
-#  endif
+#    endif
 
 class _LIBCPP_EXPORTED_FROM_ABI
 _LIBCPP_THREAD_SAFETY_ANNOTATION(__capability__("shared_timed_mutex")) shared_timed_mutex {
@@ -308,6 +305,8 @@ bool shared_timed_mutex::try_lock_shared_until(const chrono::time_point<_Clock,
   return true;
 }
 
+#  endif   // !_LIBCPP_HAS_NO_THREADS
+
 template <class _Mutex>
 class shared_lock {
 public:

github-actions · 2024-07-13T05:58:23Z

✅ With the latest revision this PR passed the C/C++ code formatter.

philnik777

There should be at least some test that this works with threading disabled. I don't know whether we have any tests currently that could now be enabled. If not, let's add at least a simple one for all the classes.

libcxx/include/mutex

petrhosek · 2024-07-13T23:29:56Z

There should be at least some test that this works with threading disabled. I don't know whether we have any tests currently that could now be enabled. If not, let's add at least a simple one for all the classes.

There are several tests that should now pass even with threading disabled, I removed UNSUPPORTED: no-threads from all of them.

petrhosek · 2024-07-15T07:11:48Z

After doing more testing, I decide to exclude std::unique_lock, std::scoped_lock, and std::shared_lock since those have additional dependencies. The change should be ready for review as-is since all tests appear to be passing.

ldionne · 2024-07-15T16:15:37Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/assign.compile.fail.cpp

@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//

+// UNSUPPORTED: no-threads


We can lift this UNSUPPORTED by defining our own mutex-like class. Also, while we're at it this test should be a .compile.pass.cpp and it should test static_assert(!std::is_copy_assignable<std::lock_guard<MyMutexType>>::value, "");.

ldionne · 2024-07-15T16:16:02Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/copy.compile.fail.cpp

@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//

+// UNSUPPORTED: no-threads


Similar comment here.

ldionne · 2024-07-15T16:18:29Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/mutex.pass.cpp

@@ -18,31 +18,44 @@
 //     -> lock_guard<_Mutex>;  // C++17

 #include <mutex>


I would suggest that we formulate the tests in terms of the TestMutex only, that way they would all become enabled when threading is disabled.

And then we can have a single test (probably in this test file) where we test basic interoperation between std::lock_guard and std::mutex, and that one would be the only one with UNSUPPORTED: no-threads.

ldionne

A few more comments but this LGTM once they are applied.

ldionne · 2024-07-16T13:30:56Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/assign.compile.pass.cpp

+int main(int, char**) {
+  static_assert(!std::is_copy_assignable<std::lock_guard<MyMutex> >::value, "");
  return 0;
 }


Suggested change

int main(int, char**) {

static_assert(!std::is_copy_assignable<std::lock_guard<MyMutex> >::value, "");

return 0;

}

static_assert(!std::is_copy_assignable<std::lock_guard<MyMutex> >::value, "");

Just as a matter of style, we don't use main function in these compile-only tests when they are not needed to avoid any confusion about the test running.

ldionne · 2024-07-16T13:31:02Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/copy.compile.pass.cpp


+int main(int, char**) {


ldionne · 2024-07-16T13:32:21Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/mutex.verify.cpp

@@ -16,10 +14,12 @@

 #include <mutex>

+struct MyMutex {};


I think you need to use a valid BasicLockable all the time, otherwise the code is ill-formed. I suggest you define MyMutex in a local header like types.h and include it from all the tests in this directory that use MyMutex.

ldionne · 2024-07-16T13:32:41Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/types.pass.cpp

-                   std::mutex>::value), "");
+struct MyMutex {};
+
+int main(int, char**) {


No main needed, and this can be a .compile.pass.cpp

ldionne · 2024-07-16T13:34:44Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/mutex.verify.cpp

@@ -6,8 +6,6 @@
 //


Maybe rename to ctor.mutex.pass.cpp and test the constructor as a whole instead?

ldionne · 2024-07-16T13:35:48Z

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/mutex.verify.cpp

-    std::mutex m;
-    std::lock_guard<std::mutex> lg = m; // expected-error{{no viable conversion}}
+  MyMutex m;
+  std::lock_guard<MyMutex> lg = m; // expected-error{{no viable conversion}}


Instead, we should do something like

std::lock_guard<MyMutex> lg(m); // make sure this compiles and runs static_assert(!std::is_convertible_v<MyMutex, std::lock_guard<MyMutex>>, "constructor must be explicit");

petrhosek · 2024-07-16T16:33:24Z

A few more comments but this LGTM once they are applied.

Thanks, I believe it should be all addressed.

ldionne

LGTM if CI is green.

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/ctor.adopt_lock.pass.cpp

…S` (llvm#98717) This change makes `std::lock_guard` available when `_LIBCPP_HAS_NO_THREADS` is set. This class is generic and doesn't require threading support, and is regularly used even in environments where threading isn't available like embedded. fixes llvm#89891 --------- Co-authored-by: Louis Dionne <[email protected]>

…S` (#98717) Summary: This change makes `std::lock_guard` available when `_LIBCPP_HAS_NO_THREADS` is set. This class is generic and doesn't require threading support, and is regularly used even in environments where threading isn't available like embedded. fixes #89891 --------- Co-authored-by: Louis Dionne <[email protected]> Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251575

…99562) This is a follow up to #98717, which made lock_guard available under _LIBCPP_HAS_NO_THREADS. We can make unique_lock available under similar circumstances. This patch follows the example in #98717, by: - Removing the preprocessor guards for _LIBCPP_HAS_NO_THREADS in the unique_lock header. - providing a set of custom mutex implementations in a local header. - using custom locks in tests that can be made to work under `no-threads`.

…lvm#99562) This is a follow up to llvm#98717, which made lock_guard available under _LIBCPP_HAS_NO_THREADS. We can make unique_lock available under similar circumstances. This patch follows the example in llvm#98717, by: - Removing the preprocessor guards for _LIBCPP_HAS_NO_THREADS in the unique_lock header. - providing a set of custom mutex implementations in a local header. - using custom locks in tests that can be made to work under `no-threads`.

petrhosek requested a review from ldionne July 13, 2024 05:54

petrhosek requested a review from a team as a code owner July 13, 2024 05:54

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 13, 2024

petrhosek changed the title ~~[libcxx] Make locks available with _LIBCPP_HAS_NO_THREADS~~ [libc++] Make locks available with _LIBCPP_HAS_NO_THREADS Jul 13, 2024

philnik777 reviewed Jul 13, 2024

View reviewed changes

ilovepi reviewed Jul 13, 2024

View reviewed changes

libcxx/include/mutex Outdated Show resolved Hide resolved

Update test annotations

2b4d430

petrhosek added 5 commits July 13, 2024 16:59

More tests

e06a8e6

Fix formatting

896cdc3

Make defer_lock_t, try_to_lock_t and adopt_lock_t available also

d717ad2

Make std::lock and std::try_lock available also

252198d

Exclude std::unique_lock, std::scoped_lock and std::shared_lock

3ffe2d1

petrhosek changed the title ~~[libc++] Make locks available with _LIBCPP_HAS_NO_THREADS~~ [libc++] Make std::lock_guard available with _LIBCPP_HAS_NO_THREADS Jul 15, 2024

petrhosek requested a review from a team July 15, 2024 15:04

ldionne requested changes Jul 15, 2024

View reviewed changes

petrhosek added 5 commits July 15, 2024 10:40

Merge remote-tracking branch 'origin/main' into libcxx-no-threads-locks

b5d686e

Update tests

cd00542

Update test

757a485

Update tests

38d2425

Fix formatting

d7eb493

ldionne reviewed Jul 16, 2024

View reviewed changes

Extract helper class into a header

f07b064

petrhosek added 2 commits July 16, 2024 09:50

Missing include

da38a9f

std::is_convertible_v is only available since C++17

5cdda4a

Apply minor review comments

9f95bb1

ldionne approved these changes Jul 16, 2024

View reviewed changes

petrhosek commented Jul 16, 2024

View reviewed changes

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/ctor.adopt_lock.pass.cpp Show resolved Hide resolved

ldionne and others added 4 commits July 16, 2024 15:57

Fix out-of-scope issue

c1384a3

Fix other ref issue

09c8e0c

Fix issues in C++03 mode

5273378

Update test name

2ed78f1

petrhosek merged commit 6192f45 into llvm:main Jul 17, 2024
9 of 11 checks passed

ilovepi mentioned this pull request Jul 18, 2024

[libc++] Make std::unique_lock available with _LIBCPP_HAS_NO_THREADS #99562

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++] Make `std::lock_guard` available with `_LIBCPP_HAS_NO_THREADS` #98717

[libc++] Make `std::lock_guard` available with `_LIBCPP_HAS_NO_THREADS` #98717

petrhosek commented Jul 13, 2024 •

edited

Loading

llvmbot commented Jul 13, 2024

github-actions bot commented Jul 13, 2024 •

edited

Loading

philnik777 left a comment

petrhosek commented Jul 13, 2024

petrhosek commented Jul 15, 2024

ldionne Jul 15, 2024

petrhosek Jul 15, 2024

ldionne Jul 15, 2024

petrhosek Jul 15, 2024

ldionne Jul 15, 2024

petrhosek Jul 15, 2024

ldionne left a comment

ldionne Jul 16, 2024

petrhosek Jul 16, 2024

ldionne Jul 16, 2024

petrhosek Jul 16, 2024

ldionne Jul 16, 2024

petrhosek Jul 16, 2024

ldionne Jul 16, 2024

petrhosek Jul 16, 2024

ldionne Jul 16, 2024

petrhosek Jul 16, 2024

ldionne Jul 16, 2024

petrhosek Jul 16, 2024

petrhosek commented Jul 16, 2024

ldionne left a comment

		@@ -18,31 +18,44 @@
		// -> lock_guard<_Mutex>; // C++17

		#include <mutex>

[libc++] Make std::lock_guard available with _LIBCPP_HAS_NO_THREADS #98717

[libc++] Make std::lock_guard available with _LIBCPP_HAS_NO_THREADS #98717

Conversation

petrhosek commented Jul 13, 2024 • edited Loading

llvmbot commented Jul 13, 2024

github-actions bot commented Jul 13, 2024 • edited Loading

philnik777 left a comment

Choose a reason for hiding this comment

petrhosek commented Jul 13, 2024

petrhosek commented Jul 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldionne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrhosek commented Jul 16, 2024

ldionne left a comment

Choose a reason for hiding this comment

[libc++] Make `std::lock_guard` available with `_LIBCPP_HAS_NO_THREADS` #98717

[libc++] Make `std::lock_guard` available with `_LIBCPP_HAS_NO_THREADS` #98717

petrhosek commented Jul 13, 2024 •

edited

Loading

github-actions bot commented Jul 13, 2024 •

edited

Loading