[libcxx-commits] [libcxx] c9af0e6 - [libc++] counting_semaphore should not be default-constructible.

[Arthur O'Dwyer via libcxx-commits]

Author: Arthur O'Dwyer
Date: 2021-09-21T16:19:31-04:00
New Revision: c9af0e61fa85842ce280ddab8ab491de38a7ae5b

URL: https://github.com/llvm/llvm-project/commit/c9af0e61fa85842ce280ddab8ab491de38a7ae5b
DIFF: https://github.com/llvm/llvm-project/commit/c9af0e61fa85842ce280ddab8ab491de38a7ae5b.diff

LOG: [libc++] counting_semaphore should not be default-constructible.

Neither the current C++2b draft, nor any revision of [p1135],
nor libstdc++, claims that `counting_semaphore` should be
default-constructible. I think this was just a copy-paste issue
somehow.

Also, `explicit` was missing from the constructor.

Also, `constexpr` remains missing; but that's probably more of a
technical limitation, since apparently there are some platforms
where we don't (can't??) use the atomic implementation and
have to rely on pthreads, which obviously isn't constexpr.

Differential Revision: https://reviews.llvm.org/D110042

Added: 
    libcxx/test/std/thread/thread.semaphore/ctor.compile.pass.cpp

Modified: 
    libcxx/include/semaphore
    libcxx/test/std/thread/thread.semaphore/binary.pass.cpp
    libcxx/test/std/thread/thread.semaphore/max.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/semaphore b/libcxx/include/semaphore
index 906f62e0f07aa..4f9ecd0461b26 100644
--- a/libcxx/include/semaphore
+++ b/libcxx/include/semaphore
@@ -82,7 +82,7 @@ class __atomic_semaphore_base
 
 public:
     _LIBCPP_INLINE_VISIBILITY
-    __atomic_semaphore_base(ptr
diff _t __count) : __a(__count)
+    constexpr explicit __atomic_semaphore_base(ptr
diff _t __count) : __a(__count)
     {
     }
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
@@ -136,7 +136,7 @@ class __platform_semaphore_base
 
 public:
     _LIBCPP_INLINE_VISIBILITY
-    __platform_semaphore_base(ptr
diff _t __count) :
+    explicit __platform_semaphore_base(ptr
diff _t __count) :
         __semaphore()
     {
         __libcpp_semaphore_init(&__semaphore, __count);
@@ -190,7 +190,7 @@ public:
     }
 
     _LIBCPP_INLINE_VISIBILITY
-    counting_semaphore(ptr
diff _t __count = 0) : __semaphore(__count) { }
+    constexpr explicit counting_semaphore(ptr
diff _t __count) : __semaphore(__count) { }
     ~counting_semaphore() = default;
 
     counting_semaphore(const counting_semaphore&) = delete;

diff  --git a/libcxx/test/std/thread/thread.semaphore/binary.pass.cpp b/libcxx/test/std/thread/thread.semaphore/binary.pass.cpp
index b80c9fea0295c..20e4efa70ee8d 100644
--- a/libcxx/test/std/thread/thread.semaphore/binary.pass.cpp
+++ b/libcxx/test/std/thread/thread.semaphore/binary.pass.cpp
@@ -18,10 +18,13 @@
 #include <semaphore>
 #include <chrono>
 #include <thread>
+#include <type_traits>
 
 #include "make_test_thread.h"
 #include "test_macros.h"
 
+static_assert(std::is_same<std::binary_semaphore, std::counting_semaphore<1>>::value, "");
+
 int main(int, char**)
 {
   std::binary_semaphore s(1);

diff  --git a/libcxx/test/std/thread/thread.semaphore/ctor.compile.pass.cpp b/libcxx/test/std/thread/thread.semaphore/ctor.compile.pass.cpp
new file mode 100644
index 0000000000000..785a57e29d492
--- /dev/null
+++ b/libcxx/test/std/thread/thread.semaphore/ctor.compile.pass.cpp
@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: libcpp-has-no-threads
+// UNSUPPORTED: c++03, c++11
+
+// <semaphore>
+
+// constexpr explicit counting_semaphore(ptr
diff _t desired);
+
+#include <semaphore>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_default_constructible<std::binary_semaphore>::value, "");
+static_assert(!std::is_default_constructible<std::counting_semaphore<>>::value, "");
+
+static_assert(!std::is_convertible<int, std::binary_semaphore>::value, "");
+static_assert(!std::is_convertible<int, std::counting_semaphore<>>::value, "");
+
+#if 0 // TODO FIXME: the ctor should be constexpr when TEST_STD_VER > 17
+constinit std::binary_semaphore bs(1);
+constinit std::counting_semaphore cs(1);
+#endif

diff  --git a/libcxx/test/std/thread/thread.semaphore/max.pass.cpp b/libcxx/test/std/thread/thread.semaphore/max.pass.cpp
index 8007bc1383750..8f48640a19459 100644
--- a/libcxx/test/std/thread/thread.semaphore/max.pass.cpp
+++ b/libcxx/test/std/thread/thread.semaphore/max.pass.cpp
@@ -18,10 +18,9 @@
 
 int main(int, char**)
 {
-  static_assert(std::counting_semaphore<>::max() > 0, "");
+  static_assert(std::counting_semaphore<>::max() >= 1, "");
   static_assert(std::counting_semaphore<1>::max() >= 1, "");
-  static_assert(std::counting_semaphore<std::numeric_limits<int>::max()>::max() >= 1, "");
-  static_assert(std::counting_semaphore<std::numeric_limits<ptr
diff _t>::max()>::max() >= 1, "");
-  static_assert(std::counting_semaphore<1>::max() == std::binary_semaphore::max(), "");
+  static_assert(std::counting_semaphore<std::numeric_limits<int>::max()>::max() >= std::numeric_limits<int>::max(), "");
+  static_assert(std::counting_semaphore<std::numeric_limits<ptr
diff _t>::max()>::max() == std::numeric_limits<ptr
diff _t>::max(), "");
   return 0;
 }