On Sun, Jan 7, 2024 at 3:33 PM Patrick Palka <ppa...@redhat.com> wrote: > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
Ping. > > -- >8 -- > > The recursively defined constraints on _Variadic_union's user-defined > destructor (necessary for maintaining trivial destructibility of the > variant iff all of its alternatives are) effectively require a template > instantiation depth of 3x the number of variants, with the instantiation > stack looking like > > ... > _Variadic_union<B, C, ...> > std::is_trivially_destructible_v<_Variadic_union<B, C, ...>> > _Variadic_union<A, B, C, ...>::~_Variadic_union() > _Variadic_union<A, B, C, ...> > ... > > Ideally the template depth should be ~equal to the number of variants > (plus a constant). Luckily it seems we don't need to compute trivial > destructibility of the alternatives at all from _Variadic_union, since > its only user _Variant_storage already has that information. To that > end this patch removes these recursive constraints and instead passes > this information down from _Variant_storage. After this patch, the > template instantiation depth for 87619.cc is ~270 instead of ~780. > > libstdc++-v3/ChangeLog: > > * include/std/variant (__detail::__variant::_Variadic_union): > Add bool __trivially_destructible template parameter. > (__detail::__variant::_Variadic_union::~_Variadic_union): > Use __trivially_destructible in constraints instead. > (_Variant_storage): Pass __trivially_destructible value to > _Variadic_union. > --- > libstdc++-v3/include/std/variant | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/libstdc++-v3/include/std/variant > b/libstdc++-v3/include/std/variant > index 20a76c8aa87..4b9002e0917 100644 > --- a/libstdc++-v3/include/std/variant > +++ b/libstdc++-v3/include/std/variant > @@ -392,7 +392,7 @@ namespace __variant > }; > > // Defines members and ctors. > - template<typename... _Types> > + template<bool __trivially_destructible, typename... _Types> > union _Variadic_union > { > _Variadic_union() = default; > @@ -401,8 +401,8 @@ namespace __variant > _Variadic_union(in_place_index_t<_Np>, _Args&&...) = delete; > }; > > - template<typename _First, typename... _Rest> > - union _Variadic_union<_First, _Rest...> > + template<bool __trivially_destructible, typename _First, typename... _Rest> > + union _Variadic_union<__trivially_destructible, _First, _Rest...> > { > constexpr _Variadic_union() : _M_rest() { } > > @@ -427,13 +427,12 @@ namespace __variant > ~_Variadic_union() = default; > > constexpr ~_Variadic_union() > - requires (!is_trivially_destructible_v<_First>) > - || (!is_trivially_destructible_v<_Variadic_union<_Rest...>>) > + requires (!__trivially_destructible) > { } > #endif > > _Uninitialized<_First> _M_first; > - _Variadic_union<_Rest...> _M_rest; > + _Variadic_union<__trivially_destructible, _Rest...> _M_rest; > }; > > // _Never_valueless_alt is true for variant alternatives that can > @@ -514,7 +513,7 @@ namespace __variant > return this->_M_index != __index_type(variant_npos); > } > > - _Variadic_union<_Types...> _M_u; > + _Variadic_union<false, _Types...> _M_u; > using __index_type = __select_index<_Types...>; > __index_type _M_index; > }; > @@ -552,7 +551,7 @@ namespace __variant > return this->_M_index != static_cast<__index_type>(variant_npos); > } > > - _Variadic_union<_Types...> _M_u; > + _Variadic_union<true, _Types...> _M_u; > using __index_type = __select_index<_Types...>; > __index_type _M_index; > }; > -- > 2.43.0.254.ga26002b628 >