[libcxx][P1831R1] Deprecating volatile: library #101439
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Josh Karns (jkarns275) ChangesP1831R1 introduces 2 key sets of changes:
In the case of (2) I am wondering if this should be feature gated? Currently it will add the restriction regardless of I'm also unsure if I'm supposed to update the status page myself. Full diff: https://github.com/llvm/llvm-project/pull/101439.diff 5 Files Affected:
diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h
index bcea21f5ce2e1..74c7d49c65055 100644
--- a/libcxx/include/__atomic/atomic.h
+++ b/libcxx/include/__atomic/atomic.h
@@ -46,7 +46,9 @@ struct atomic : public __atomic_base<_Tp> {
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR atomic(_Tp __d) _NOEXCEPT : __base(__d) {}
- _LIBCPP_HIDE_FROM_ABI _Tp operator=(_Tp __d) volatile _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _Tp operator=(_Tp __d) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
__base::store(__d);
return __d;
}
@@ -71,7 +73,9 @@ struct atomic<_Tp*> : public __atomic_base<_Tp*> {
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR atomic(_Tp* __d) _NOEXCEPT : __base(__d) {}
- _LIBCPP_HIDE_FROM_ABI _Tp* operator=(_Tp* __d) volatile _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _Tp* operator=(_Tp* __d) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
__base::store(__d);
return __d;
}
@@ -80,7 +84,9 @@ struct atomic<_Tp*> : public __atomic_base<_Tp*> {
return __d;
}
- _LIBCPP_HIDE_FROM_ABI _Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
// __atomic_fetch_add accepts function pointers, guard against them.
static_assert(!is_function<__remove_pointer_t<_Tp> >::value, "Pointer to function isn't allowed");
return std::__cxx_atomic_fetch_add(std::addressof(this->__a_), __op, __m);
@@ -92,7 +98,9 @@ struct atomic<_Tp*> : public __atomic_base<_Tp*> {
return std::__cxx_atomic_fetch_add(std::addressof(this->__a_), __op, __m);
}
- _LIBCPP_HIDE_FROM_ABI _Tp* fetch_sub(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _Tp* fetch_sub(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
// __atomic_fetch_add accepts function pointers, guard against them.
static_assert(!is_function<__remove_pointer_t<_Tp> >::value, "Pointer to function isn't allowed");
return std::__cxx_atomic_fetch_sub(std::addressof(this->__a_), __op, __m);
@@ -104,18 +112,47 @@ struct atomic<_Tp*> : public __atomic_base<_Tp*> {
return std::__cxx_atomic_fetch_sub(std::addressof(this->__a_), __op, __m);
}
- _LIBCPP_HIDE_FROM_ABI _Tp* operator++(int) volatile _NOEXCEPT { return fetch_add(1); }
_LIBCPP_HIDE_FROM_ABI _Tp* operator++(int) _NOEXCEPT { return fetch_add(1); }
- _LIBCPP_HIDE_FROM_ABI _Tp* operator--(int) volatile _NOEXCEPT { return fetch_sub(1); }
+ _LIBCPP_HIDE_FROM_ABI _Tp* operator++(int) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_add(1);
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp* operator--(int) _NOEXCEPT { return fetch_sub(1); }
- _LIBCPP_HIDE_FROM_ABI _Tp* operator++() volatile _NOEXCEPT { return fetch_add(1) + 1; }
+ _LIBCPP_HIDE_FROM_ABI _Tp* operator--(int) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_sub(1);
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp* operator++() _NOEXCEPT { return fetch_add(1) + 1; }
- _LIBCPP_HIDE_FROM_ABI _Tp* operator--() volatile _NOEXCEPT { return fetch_sub(1) - 1; }
+ _LIBCPP_HIDE_FROM_ABI _Tp* operator++() volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_add(1) + 1;
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp* operator--() _NOEXCEPT { return fetch_sub(1) - 1; }
- _LIBCPP_HIDE_FROM_ABI _Tp* operator+=(ptrdiff_t __op) volatile _NOEXCEPT { return fetch_add(__op) + __op; }
+ _LIBCPP_HIDE_FROM_ABI _Tp* operator--() volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_sub(1) - 1;
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp* operator+=(ptrdiff_t __op) _NOEXCEPT { return fetch_add(__op) + __op; }
- _LIBCPP_HIDE_FROM_ABI _Tp* operator-=(ptrdiff_t __op) volatile _NOEXCEPT { return fetch_sub(__op) - __op; }
+ _LIBCPP_HIDE_FROM_ABI _Tp* operator+=(ptrdiff_t __op) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_add(__op) + __op;
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp* operator-=(ptrdiff_t __op) _NOEXCEPT { return fetch_sub(__op) - __op; }
+ _LIBCPP_HIDE_FROM_ABI _Tp* operator-=(ptrdiff_t __op) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_sub(__op) - __op;
+ }
atomic& operator=(const atomic&) = delete;
atomic& operator=(const atomic&) volatile = delete;
@@ -267,7 +304,9 @@ _LIBCPP_HIDE_FROM_ABI bool atomic_is_lock_free(const atomic<_Tp>* __o) _NOEXCEPT
template <class _Tp>
_LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI void
-atomic_init(volatile atomic<_Tp>* __o, typename atomic<_Tp>::value_type __d) _NOEXCEPT {
+atomic_init(volatile atomic<_Tp>* __o, typename atomic<_Tp>::value_type __d) _NOEXCEPT
+ requires __atomic_base<_Tp>::is_always_lock_free
+{
std::__cxx_atomic_init(std::addressof(__o->__a_), __d);
}
diff --git a/libcxx/include/__atomic/atomic_base.h b/libcxx/include/__atomic/atomic_base.h
index 93f5c4cff0d1b..35a6fb00dbc3e 100644
--- a/libcxx/include/__atomic/atomic_base.h
+++ b/libcxx/include/__atomic/atomic_base.h
@@ -43,7 +43,8 @@ struct __atomic_base // false
return static_cast<__atomic_base const volatile*>(this)->is_lock_free();
}
_LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
- _LIBCPP_CHECK_STORE_MEMORY_ORDER(__m) {
+ requires is_always_lock_free
+ _LIBCPP_CHECK_STORE_MEMORY_ORDER(__m) {
std::__cxx_atomic_store(std::addressof(__a_), __d, __m);
}
_LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order __m = memory_order_seq_cst) _NOEXCEPT
@@ -51,7 +52,8 @@ struct __atomic_base // false
std::__cxx_atomic_store(std::addressof(__a_), __d, __m);
}
_LIBCPP_HIDE_FROM_ABI _Tp load(memory_order __m = memory_order_seq_cst) const volatile _NOEXCEPT
- _LIBCPP_CHECK_LOAD_MEMORY_ORDER(__m) {
+ requires is_always_lock_free
+ _LIBCPP_CHECK_LOAD_MEMORY_ORDER(__m) {
return std::__cxx_atomic_load(std::addressof(__a_), __m);
}
_LIBCPP_HIDE_FROM_ABI _Tp load(memory_order __m = memory_order_seq_cst) const _NOEXCEPT
@@ -60,7 +62,9 @@ struct __atomic_base // false
}
_LIBCPP_HIDE_FROM_ABI operator _Tp() const volatile _NOEXCEPT { return load(); }
_LIBCPP_HIDE_FROM_ABI operator _Tp() const _NOEXCEPT { return load(); }
- _LIBCPP_HIDE_FROM_ABI _Tp exchange(_Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _Tp exchange(_Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires is_always_lock_free
+ {
return std::__cxx_atomic_exchange(std::addressof(__a_), __d, __m);
}
_LIBCPP_HIDE_FROM_ABI _Tp exchange(_Tp __d, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
@@ -68,7 +72,8 @@ struct __atomic_base // false
}
_LIBCPP_HIDE_FROM_ABI bool
compare_exchange_weak(_Tp& __e, _Tp __d, memory_order __s, memory_order __f) volatile _NOEXCEPT
- _LIBCPP_CHECK_EXCHANGE_MEMORY_ORDER(__s, __f) {
+ requires is_always_lock_free
+ _LIBCPP_CHECK_EXCHANGE_MEMORY_ORDER(__s, __f) {
return std::__cxx_atomic_compare_exchange_weak(std::addressof(__a_), std::addressof(__e), __d, __s, __f);
}
_LIBCPP_HIDE_FROM_ABI bool compare_exchange_weak(_Tp& __e, _Tp __d, memory_order __s, memory_order __f) _NOEXCEPT
@@ -77,7 +82,8 @@ struct __atomic_base // false
}
_LIBCPP_HIDE_FROM_ABI bool
compare_exchange_strong(_Tp& __e, _Tp __d, memory_order __s, memory_order __f) volatile _NOEXCEPT
- _LIBCPP_CHECK_EXCHANGE_MEMORY_ORDER(__s, __f) {
+ requires is_always_lock_free
+ _LIBCPP_CHECK_EXCHANGE_MEMORY_ORDER(__s, __f) {
return std::__cxx_atomic_compare_exchange_strong(std::addressof(__a_), std::addressof(__e), __d, __s, __f);
}
_LIBCPP_HIDE_FROM_ABI bool compare_exchange_strong(_Tp& __e, _Tp __d, memory_order __s, memory_order __f) _NOEXCEPT
@@ -85,7 +91,9 @@ struct __atomic_base // false
return std::__cxx_atomic_compare_exchange_strong(std::addressof(__a_), std::addressof(__e), __d, __s, __f);
}
_LIBCPP_HIDE_FROM_ABI bool
- compare_exchange_weak(_Tp& __e, _Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+ compare_exchange_weak(_Tp& __e, _Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires is_always_lock_free
+ {
return std::__cxx_atomic_compare_exchange_weak(std::addressof(__a_), std::addressof(__e), __d, __m, __m);
}
_LIBCPP_HIDE_FROM_ABI bool
@@ -93,7 +101,9 @@ struct __atomic_base // false
return std::__cxx_atomic_compare_exchange_weak(std::addressof(__a_), std::addressof(__e), __d, __m, __m);
}
_LIBCPP_HIDE_FROM_ABI bool
- compare_exchange_strong(_Tp& __e, _Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+ compare_exchange_strong(_Tp& __e, _Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires is_always_lock_free
+ {
return std::__cxx_atomic_compare_exchange_strong(std::addressof(__a_), std::addressof(__e), __d, __m, __m);
}
_LIBCPP_HIDE_FROM_ABI bool
@@ -141,55 +151,127 @@ struct __atomic_base<_Tp, true> : public __atomic_base<_Tp, false> {
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : __base(__d) {}
- _LIBCPP_HIDE_FROM_ABI _Tp fetch_add(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _Tp fetch_add(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
return std::__cxx_atomic_fetch_add(std::addressof(this->__a_), __op, __m);
}
+
_LIBCPP_HIDE_FROM_ABI _Tp fetch_add(_Tp __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
return std::__cxx_atomic_fetch_add(std::addressof(this->__a_), __op, __m);
}
- _LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+
+ _LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
return std::__cxx_atomic_fetch_sub(std::addressof(this->__a_), __op, __m);
}
+
_LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
return std::__cxx_atomic_fetch_sub(std::addressof(this->__a_), __op, __m);
}
- _LIBCPP_HIDE_FROM_ABI _Tp fetch_and(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+
+ _LIBCPP_HIDE_FROM_ABI _Tp fetch_and(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
return std::__cxx_atomic_fetch_and(std::addressof(this->__a_), __op, __m);
}
+
_LIBCPP_HIDE_FROM_ABI _Tp fetch_and(_Tp __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
return std::__cxx_atomic_fetch_and(std::addressof(this->__a_), __op, __m);
}
- _LIBCPP_HIDE_FROM_ABI _Tp fetch_or(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+
+ _LIBCPP_HIDE_FROM_ABI _Tp fetch_or(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
return std::__cxx_atomic_fetch_or(std::addressof(this->__a_), __op, __m);
}
+
_LIBCPP_HIDE_FROM_ABI _Tp fetch_or(_Tp __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
return std::__cxx_atomic_fetch_or(std::addressof(this->__a_), __op, __m);
}
- _LIBCPP_HIDE_FROM_ABI _Tp fetch_xor(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+
+ _LIBCPP_HIDE_FROM_ABI _Tp fetch_xor(_Tp __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
return std::__cxx_atomic_fetch_xor(std::addressof(this->__a_), __op, __m);
}
+
_LIBCPP_HIDE_FROM_ABI _Tp fetch_xor(_Tp __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
return std::__cxx_atomic_fetch_xor(std::addressof(this->__a_), __op, __m);
}
- _LIBCPP_HIDE_FROM_ABI _Tp operator++(int) volatile _NOEXCEPT { return fetch_add(_Tp(1)); }
_LIBCPP_HIDE_FROM_ABI _Tp operator++(int) _NOEXCEPT { return fetch_add(_Tp(1)); }
- _LIBCPP_HIDE_FROM_ABI _Tp operator--(int) volatile _NOEXCEPT { return fetch_sub(_Tp(1)); }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator++(int) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_add(_Tp(1));
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator--(int) _NOEXCEPT { return fetch_sub(_Tp(1)); }
- _LIBCPP_HIDE_FROM_ABI _Tp operator++() volatile _NOEXCEPT { return fetch_add(_Tp(1)) + _Tp(1); }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator--(int) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_sub(_Tp(1));
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator++() _NOEXCEPT { return fetch_add(_Tp(1)) + _Tp(1); }
- _LIBCPP_HIDE_FROM_ABI _Tp operator--() volatile _NOEXCEPT { return fetch_sub(_Tp(1)) - _Tp(1); }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator++() volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_add(_Tp(1)) + _Tp(1);
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator--() _NOEXCEPT { return fetch_sub(_Tp(1)) - _Tp(1); }
- _LIBCPP_HIDE_FROM_ABI _Tp operator+=(_Tp __op) volatile _NOEXCEPT { return fetch_add(__op) + __op; }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator--() volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_sub(_Tp(1)) - _Tp(1);
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator+=(_Tp __op) _NOEXCEPT { return fetch_add(__op) + __op; }
- _LIBCPP_HIDE_FROM_ABI _Tp operator-=(_Tp __op) volatile _NOEXCEPT { return fetch_sub(__op) - __op; }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator+=(_Tp __op) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_add(__op) + __op;
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator-=(_Tp __op) _NOEXCEPT { return fetch_sub(__op) - __op; }
- _LIBCPP_HIDE_FROM_ABI _Tp operator&=(_Tp __op) volatile _NOEXCEPT { return fetch_and(__op) & __op; }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator-=(_Tp __op) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_sub(__op) - __op;
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator&=(_Tp __op) _NOEXCEPT { return fetch_and(__op) & __op; }
- _LIBCPP_HIDE_FROM_ABI _Tp operator|=(_Tp __op) volatile _NOEXCEPT { return fetch_or(__op) | __op; }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator&=(_Tp __op) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_and(__op) & __op;
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator|=(_Tp __op) _NOEXCEPT { return fetch_or(__op) | __op; }
- _LIBCPP_HIDE_FROM_ABI _Tp operator^=(_Tp __op) volatile _NOEXCEPT { return fetch_xor(__op) ^ __op; }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator|=(_Tp __op) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_or(__op) | __op;
+ }
+
_LIBCPP_HIDE_FROM_ABI _Tp operator^=(_Tp __op) _NOEXCEPT { return fetch_xor(__op) ^ __op; }
+
+ _LIBCPP_HIDE_FROM_ABI _Tp operator^=(_Tp __op) volatile _NOEXCEPT
+ requires __base::is_always_lock_free
+ {
+ return fetch_xor(__op) ^ __op;
+ }
};
// Here we need _IsIntegral because the default template argument is not enough
diff --git a/libcxx/include/__tuple/tuple_element.h b/libcxx/include/__tuple/tuple_element.h
index 9127c47dc8f1a..bcffc540eaf02 100644
--- a/libcxx/include/__tuple/tuple_element.h
+++ b/libcxx/include/__tuple/tuple_element.h
@@ -28,16 +28,6 @@ struct _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, const _Tp> {
typedef _LIBCPP_NODEBUG const typename tuple_element<_Ip, _Tp>::type type;
};
-template <size_t _Ip, class _Tp>
-struct _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, volatile _Tp> {
- typedef _LIBCPP_NODEBUG volatile typename tuple_element<_Ip, _Tp>::type type;
-};
-
-template <size_t _Ip, class _Tp>
-struct _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, const volatile _Tp> {
- typedef _LIBCPP_NODEBUG const volatile typename tuple_element<_Ip, _Tp>::type type;
-};
-
#ifndef _LIBCPP_CXX03_LANG
template <size_t _Ip, class... _Types>
diff --git a/libcxx/include/__tuple/tuple_size.h b/libcxx/include/__tuple/tuple_size.h
index 18a17fd4d5878..8dc6bfdb828d8 100644
--- a/libcxx/include/__tuple/tuple_size.h
+++ b/libcxx/include/__tuple/tuple_size.h
@@ -35,17 +35,6 @@ struct _LIBCPP_TEMPLATE_VIS tuple_size<__enable_if_tuple_size_imp< const _Tp,
integral_constant<size_t, sizeof(tuple_size<_Tp>)>>>
: public integral_constant<size_t, tuple_size<_Tp>::value> {};
-template <class _Tp>
-struct _LIBCPP_TEMPLATE_VIS tuple_size<__enable_if_tuple_size_imp< volatile _Tp,
- __enable_if_t<!is_const<_Tp>::value>,
- integral_constant<size_t, sizeof(tuple_size<_Tp>)>>>
- : public integral_constant<size_t, tuple_size<_Tp>::value> {};
-
-template <class _Tp>
-struct _LIBCPP_TEMPLATE_VIS
-tuple_size<__enable_if_tuple_size_imp<const volatile _Tp, integral_constant<size_t, sizeof(tuple_size<_Tp>)>>>
- : public integral_constant<size_t, tuple_size<_Tp>::value> {};
-
#else
template <class _Tp>
struct _LIBCPP_TEMPLATE_VIS tuple_size<const _Tp> : public tuple_size<_Tp> {};
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 5f2d03b7227b8..94767e482bcca 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -317,12 +317,6 @@ inline constexpr size_t variant_size_v = variant_size<_Tp>::value;
template <class _Tp>
struct _LIBCPP_TEMPLATE_VIS variant_size<const _Tp> : variant_size<_Tp> {};
-template <class _Tp>
-struct _LIBCPP_TEMPLATE_VIS variant_size<volatile _Tp> : variant_size<_Tp> {};
-
-template <class _Tp>
-struct _LIBCPP_TEMPLATE_VIS variant_size<const volatile _Tp> : variant_size<_Tp> {};
-
template <class... _Types>
struct _LIBCPP_TEMPLATE_VIS variant_size<variant<_Types...>> : integral_constant<size_t, sizeof...(_Types)> {};
@@ -335,12 +329,6 @@ using variant_alternative_t = typename variant_alternative<_Ip, _Tp>::type;
template <size_t _Ip, class _Tp>
struct _LIBCPP_TEMPLATE_VIS variant_alternative<_Ip, const _Tp> : add_const<variant_alternative_t<_Ip, _Tp>> {};
-template <size_t _Ip, class _Tp>
-struct _LIBCPP_TEMPLATE_VIS variant_alternative<_Ip, volatile _Tp> : add_volatile<variant_alternative_t<_Ip, _Tp>> {};
-
-template <size_t _Ip, class _Tp>
-struct _LIBCPP_TEMPLATE_VIS variant_alternative<_Ip, const volatile _Tp> : add_cv<variant_alternative_t<_Ip, _Tp>> {};
-
template <size_t _Ip, class... _Types>
struct _LIBCPP_TEMPLATE_VIS variant_alternative<_Ip, variant<_Types...>> {
static_assert(_Ip < sizeof...(_Types), "Index out of bounds in std::variant_alternative<>");
|
You misread the paper. The paper didn't remove anything. It just conditionally deprecated some components. |
Yes + tests? |
You are right... I've reverted and simply deprecated the functions instead. Figuring out how to test for deprecation warnings now :) |
You can also check my reverted PR (reverted because it was too early to commit): #87111 |
|
It seems like adding I can managed to get a warning to emit at the site of the template specializations but that would seemingly be confusing for users, and it requires duplicating some code which just doesn't make sense. For now I am just going to update the synopses to reflect the deprecation stuff. |
In any case, it's probably I think we should continue to add the attribute since it works with GCC. For Clang, I think I've found a partial workaround but it's unknown whether there's a complete one, see #44496 (comment). Note there're currently pitfalls. |
libcxx/include/__atomic/atomic.h
Outdated
| @@ -74,7 +74,7 @@ struct atomic<_Tp*> : public __atomic_base<_Tp*> { | |||
| _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR atomic(_Tp* __d) _NOEXCEPT : __base(__d) {} | |||
|
|
|||
| _LIBCPP_HIDE_FROM_ABI _Tp* operator=(_Tp* __d) volatile _NOEXCEPT | |||
| requires __base::is_always_lock_free | |||
| _LIBCPP_REQUIRE_IS_ALWAYS_LOCK_FREE | |||
There was a problem hiding this comment.
I'm not sure how to test these changes to std::atomic<T*> since pointer type since virtually all systems with atomic instructions will support atomic reads and writes of pointer width.
Perhaps I should constrain this to architectures where this is not the case?
There was a problem hiding this comment.
since virtually all systems with atomic instructions will support atomic reads and writes of pointer width
This is not guaranteed/required by the standard.
I think the approach in microsoft/STL#634 works - define a conditionally deprecated but always true constant and then static_assert it. With this approach you don't need to be worried about whether some atomic types are always lock-free.
There was a problem hiding this comment.
I understand that it is not a guarantee, but I am not sure I an write a test that will run on any ISAs i have access to since ARM and x86 both have instructions to atomically write 16 bytes of data.
Perhaps I can restrict the test to MIPS or some simpler architecture? I'm not familiar enough with the testing infrastructure to know the correct manner to proceed here.
There was a problem hiding this comment.
I think the approach in microsoft/STL#634 works - define a conditionally deprecated but always
trueconstant and thenstatic_assertit. With this approach you don't need to be worried about whether some atomic types are always lock-free.
I am pursuing this right now, as it seems to be the only way to consistently raise deprecated warnings.
I went ahead and borrowed from the Microsoft STL here: microsoft/STL#634. This should create warnings for the deprecated parts of the library, but they won't be direct. I have noticed that that throughout libcxx there are no custom deprecation messages -- is this for compatibility reasons? Custom messages would make things a bit clearer here, but I don't know if it is acceptable here. |
There was a problem hiding this comment.
Thanks a lot for working on this!
I just did a quick review and have some comments. Once they are addressed I'll do a full review.
Can you update the commit message to include the title of the paper. I'm happy with the title being short, but reading the paper title in the message gives more information when looking at the commit in the future.
You still need to update the status page manually. There will be automation in the future, but that is not there yet.
...ibcxx/atomics/atomics.types.operations/atomic_volatile_require_lock_free_in_cxx20.verify.cpp
Outdated
Show resolved
Hide resolved
|
|
||
| template <class _Tp> | ||
| _LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE constexpr bool __deprecated_if_not_awlays_lock_free<_Tp, false> = true; | ||
|
|
There was a problem hiding this comment.
I'm not too fond of this entire code, the static_assert feels very misleading. It is only used to generate a diagnostic. How about something along the lines of:
#if _LIBCPP_STD_VER >= 20
inline constexpr bool __deprecated_if_not_awlays_lock_free = true;
template <class _Tp>
[[deprecated("volatile atomic operations are deprecated when std::atomic<T>::is_always_lock_free is false")]]
inline constexpr bool __deprecated_if_not_awlays_lock_free<_Tp, false> = true;
# define _LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE(_Tp, __is_always_lock_free) \
static_assert(__deprecated_if_not_awlays_lock_free<_Tp, __is_always_lock_free>)
#else
define _LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE(_Tp, __is_always_lock_free) \
do { } while(1)
#endif
Then change the code to
_LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
_LIBCPP_CHECK_STORE_MEMORY_ORDER(__m) {
_LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE(_Tp, is_always_lock_free)
std::__cxx_atomic_store(std::addressof(__a_), __d, __m);
}
It also needs a comment describing why.
There was a problem hiding this comment.
I share your distaste for this use of static assertions, hopefully the issue with attributes on template specializations is eventually fixed (though I'm not 100% sure if its a bug or a feature).
There was a problem hiding this comment.
Regarding a comments - do you mean at every single place the macro is used, or do you mean at the definition of the macro?
There was a problem hiding this comment.
No just comment for the macro itself.
| static_assert(__deprecated_if_not_always_lock_free<_Tp, __is_always_lock_free>) | ||
| #else | ||
| # define _LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE(_Tp, __is_always_lock_free) \ | ||
| {} |
There was a problem hiding this comment.
This needs to be a statement and require a semi-colon.
| {} | |
| do { } while(1) |
There was a problem hiding this comment.
is do { } while(1) a no-op in C++? I thought it was UB.
There was a problem hiding this comment.
I thought it was UB.
It's definitely not now per P2809R3.
However..., shouldn't this be do { } while(0)?
There was a problem hiding this comment.
Alternatively, static_assert(true, "")
|
|
||
| template <class _Tp> | ||
| _LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE constexpr bool __deprecated_if_not_awlays_lock_free<_Tp, false> = true; | ||
|
|
There was a problem hiding this comment.
No just comment for the macro itself.
| _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI constexpr bool __volatile_deprecated_since_cxx20_warning<_Tp, true> = true; | ||
| # define _LIBCPP_VOLATILE_DEPRECATED_WARNING static_assert(__volatile_deprecated_since_cxx20_warning<volatile _Tp>) | ||
| # define _LIBCPP_CONST_VOLATILE_DEPRECATED_WARNING static_assert(__volatile_deprecated_since_cxx20_warning<const volatile _Tp>) | ||
|
|
There was a problem hiding this comment.
Here we can do something similar as _LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE. In that case I don't think we need to differentiate between volatile and const volatile. I even wonder whether we then can make it a bit more generic in naming.
There was a problem hiding this comment.
Hmm I do think differentiating between volatile and const volatile may be confusing.
I'm not sure what a better name would be since there isn't a specific condition to refer to like !is_always_lock_free.
You can test this locally with the following command:git-clang-format --diff cf79aba99db4909437b8977a59c51bc8899ddb9c 015a7d80f63eb19dbee18dc19ee0865f0c7eb803 --extensions ,cpp,h -- libcxx/test/libcxx/atomics/atomics.types.operations/atomic_volatile_require_lock_free_in_cxx20.verify.cpp libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.helper/volatile_tuple_element_deprecated_in_cxx20.verify.cpp libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.helper/volatile_tuple_size_deprecated_in_cxx20.verify.cpp libcxx/test/libcxx/utilities/variant/variant.variant/variant.helper/volatile_deprecated.verify.cpp libcxx/include/__atomic/atomic.h libcxx/include/__atomic/atomic_base.h libcxx/include/__config libcxx/include/__tuple/tuple_element.h libcxx/include/__tuple/tuple_size.h libcxx/include/atomic libcxx/include/tuple libcxx/include/variant libcxx/test/std/containers/sequences/array/array.tuple/tuple_element.pass.cppView the diff from clang-format here.diff --git a/libcxx/include/__atomic/atomic_base.h b/libcxx/include/__atomic/atomic_base.h
index 57f3b8b574..210b02b59a 100644
--- a/libcxx/include/__atomic/atomic_base.h
+++ b/libcxx/include/__atomic/atomic_base.h
@@ -41,10 +41,11 @@ inline constexpr bool __deprecated_if_not_always_lock_free<_Tp, false> = true;
// when atomic<T>::is_always_lock_free is false without having to duplicate
// the method. We could do:
//
-// _LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+// _LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
// requires is_always_lock_free { ... }
//
-// [[deprecated(...)]] _LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
+// [[deprecated(...)]] _LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order __m = memory_order_seq_cst) volatile
+// _NOEXCEPT
// requires !is_always_lock_free { ... }
//
// But this creates a lot of unecessary duplicate code.
diff --git a/libcxx/include/__config b/libcxx/include/__config
index eeb6ea66a6..f7b268c4be 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -744,14 +744,13 @@ typedef __char32_t char32_t;
// P1831R1 deprecated many uses of volatile, but the way attributes work with template specializations require this
// work-around to always raise warnings in cases where templates are specialized for volatile variants of STL types.
# if _LIBCPP_STD_VER >= 20
- template <class _Tp, bool __cxx20 = _LIBCPP_STD_VER >= 20>
- _LIBCPP_HIDE_FROM_ABI constexpr bool __volatile_deprecated_since_cxx20_warning = true;
- template <class _Tp>
- _LIBCPP_DEPRECATED_IN_CXX20
- _LIBCPP_HIDE_FROM_ABI constexpr bool __volatile_deprecated_since_cxx20_warning<_Tp, true> = true;
+template <class _Tp, bool __cxx20 = _LIBCPP_STD_VER >= 20>
+_LIBCPP_HIDE_FROM_ABI constexpr bool __volatile_deprecated_since_cxx20_warning = true;
+template <class _Tp>
+_LIBCPP_DEPRECATED_IN_CXX20
+_LIBCPP_HIDE_FROM_ABI constexpr bool __volatile_deprecated_since_cxx20_warning<_Tp, true> = true;
+# define _LIBCPP_VOLATILE_DEPRECATED_WARNING static_assert(__volatile_deprecated_since_cxx20_warning<volatile _Tp>)
# define _LIBCPP_VOLATILE_DEPRECATED_WARNING static_assert(__volatile_deprecated_since_cxx20_warning<volatile _Tp>)
-# define _LIBCPP_VOLATILE_DEPRECATED_WARNING \
- static_assert(__volatile_deprecated_since_cxx20_warning<volatile _Tp>)
# else
# define _LIBCPP_VOLATILE_DEPRECATED_WARNING static_assert(true)
# endif
diff --git a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.helper/volatile_tuple_element_deprecated_in_cxx20.verify.cpp b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.helper/volatile_tuple_element_deprecated_in_cxx20.verify.cpp
index 2282e4a2c4..3649151535 100644
--- a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.helper/volatile_tuple_element_deprecated_in_cxx20.verify.cpp
+++ b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.helper/volatile_tuple_element_deprecated_in_cxx20.verify.cpp
@@ -26,4 +26,3 @@
// expected-warning@*:* {{'__volatile_deprecated_since_cxx20_warning<volatile std::array<int, 3>>' is deprecated}}
[[maybe_unused]] std::tuple_element<0, const volatile std::array<int, 3>> const_vol_arr_test;
-
|
|
Could you add the title of the paper in the title? |
There was a problem hiding this comment.
Thanks for working on this, it's really appreciated.
I'll give a proper review later on, but for now:
- Please update the commit subject so that it describes what the change is.
- Please add
Closes #100038to your commit message on a separate line so that P1831R1: Deprecating volatile: library #100038 is automatically closed when this PR lands.
jkarns275
changed the title
variant helper methods, and deprecate many volatile atomic methods when that atomic type is not guaranteed to always be lock-free. Closes llvm#100038
Perhaps it should also be added to the PR description, which makes Github automatically associate this PR with the issue. |
|
Some of the CI workflows revealed a few tests that I broke by deprecating, patching those up. |
I had a look at the failing buildkite builds. (For example, https://buildkite.com/llvm-project/libcxx-ci/builds/37023#0191294e-03e1-4523-ad22-074504bf1b23) Can you fix this issue before starting the full CI? |
|
The It seems like using a deprecated method doesn't trigger the warning in the case of a precompiled module - is this expected behavior? |
| inline constexpr bool __deprecated_if_not_always_lock_free<_Tp, false> = true; | ||
|
|
||
| // Many volatile overloads of of atomic<T> methods have a requirement to | ||
| // guarantee atomic<T>::is_always_lock_free is truen in C++20. |
There was a problem hiding this comment.
| // guarantee atomic<T>::is_always_lock_free is truen in C++20. | |
| // guarantee atomic<T>::is_always_lock_free is true in C++20. |
| static_assert(__deprecated_if_not_always_lock_free<_Tp, __is_always_lock_free>) | ||
| #else | ||
| # define _LIBCPP_DEPRECATED_NOT_ALWAYS_LOCK_FREE(_Tp, __is_always_lock_free) \ | ||
| {} |
There was a problem hiding this comment.
Alternatively, static_assert(true, "")
| @@ -47,6 +47,7 @@ struct atomic : public __atomic_base<_Tp> { | |||
| _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR atomic(_Tp __d) _NOEXCEPT : __base(__d) {} | |||
There was a problem hiding this comment.
It seems like using a deprecated method doesn't trigger the warning in the case of a precompiled module - is this expected behavior?
This might be because the warning is emitted inside the body of the function. An alternative could be to try emitting the diagnostic in the declaration by doing something like
_LIBCPP_HIDE_FROM_ABI __deprecated_if_not_lock_free<_Tp*> operator=(_Tp* __d) volatile _NOEXCEPT {
__base::store(__d);
return __d;
}where __deprecated_if_not_lock_free is defined to always expand to its argument as-is, but also trigger the warning if needed.
|
@jkarns275 Gentle ping on this. If you'd like someone to take this over, please let us know! |
|
@ldionne I will take another stab at this over the holiday weekend and if I'm not successful I'll release it :) |
P1831R1 introduces 2 key sets of changes:
tuple_size,tuple_element,variant_size, andvariant_alternative.atomic<T>methods in the event thatatomic<T>::is_always_lock_free == falseRegarding 1, I have added a some static assertions that will emit warnings on the volatile template specializations if std >= 20. This weird work around has to be used rather than a simple deprecation attribute because of the way attributes and template specialization interact (see for an old issue report).
As for 2, a similar approach is used to variably emit the warning for the specified methods if std >= and
atomic<T>::is_always_lock_free == false. It would be possible to do this with an actualrequiresclause as well, but in order for this to be a non-breaking change we would have to introduce a second, deprecated version of the method without the requires clause (this may be preferable, I'm not sure).Closes #100038