On Fri, 29 Aug 2025, 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.
> ---
> 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.
>
>
> 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;
I believe this makes _Variadic_union instantiation quadratic with
respect to the number of alternatives. How compile-time/memory
usage fare for the 87619.cc test with this change? (can use
-ftime-report to measure that)
> };
>
> // _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
>
>
