Revert "constexpr mutex constructor (microsoft#3824)"

This reverts commit 8d18fec.

Conflicts:
* stl/inc/xthreads.h
* stl/src/cond.cpp
* stl/src/mutex.cpp
* stl/src/primitives.hpp

stl/inc/mutex

@@ -30,20 +30,37 @@ _STD_BEGIN
 _EXPORT_STD class condition_variable;
 _EXPORT_STD class condition_variable_any;
 
+struct _Mtx_internal_imp_mirror {
+#ifdef _CRT_WINDOWS
+#ifdef _WIN64
+    static constexpr size_t _Critical_section_size = 16;
+#else // _WIN64
+    static constexpr size_t _Critical_section_size = 8;
+#endif // _WIN64
+#else // _CRT_WINDOWS
+#ifdef _WIN64
+    static constexpr size_t _Critical_section_size = 64;
+#else // _WIN64
+    static constexpr size_t _Critical_section_size = 36;
+#endif // _WIN64
+#endif // _CRT_WINDOWS
+
+    static constexpr size_t _Critical_section_align = alignof(void*);
+
+    int _Type;
+    _Aligned_storage_t<_Critical_section_size, _Critical_section_align> _Cs_storage;
+    long _Thread_id;
+    int _Count;
+};
+
+static_assert(sizeof(_Mtx_internal_imp_mirror) == _Mtx_internal_imp_size, "inconsistent size for mutex");
+static_assert(alignof(_Mtx_internal_imp_mirror) == _Mtx_internal_imp_alignment, "inconsistent alignment for mutex");
+
 class _Mutex_base { // base class for all mutex types
 public:
-#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
     _Mutex_base(int _Flags = 0) noexcept {
         _Mtx_init_in_situ(_Mymtx(), _Flags | _Mtx_try);
     }
-#else // ^^^ _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR / !_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR vvv
-    constexpr _Mutex_base(int _Flags = 0) noexcept {
-        _Mtx_storage._Critical_section = {};
-        _Mtx_storage._Thread_id        = -1;
-        _Mtx_storage._Type             = _Flags | _Mtx_try;
-        _Mtx_storage._Count            = 0;
-    }
-#endif // !_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
 
     ~_Mutex_base() noexcept {
         _Mtx_destroy_in_situ(_Mymtx());
@@ -79,9 +96,9 @@ public:
 
 protected:
     _NODISCARD_TRY_CHANGE_STATE bool _Verify_ownership_levels() noexcept {
-        if (_Mtx_storage._Count == INT_MAX) {
+        if (_Mtx_storage_mirror._Count == INT_MAX) {
             // only occurs for recursive mutexes (N4950 [thread.mutex.recursive]/3)
-            --_Mtx_storage._Count;
+            --_Mtx_storage_mirror._Count;
             return false;
         }
 
@@ -92,16 +109,23 @@ private:
     friend condition_variable;
     friend condition_variable_any;
 
-    _Mtx_internal_imp_t _Mtx_storage{};
+    union {
+        _Aligned_storage_t<_Mtx_internal_imp_size, _Mtx_internal_imp_alignment> _Mtx_storage;
+        _Mtx_internal_imp_mirror _Mtx_storage_mirror;
+    };
 
-    _Mtx_t _Mymtx() noexcept {
-        return &_Mtx_storage;
+    _Mtx_t _Mymtx() noexcept { // get pointer to _Mtx_internal_imp_t inside _Mtx_storage
+        return reinterpret_cast<_Mtx_t>(&_Mtx_storage);
     }
 };
 
+static_assert(sizeof(_Mutex_base) == _Mtx_internal_imp_size, "inconsistent size for mutex");
+static_assert(alignof(_Mutex_base) == _Mtx_internal_imp_alignment, "inconsistent alignment for mutex");
+
 _EXPORT_STD class mutex : public _Mutex_base { // class for mutual exclusion
 public:
-    mutex() noexcept = default;
+    /* constexpr */ mutex() noexcept // TRANSITION, ABI
+        : _Mutex_base() {}
 
     mutex(const mutex&)            = delete;
     mutex& operator=(const mutex&) = delete;

stl/inc/xthreads.h

@@ -8,7 +8,6 @@
 #include <yvals_core.h>
 #if _STL_COMPILER_PREPROCESSOR
 #include <climits>
-#include <type_traits>
 #include <xtimec.h>
 
 #pragma pack(push, _CRT_PACKING)
@@ -25,56 +24,40 @@ struct _Thrd_t { // thread identifier for Win32
     _Thrd_id_t _Id;
 };
 
-using _Smtx_t = void*;
-
-struct _Stl_critical_section {
-    void* _Unused       = nullptr; // TRANSITION, ABI: was the vptr
-    _Smtx_t _M_srw_lock = nullptr;
-};
-
-struct _Mtx_internal_imp_t {
+// Size and alignment for _Mtx_internal_imp_t and _Cnd_internal_imp_t
 #ifdef _CRT_WINDOWS
 #ifdef _WIN64
-    static constexpr size_t _Critical_section_size = 16;
-#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv
-    static constexpr size_t _Critical_section_size = 8;
-#endif // ^^^ !defined(_WIN64) ^^^
-#else // ^^^ defined(_CRT_WINDOWS) / !defined(_CRT_WINDOWS) vvv
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_size      = 32;
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 8;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_size      = 16;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 8;
+#else // _WIN64
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_size      = 20;
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 4;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_size      = 8;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 4;
+#endif // _WIN64
+#else // _CRT_WINDOWS
 #ifdef _WIN64
-    static constexpr size_t _Critical_section_size = 64;
-#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv
-    static constexpr size_t _Critical_section_size = 36;
-#endif // ^^^ !defined(_WIN64) ^^^
-#endif // ^^^ !defined(_CRT_WINDOWS) ^^^
-
-    static constexpr size_t _Critical_section_align = alignof(void*);
-
-    int _Type{};
-    union {
-        _Stl_critical_section _Critical_section{};
-        _STD _Aligned_storage_t<_Critical_section_size, _Critical_section_align> _Cs_storage;
-    };
-    long _Thread_id{};
-    int _Count{};
-};
-
-// Size and alignment for _Cnd_internal_imp_t
-#if defined(_CRT_WINDOWS) // for Windows-internal code
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 2 * sizeof(void*);
-#elif defined(_WIN64) // ordinary 64-bit code
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 72;
-#else // vvv ordinary 32-bit code vvv
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 40;
-#endif // ^^^ ordinary 32-bit code ^^^
-
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = alignof(void*);
-
-using _Mtx_t = _Mtx_internal_imp_t*;
-
-#ifdef _M_CEE // avoid warning LNK4248: unresolved typeref token for '_Cnd_internal_imp_t'; image may not run
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_size      = 80;
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 8;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_size      = 72;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 8;
+#else // _WIN64
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_size      = 48;
+_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 4;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_size      = 40;
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 4;
+#endif // _WIN64
+#endif // _CRT_WINDOWS
+
+#ifdef _M_CEE // avoid warning LNK4248: unresolved typeref token for '_Mtx_internal_imp_t'; image may not run
+using _Mtx_t = void*;
 using _Cnd_t = void*;
 #else // ^^^ defined(_M_CEE) / !defined(_M_CEE) vvv
+struct _Mtx_internal_imp_t;
 struct _Cnd_internal_imp_t;
+using _Mtx_t = _Mtx_internal_imp_t*;
 using _Cnd_t = _Cnd_internal_imp_t*;
 #endif // ^^^ !defined(_M_CEE) ^^^
 
@@ -103,6 +86,7 @@ _CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t);
 _CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t, int);
 _CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t);
 _CRTIMP2_PURE int __cdecl _Mtx_current_owns(_Mtx_t);
+_CRTIMP2_PURE void* __cdecl _Mtx_getconcrtcs(_Mtx_t);
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_lock(_Mtx_t);
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t);
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_unlock(_Mtx_t); // TRANSITION, ABI: Always succeeds
@@ -114,6 +98,7 @@ _CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t);
 
 // shared mutex
 // these declarations must be in sync with those in sharedmutex.cpp
