[RFC] libstdc++: Refactor _Variadic_union so _Unitialized<T, false> is not needed.

[Jonathan Wakely]

On Fri, 29 Aug 2025 at 13:33 +0200, Tomasz Kamiński wrote:
>The changes the _Variadic_union implementation, in a way that the
> _Unitialized<T, false> partial specialization for non-trivial types is not
>necessary.
>
>This is simply done by separating the specialization for __trivially_destructible
>being true and false, and for the later definining an empty destructor (similary
> as it was done using concepts).
>
>We also reduce the number of specialization of _Varidic_union, so specialization
>(int, int) is reused by (string, int, int) and (int,int). This is done by computing
>initializating __trivially_destructible with conjuction of is_trivially_destructible_v
>for remaining components. This is only necessary for non-trivial (false) specialization,
>as if both _First and _Rest... are trivally destructible, then _Rest must also be.
>
>This also add test for PR112591.

OK for trunk.

>---
>As in title, this is refernce for comments, espcially if we want to land this change,
>or part of it to the trunk regardless of other changes. I think the test is valuable for sure,
>but the separate specialization, and removal of _Variadic_union<false, int, int>
>specialization seem usefull.
>
>Locally I have removed the _Unitialized<T, false> specialization, and the objects
>no longer alias in C++17 mode (112591.cc works the same in C++20/17), and all
>test have passed locally, so this seem correct. We could even remove the _Unitialized
>altogether and just store _First direclty. This is however ABI break, as it does
>affect layout of classes similar to one in the example.


We can't change the ABI for C++17 now.

But we also need the ABI for C++17 and C++20 to match ... which
suggests that we need to change C++20 to have the same bad properties
as C++17 :-(


> libstdc++-v3/include/std/variant              | 34 ++++++++++++++-----
> .../testsuite/20_util/variant/112591.cc       | 32 +++++++++++++++++
> 2 files changed, 57 insertions(+), 9 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/20_util/variant/112591.cc
>
>diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
>index 2f44f970028..f2f55837461 100644
>--- a/libstdc++-v3/include/std/variant
>+++ b/libstdc++-v3/include/std/variant
>@@ -393,8 +393,29 @@ namespace __variant
> 	_Variadic_union(in_place_index_t<_Np>, _Args&&...) = delete;
>     };
>
>-  template<bool __trivially_destructible, typename _First, typename... _Rest>
>-    union _Variadic_union<__trivially_destructible, _First, _Rest...>
>+  template<typename _First, typename... _Rest>
>+    union _Variadic_union<true, _First, _Rest...>
>+    {
>+      constexpr _Variadic_union() : _M_rest() { }
>+
>+      template<typename... _Args>
>+	constexpr
>+	_Variadic_union(in_place_index_t<0>, _Args&&... __args)
>+	: _M_first(in_place_index<0>, std::forward<_Args>(__args)...)
>+	{ }
>+
>+      template<size_t _Np, typename... _Args>
>+	constexpr
>+	_Variadic_union(in_place_index_t<_Np>, _Args&&... __args)
>+	: _M_rest(in_place_index<_Np-1>, std::forward<_Args>(__args)...)
>+	{ }
>+
>+      _Uninitialized<_First> _M_first;
>+      _Variadic_union<true, _Rest...> _M_rest;
>+    };
>+
>+  template<typename _First, typename... _Rest>
>+    union _Variadic_union<false, _First, _Rest...>
>     {
>       constexpr _Variadic_union() : _M_rest() { }
>
>@@ -410,24 +431,19 @@ namespace __variant
> 	: _M_rest(in_place_index<_Np-1>, std::forward<_Args>(__args)...)
> 	{ }
>
>-#if __cpp_lib_variant >= 202106L
>       _Variadic_union(const _Variadic_union&) = default;
>       _Variadic_union(_Variadic_union&&) = default;
>       _Variadic_union& operator=(const _Variadic_union&) = default;
>       _Variadic_union& operator=(_Variadic_union&&) = default;
>
>-      ~_Variadic_union() = default;
>-
>       // If any alternative type is not trivially destructible then we need a
>       // user-provided destructor that does nothing. The active alternative
>       // will be destroyed by _Variant_storage::_M_reset() instead of here.
>-      constexpr ~_Variadic_union()
>-	requires (!__trivially_destructible)
>+      _GLIBCXX20_CONSTEXPR ~_Variadic_union()
>       { }
>-#endif
>
>       _Uninitialized<_First> _M_first;
>-      _Variadic_union<__trivially_destructible, _Rest...> _M_rest;
>+      _Variadic_union<(is_trivially_destructible_v<_Rest> && ...), _Rest...> _M_rest;
>     };
>
>   // _Never_valueless_alt is true for variant alternatives that can
>diff --git a/libstdc++-v3/testsuite/20_util/variant/112591.cc b/libstdc++-v3/testsuite/20_util/variant/112591.cc
>new file mode 100644
>index 00000000000..b1b07c4f46e
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/variant/112591.cc
>@@ -0,0 +1,32 @@
>+// { dg-do run { target c++17 } }
>+
>+#include <variant>
>+#include <testsuite_hooks.h>
>+
>+struct NonEmpty { int x; };
>+struct TrivialEmpty {};
>+struct NonTrivialEmpty { ~NonTrivialEmpty() {} };
>+
>+template<typename T>
>+struct Compose : T
>+{
>+  std::variant<T, int> v;
>+};
>+
>+template<typename T>
>+bool testAlias()
>+{
>+  Compose<T> c;
>+  return static_cast<T*>(&c) == &std::get<T>(c.v);
>+}
>+
>+int main()
>+{
>+  VERIFY( !testAlias<NonEmpty>() );
>+  VERIFY( !testAlias<TrivialEmpty>() );
>+#if __cplusplus >= 202002L
>+  VERIFY( !testAlias<NonTrivialEmpty>() );
>+#else
>+  VERIFY( testAlias<NonTrivialEmpty>() );
>+#endif
>+}
>-- 
>2.50.1
>
>