Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 20 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions libcxx/include/__mutex/lock_guard.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
# pragma GCC system_header
#endif

#ifndef _LIBCPP_HAS_NO_THREADS

_LIBCPP_BEGIN_NAMESPACE_STD

template <class _Mutex>
Expand Down Expand Up @@ -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
10 changes: 3 additions & 7 deletions libcxx/include/__mutex/tag_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
# pragma GCC system_header
#endif

#ifndef _LIBCPP_HAS_NO_THREADS

_LIBCPP_BEGIN_NAMESPACE_STD

struct _LIBCPP_EXPORTED_FROM_ABI defer_lock_t {
Expand All @@ -31,18 +29,16 @@ struct _LIBCPP_EXPORTED_FROM_ABI adopt_lock_t {
explicit adopt_lock_t() = default;
};

# if _LIBCPP_STD_VER >= 17
#if _LIBCPP_STD_VER >= 17
inline constexpr defer_lock_t defer_lock = defer_lock_t();
inline constexpr try_to_lock_t try_to_lock = try_to_lock_t();
inline constexpr adopt_lock_t adopt_lock = adopt_lock_t();
# elif !defined(_LIBCPP_CXX03_LANG)
#elif !defined(_LIBCPP_CXX03_LANG)
constexpr defer_lock_t defer_lock = defer_lock_t();
constexpr try_to_lock_t try_to_lock = try_to_lock_t();
constexpr adopt_lock_t adopt_lock = adopt_lock_t();
# endif
#endif

_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP_HAS_NO_THREADS

#endif // _LIBCPP___MUTEX_TAG_TYPES_H
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// UNSUPPORTED: no-threads

// UNSUPPORTED: c++03

// <mutex>
Expand All @@ -19,21 +18,33 @@
#include <cstdlib>
#include <cassert>

#include "make_test_thread.h"
#include "test_macros.h"

std::mutex m;
struct MyMutex {
bool locked = false;

void do_try_lock() {
assert(m.try_lock() == false);
}
MyMutex() = default;
~MyMutex() { assert(!locked); }

void lock() {
assert(!locked);
locked = true;
}
void unlock() {
assert(locked);
locked = false;
}

MyMutex(MyMutex const&) = delete;
MyMutex& operator=(MyMutex const&) = delete;
};

int main(int, char**) {
MyMutex m;
{
m.lock();
std::lock_guard<std::mutex> lg(m, std::adopt_lock);
std::thread t = support::make_test_thread(do_try_lock);
t.join();
std::lock_guard<MyMutex> lg(m, std::adopt_lock);
assert(m.locked);
}

m.lock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,9 @@

#include <mutex>

int main(int, char**)
{
std::mutex m0;
std::mutex m1;
std::lock_guard<std::mutex> lg0(m0);
std::lock_guard<std::mutex> lg(m1);
lg = lg0;
class MyMutex;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@

#include <mutex>

int main(int, char**)
{
std::mutex m;
std::lock_guard<std::mutex> lg0(m);
std::lock_guard<std::mutex> lg(lg0);
class MyMutex;

int main(int, char**) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

static_assert(!std::is_copy_constructible<std::lock_guard<MyMutex> >::value, "");
return 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
//
//===----------------------------------------------------------------------===//

// UNSUPPORTED: no-threads
// UNSUPPORTED: c++03, c++11, c++14

// <mutex>
Expand All @@ -19,11 +18,16 @@

#include "test_macros.h"

struct MyMutex {
void lock() {}
void unlock() {}
};

int main(int, char**) {
std::mutex mutex;
MyMutex m;
{
std::lock_guard lock(mutex);
ASSERT_SAME_TYPE(decltype(lock), std::lock_guard<std::mutex>);
std::lock_guard lg(m);
ASSERT_SAME_TYPE(decltype(lg), std::lock_guard<MyMutex>);
}

return 0;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
//===----------------------------------------------------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//

// <mutex>

// template <class Mutex> class lock_guard;

// explicit lock_guard(mutex_type& m);

// template<class _Mutex> lock_guard(lock_guard<_Mutex>)
// -> lock_guard<_Mutex>; // C++17

#include <mutex>
#include <cassert>

#include "test_macros.h"

struct MyMutex {
bool locked = false;

MyMutex() = default;
~MyMutex() { assert(!locked); }

void lock() {
assert(!locked);
locked = true;
}
void unlock() {
assert(locked);
locked = false;
}

MyMutex(MyMutex const&) = delete;
MyMutex& operator=(MyMutex const&) = delete;
};

int main(int, char**) {
MyMutex m;
{
std::lock_guard<MyMutex> lg(m);
assert(m.locked);
}

m.lock();
m.unlock();

#if TEST_STD_VER >= 17
std::lock_guard lg(m);
static_assert((std::is_same<decltype(lg), std::lock_guard<decltype(m)>>::value), "");
#endif

return 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
//
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

//===----------------------------------------------------------------------===//

// UNSUPPORTED: no-threads

// <mutex>

// template <class Mutex> class lock_guard;
Expand All @@ -16,10 +14,12 @@

#include <mutex>

struct MyMutex {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


int main(int, char**)
{
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}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


return 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// UNSUPPORTED: no-threads

// <mutex>

Expand All @@ -23,10 +21,10 @@

#include "test_macros.h"

int main(int, char**)
{
static_assert((std::is_same<std::lock_guard<std::mutex>::mutex_type,
std::mutex>::value), "");
struct MyMutex {};

int main(int, char**) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

static_assert((std::is_same<std::lock_guard<MyMutex>::mutex_type, MyMutex>::value), "");

return 0;
}