[libcxx-commits] [libcxx] [libc++] Reorganize the std::variant macros (PR #89419)
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Apr 22 06:56:51 PDT 2024
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/89419
>From 0ef8e2c23c1b5cb42c95930a1e6b1d783607cb9a Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 19 Apr 2024 12:51:31 -0400
Subject: [PATCH 1/3] [libc++] Reorganize the std::variant macros
std::variant uses multiple macros to generate special member functions.
These macros were very subtle to read because of e.g. a macro argument
appearing in the middle of a macro-ized class definition. In conjunction
with clang-format, this could lead to extremely subtle macro expansions
that were not easy to parse for humans.
By adding semi-colons in macro expansions in judicious places, clang-format
does a better job and makes these macros a lot easier to read.
As a drive-by fix, I noticed that several of these functions were missing
HIDE_FROM_ABI annotations, so I added them.
---
libcxx/include/variant | 102 +++++++++++++++++++++++------------------
1 file changed, 57 insertions(+), 45 deletions(-)
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 1b5e84e9547953..8c548c428cc67f 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -657,6 +657,8 @@ private:
} // namespace __visitation
+# define _LIBCPP_EAT_SEMICOLON static_assert(true, "")
+
template <size_t _Index, class _Tp>
struct _LIBCPP_TEMPLATE_VIS __alt {
using __value_type = _Tp;
@@ -691,11 +693,10 @@ union _LIBCPP_TEMPLATE_VIS __union<_DestructibleTrait, _Index> {};
__union(const __union&) = default; \
__union(__union&&) = default; \
\
- destructor \
+ destructor; \
\
- __union& \
- operator=(const __union&) = default; \
- __union& operator=(__union&&) = default; \
+ __union& operator=(const __union&) = default; \
+ __union& operator=(__union&&) = default; \
\
private: \
char __dummy; \
@@ -705,9 +706,10 @@ union _LIBCPP_TEMPLATE_VIS __union<_DestructibleTrait, _Index> {};
friend struct __access::__union; \
}
-_LIBCPP_VARIANT_UNION(_Trait::_TriviallyAvailable, ~__union() = default;);
-_LIBCPP_VARIANT_UNION(_Trait::_Available, ~__union(){});
-_LIBCPP_VARIANT_UNION(_Trait::_Unavailable, ~__union() = delete;);
+_LIBCPP_VARIANT_UNION(_Trait::_TriviallyAvailable, ~__union() = default);
+_LIBCPP_VARIANT_UNION(
+ _Trait::_Available, _LIBCPP_HIDE_FROM_ABI ~__union() {} _LIBCPP_EAT_SEMICOLON);
+_LIBCPP_VARIANT_UNION(_Trait::_Unavailable, ~__union() = delete);
# undef _LIBCPP_VARIANT_UNION
@@ -761,23 +763,27 @@ class _LIBCPP_TEMPLATE_VIS __dtor;
using __base_type::__base_type; \
using __base_type::operator=; \
\
- __dtor(const __dtor&) = default; \
- __dtor(__dtor&&) = default; \
- destructor __dtor& operator=(const __dtor&) = default; \
- __dtor& operator=(__dtor&&) = default; \
+ __dtor(const __dtor&) = default; \
+ __dtor(__dtor&&) = default; \
+ __dtor& operator=(const __dtor&) = default; \
+ __dtor& operator=(__dtor&&) = default; \
+ destructor; \
\
protected: \
- inline _LIBCPP_HIDE_FROM_ABI destroy \
+ inline _LIBCPP_HIDE_FROM_ABI destroy; \
}
_LIBCPP_VARIANT_DESTRUCTOR(
- _Trait::_TriviallyAvailable, ~__dtor() = default;
- , void __destroy() noexcept { this->__index = __variant_npos<__index_t>; });
+ _Trait::_TriviallyAvailable,
+ ~__dtor() = default, //
+ _LIBCPP_HIDE_FROM_ABI void __destroy() noexcept {
+ this->__index = __variant_npos<__index_t>;
+ } _LIBCPP_EAT_SEMICOLON);
_LIBCPP_VARIANT_DESTRUCTOR(
_Trait::_Available,
- ~__dtor() { __destroy(); },
- void __destroy() noexcept {
+ _LIBCPP_HIDE_FROM_ABI ~__dtor() { __destroy(); },
+ _LIBCPP_HIDE_FROM_ABI void __destroy() noexcept {
if (!this->valueless_by_exception()) {
__visitation::__base::__visit_alt(
[](auto& __alt) noexcept {
@@ -787,9 +793,9 @@ _LIBCPP_VARIANT_DESTRUCTOR(
*this);
}
this->__index = __variant_npos<__index_t>;
- });
+ } _LIBCPP_EAT_SEMICOLON);
-_LIBCPP_VARIANT_DESTRUCTOR(_Trait::_Unavailable, ~__dtor() = delete;, void __destroy() noexcept = delete;);
+_LIBCPP_VARIANT_DESTRUCTOR(_Trait::_Unavailable, ~__dtor() = delete, void __destroy() noexcept = delete);
# undef _LIBCPP_VARIANT_DESTRUCTOR
@@ -839,20 +845,24 @@ class _LIBCPP_TEMPLATE_VIS __move_constructor;
using __base_type::operator=; \
\
__move_constructor(const __move_constructor&) = default; \
- move_constructor ~__move_constructor() = default; \
+ ~__move_constructor() = default; \
__move_constructor& operator=(const __move_constructor&) = default; \
__move_constructor& operator=(__move_constructor&&) = default; \
+ move_constructor; \
}
_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(_Trait::_TriviallyAvailable,
- __move_constructor(__move_constructor&& __that) = default;);
+ __move_constructor(__move_constructor&& __that) = default);
_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(
_Trait::_Available,
- __move_constructor(__move_constructor&& __that) noexcept(__all<is_nothrow_move_constructible_v<_Types>...>::value)
- : __move_constructor(__valueless_t{}) { this->__generic_construct(*this, std::move(__that)); });
+ _LIBCPP_HIDE_FROM_ABI __move_constructor(__move_constructor&& __that) noexcept(
+ __all<is_nothrow_move_constructible_v<_Types>...>::value)
+ : __move_constructor(__valueless_t{}) {
+ this->__generic_construct(*this, std::move(__that));
+ } _LIBCPP_EAT_SEMICOLON);
-_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(_Trait::_Unavailable, __move_constructor(__move_constructor&&) = delete;);
+_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(_Trait::_Unavailable, __move_constructor(__move_constructor&&) = delete);
# undef _LIBCPP_VARIANT_MOVE_CONSTRUCTOR
@@ -869,20 +879,21 @@ class _LIBCPP_TEMPLATE_VIS __copy_constructor;
using __base_type::__base_type; \
using __base_type::operator=; \
\
- copy_constructor __copy_constructor(__copy_constructor&&) = default; \
- ~__copy_constructor() = default; \
- __copy_constructor& operator=(const __copy_constructor&) = default; \
- __copy_constructor& operator=(__copy_constructor&&) = default; \
- }
+ __copy_constructor(__copy_constructor&&) = default; \
+ ~__copy_constructor() = default; \
+ __copy_constructor& operator=(const __copy_constructor&) = default; \
+ __copy_constructor& operator=(__copy_constructor&&) = default; \
+ copy_constructor; \
+ } // namespace __variant_detail
_LIBCPP_VARIANT_COPY_CONSTRUCTOR(_Trait::_TriviallyAvailable,
- __copy_constructor(const __copy_constructor& __that) = default;);
+ __copy_constructor(const __copy_constructor& __that) = default);
_LIBCPP_VARIANT_COPY_CONSTRUCTOR(
- _Trait::_Available, __copy_constructor(const __copy_constructor& __that)
- : __copy_constructor(__valueless_t{}) { this->__generic_construct(*this, __that); });
+ _Trait::_Available, _LIBCPP_HIDE_FROM_ABI __copy_constructor(const __copy_constructor& __that)
+ : __copy_constructor(__valueless_t{}) { this->__generic_construct(*this, __that); } _LIBCPP_EAT_SEMICOLON);
-_LIBCPP_VARIANT_COPY_CONSTRUCTOR(_Trait::_Unavailable, __copy_constructor(const __copy_constructor&) = delete;);
+_LIBCPP_VARIANT_COPY_CONSTRUCTOR(_Trait::_Unavailable, __copy_constructor(const __copy_constructor&) = delete);
# undef _LIBCPP_VARIANT_COPY_CONSTRUCTOR
@@ -955,22 +966,22 @@ class _LIBCPP_TEMPLATE_VIS __move_assignment;
__move_assignment(__move_assignment&&) = default; \
~__move_assignment() = default; \
__move_assignment& operator=(const __move_assignment&) = default; \
- move_assignment \
+ move_assignment; \
}
_LIBCPP_VARIANT_MOVE_ASSIGNMENT(_Trait::_TriviallyAvailable,
- __move_assignment& operator=(__move_assignment&& __that) = default;);
+ __move_assignment& operator=(__move_assignment&& __that) = default);
_LIBCPP_VARIANT_MOVE_ASSIGNMENT(
_Trait::_Available,
- __move_assignment&
+ _LIBCPP_HIDE_FROM_ABI __move_assignment&
operator=(__move_assignment&& __that) noexcept(
__all<(is_nothrow_move_constructible_v<_Types> && is_nothrow_move_assignable_v<_Types>)...>::value) {
this->__generic_assign(std::move(__that));
return *this;
- });
+ } _LIBCPP_EAT_SEMICOLON);
-_LIBCPP_VARIANT_MOVE_ASSIGNMENT(_Trait::_Unavailable, __move_assignment& operator=(__move_assignment&&) = delete;);
+_LIBCPP_VARIANT_MOVE_ASSIGNMENT(_Trait::_Unavailable, __move_assignment& operator=(__move_assignment&&) = delete);
# undef _LIBCPP_VARIANT_MOVE_ASSIGNMENT
@@ -987,22 +998,23 @@ class _LIBCPP_TEMPLATE_VIS __copy_assignment;
using __base_type::__base_type; \
using __base_type::operator=; \
\
- __copy_assignment(const __copy_assignment&) = default; \
- __copy_assignment(__copy_assignment&&) = default; \
- ~__copy_assignment() = default; \
- copy_assignment __copy_assignment& operator=(__copy_assignment&&) = default; \
+ __copy_assignment(const __copy_assignment&) = default; \
+ __copy_assignment(__copy_assignment&&) = default; \
+ ~__copy_assignment() = default; \
+ __copy_assignment& operator=(__copy_assignment&&) = default; \
+ copy_assignment; \
}
_LIBCPP_VARIANT_COPY_ASSIGNMENT(_Trait::_TriviallyAvailable,
- __copy_assignment& operator=(const __copy_assignment& __that) = default;);
+ __copy_assignment& operator=(const __copy_assignment& __that) = default);
_LIBCPP_VARIANT_COPY_ASSIGNMENT(
- _Trait::_Available, __copy_assignment& operator=(const __copy_assignment& __that) {
+ _Trait::_Available, _LIBCPP_HIDE_FROM_ABI __copy_assignment& operator=(const __copy_assignment& __that) {
this->__generic_assign(__that);
return *this;
- });
+ } _LIBCPP_EAT_SEMICOLON);
-_LIBCPP_VARIANT_COPY_ASSIGNMENT(_Trait::_Unavailable, __copy_assignment& operator=(const __copy_assignment&) = delete;);
+_LIBCPP_VARIANT_COPY_ASSIGNMENT(_Trait::_Unavailable, __copy_assignment& operator=(const __copy_assignment&) = delete);
# undef _LIBCPP_VARIANT_COPY_ASSIGNMENT
>From 8ef5dda11bdc68ec00be88599d3adb40a0dee8c8 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 19 Apr 2024 13:23:13 -0400
Subject: [PATCH 2/3] Eat semi-colon
---
libcxx/include/variant | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 8c548c428cc67f..2dd4ff13dc6f8e 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -782,7 +782,7 @@ _LIBCPP_VARIANT_DESTRUCTOR(
_LIBCPP_VARIANT_DESTRUCTOR(
_Trait::_Available,
- _LIBCPP_HIDE_FROM_ABI ~__dtor() { __destroy(); },
+ _LIBCPP_HIDE_FROM_ABI ~__dtor() { __destroy(); } _LIBCPP_EAT_SEMICOLON,
_LIBCPP_HIDE_FROM_ABI void __destroy() noexcept {
if (!this->valueless_by_exception()) {
__visitation::__base::__visit_alt(
>From 063adfbd15f163661f3b7ccf307b6af2400634f8 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 22 Apr 2024 08:49:43 -0400
Subject: [PATCH 3/3] Document EAT_SEMICOLON macro
---
libcxx/include/variant | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 2dd4ff13dc6f8e..d8dfbcba486b02 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -657,6 +657,8 @@ private:
} // namespace __visitation
+// Adding semi-colons in macro expansions helps clang-format to do a better job.
+// This macro is used to avoid compilation errors due to "stray" semi-colons.
# define _LIBCPP_EAT_SEMICOLON static_assert(true, "")
template <size_t _Index, class _Tp>
More information about the libcxx-commits
mailing list