+using _Smtx_t = void*;
 void __cdecl _Smtx_lock_exclusive(_Smtx_t*);
 void __cdecl _Smtx_lock_shared(_Smtx_t*);
 int __cdecl _Smtx_try_lock_exclusive(_Smtx_t*);

stl/src/cond.cpp

@@ -49,7 +49,7 @@ _CRTIMP2_PURE void __cdecl _Cnd_destroy(const _Cnd_t cond) { // clean up
 }
 
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) { // wait until signaled
-    const auto cs = &mtx->_Critical_section;
+    const auto cs = static_cast<Concurrency::details::stl_critical_section_win7*>(_Mtx_getconcrtcs(mtx));
     _Mtx_clear_owner(mtx);
     cond->_get_cv()->wait(cs);
     _Mtx_reset_owner(mtx);
@@ -60,7 +60,7 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait(
     const _Cnd_t cond, const _Mtx_t mtx, const _timespec64* const target) {
     _Thrd_result res = _Thrd_result::_Success;
-    const auto cs    = &mtx->_Critical_section;
+    const auto cs    = static_cast<Concurrency::details::stl_critical_section_win7*>(_Mtx_getconcrtcs(mtx));
     if (target == nullptr) { // no target time specified, wait on mutex
         _Mtx_clear_owner(mtx);
         cond->_get_cv()->wait(cs);

stl/src/mutex.cpp

@@ -33,20 +33,35 @@ _EXTERN_C
 enum class __stl_sync_api_modes_enum { normal, win7, vista, concrt };
 _CRTIMP2 void __cdecl __set_stl_sync_api_mode(__stl_sync_api_modes_enum) {}
 
-[[nodiscard]] static PSRWLOCK get_srw_lock(_Mtx_t mtx) {
-    return reinterpret_cast<PSRWLOCK>(&mtx->_Critical_section._M_srw_lock);
-}
+struct _Mtx_internal_imp_t { // ConcRT mutex
+    int type;
+    typename std::_Aligned_storage<Concurrency::details::stl_critical_section_max_size,
+        Concurrency::details::stl_critical_section_max_alignment>::type cs;
+    long thread_id;
+    int count;
+    [[nodiscard]] Concurrency::details::stl_critical_section_win7* _get_cs() { // get pointer to implementation
+        return reinterpret_cast<Concurrency::details::stl_critical_section_win7*>(&cs);
+    }
+};
+
+static_assert(sizeof(_Mtx_internal_imp_t) == _Mtx_internal_imp_size, "incorrect _Mtx_internal_imp_size");
+static_assert(alignof(_Mtx_internal_imp_t) == _Mtx_internal_imp_alignment, "incorrect _Mtx_internal_imp_alignment");
+
+static_assert(
+    std::_Mtx_internal_imp_mirror::_Critical_section_size == Concurrency::details::stl_critical_section_max_size);
+static_assert(
+    std::_Mtx_internal_imp_mirror::_Critical_section_align == Concurrency::details::stl_critical_section_max_alignment);
 
 // TRANSITION, only used when constexpr mutex constructor is not enabled
 _CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t mtx, int type) { // initialize mutex in situ
-    Concurrency::details::create_stl_critical_section(&mtx->_Critical_section);
-    mtx->_Thread_id = -1;
-    mtx->_Type      = type;
-    mtx->_Count     = 0;
+    Concurrency::details::create_stl_critical_section(mtx->_get_cs());
+    mtx->thread_id = -1;
+    mtx->type      = type;
+    mtx->count     = 0;
 }
 
 _CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t mtx) { // destroy mutex in situ
-    _THREAD_ASSERT(mtx->_Count == 0, "mutex destroyed while busy");
+    _THREAD_ASSERT(mtx->count == 0, "mutex destroyed while busy");
     (void) mtx;
 }
 
@@ -75,29 +90,29 @@ _CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t mtx) { // destroy mutex
 static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex
     // TRANSITION, ABI: the use of `const _timespec64*` is preserved for `_Mtx_timedlock`
     const auto current_thread_id = static_cast<long>(GetCurrentThreadId());
-    if ((mtx->_Type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock
+    if ((mtx->type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock
         // TRANSITION, ABI: this branch is preserved for `_Thrd_create`
 
-        if (mtx->_Thread_id != current_thread_id) { // not current thread, do lock
-            AcquireSRWLockExclusive(get_srw_lock(mtx));
-            mtx->_Thread_id = current_thread_id;
+        if (mtx->thread_id != current_thread_id) { // not current thread, do lock
+            mtx->_get_cs()->lock();
+            mtx->thread_id = current_thread_id;
         }
-        ++mtx->_Count;
+        ++mtx->count;
 
         return _Thrd_result::_Success;
     } else { // handle timed or recursive mutex
         int res = WAIT_TIMEOUT;
         if (target == nullptr) { // no target --> plain wait (i.e. infinite timeout)
-            if (mtx->_Thread_id != current_thread_id) {
-                AcquireSRWLockExclusive(get_srw_lock(mtx));
+            if (mtx->thread_id != current_thread_id) {
+                mtx->_get_cs()->lock();
             }
 
             res = WAIT_OBJECT_0;
 
         } else if (target->tv_sec < 0 || target->tv_sec == 0 && target->tv_nsec <= 0) {
             // target time <= 0 --> plain trylock or timed wait for time that has passed; try to lock with 0 timeout
-            if (mtx->_Thread_id != current_thread_id) { // not this thread, lock it
-                if (TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) {
+            if (mtx->thread_id != current_thread_id) { // not this thread, lock it
+                if (mtx->_get_cs()->try_lock()) {
                     res = WAIT_OBJECT_0;
                 }
             } else {
@@ -110,8 +125,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock
             _Timespec64_get_sys(&now);
             while (now.tv_sec < target->tv_sec || now.tv_sec == target->tv_sec && now.tv_nsec < target->tv_nsec) {
                 // time has not expired
-                if (mtx->_Thread_id == current_thread_id
-                    || TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) { // stop waiting
+                if (mtx->thread_id == current_thread_id || mtx->_get_cs()->try_lock()) { // stop waiting
                     res = WAIT_OBJECT_0;
                     break;
                 }
@@ -121,13 +135,13 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock
         }
 
         if (res == WAIT_OBJECT_0) {
-            if (1 < ++mtx->_Count) { // check count
-                if ((mtx->_Type & _Mtx_recursive) != _Mtx_recursive) { // not recursive, fixup count
-                    --mtx->_Count;
+            if (1 < ++mtx->count) { // check count
+                if ((mtx->type & _Mtx_recursive) != _Mtx_recursive) { // not recursive, fixup count
+                    --mtx->count;
                     res = WAIT_TIMEOUT;
                 }
             } else {
-                mtx->_Thread_id = current_thread_id;
+                mtx->thread_id = current_thread_id;
             }
         }
 
@@ -147,14 +161,11 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock
 
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_unlock(_Mtx_t mtx) { // unlock mutex
     _THREAD_ASSERT(
-        1 <= mtx->_Count && mtx->_Thread_id == static_cast<long>(GetCurrentThreadId()), "unlock of unowned mutex");
-
-    if (--mtx->_Count == 0) { // leave critical section
-        mtx->_Thread_id = -1;
+        1 <= mtx->count && mtx->thread_id == static_cast<long>(GetCurrentThreadId()), "unlock of unowned mutex");
 
-        auto srw_lock = get_srw_lock(mtx);
-        _Analysis_assume_lock_held_(*srw_lock);
-        ReleaseSRWLockExclusive(srw_lock);
+    if (--mtx->count == 0) { // leave critical section
+        mtx->thread_id = -1;
+        mtx->_get_cs()->unlock();
     }
     return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
 }
@@ -165,7 +176,7 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_lock(_Mtx_t mtx) { // lock mutex
 
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t mtx) { // attempt to lock try_mutex
     _timespec64 xt;
-    _THREAD_ASSERT((mtx->_Type & (_Mtx_try | _Mtx_timed)) != 0, "trylock not supported by mutex");
+    _THREAD_ASSERT((mtx->type & (_Mtx_try | _Mtx_timed)) != 0, "trylock not supported by mutex");
     xt.tv_sec  = 0;
     xt.tv_nsec = 0;
     return mtx_do_lock(mtx, &xt);
@@ -175,28 +186,27 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t mtx) { // attempt to lock
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_timedlock(_Mtx_t mtx, const _timespec64* xt) { // attempt to lock timed mutex
     _Thrd_result res;
 
-    _THREAD_ASSERT((mtx->_Type & _Mtx_timed) != 0, "timedlock not supported by mutex");
+    _THREAD_ASSERT((mtx->type & _Mtx_timed) != 0, "timedlock not supported by mutex");
     res = mtx_do_lock(mtx, xt);
     return res == _Thrd_result::_Busy ? _Thrd_result::_Timedout : res;
 }
 
 _CRTIMP2_PURE int __cdecl _Mtx_current_owns(_Mtx_t mtx) { // test if current thread owns mutex
-    return mtx->_Count != 0 && mtx->_Thread_id == static_cast<long>(GetCurrentThreadId());
+    return mtx->count != 0 && mtx->thread_id == static_cast<long>(GetCurrentThreadId());
 }
 
-// TRANSITION, ABI: preserved for binary compatibility
 _CRTIMP2_PURE void* __cdecl _Mtx_getconcrtcs(_Mtx_t mtx) { // get internal cs impl
-    return &mtx->_Critical_section;
+    return mtx->_get_cs();
 }
 
 _CRTIMP2_PURE void __cdecl _Mtx_clear_owner(_Mtx_t mtx) { // set owner to nobody
-    mtx->_Thread_id = -1;
-    --mtx->_Count;
+    mtx->thread_id = -1;
+    --mtx->count;
 }
 
 _CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t mtx) { // set owner to current thread
-    mtx->_Thread_id = static_cast<long>(GetCurrentThreadId());
-    ++mtx->_Count;
+    mtx->thread_id = static_cast<long>(GetCurrentThreadId());
+    ++mtx->count;
 }
 
 _END_EXTERN_C