Template depth in 267 in both cases. On Fri, Aug 29, 2025 at 5:21 PM Tomasz Kaminski <[email protected]> wrote:
> > > On Fri, Aug 29, 2025 at 5:16 PM Patrick Palka <[email protected]> wrote: > >> 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) >> > I have adjusted the test to have a non-trivial destructor, so we hit this > specialization. > Results after: > Time variable wall GGC > phase setup : 0.00 ( 0%) 2015k ( 1%) > phase parsing : 0.06 ( 1%) 14M ( 6%) > phase lang. deferred : 0.56 ( 12%) 152M ( 65%) > phase opt and generate : 4.20 ( 87%) 66M ( 28%) > |name lookup : 0.01 ( 0%) 1884k ( 1%) > |overload resolution : 0.10 ( 2%) 25M ( 11%) > garbage collection : 0.11 ( 2%) 0 ( 0%) > callgraph construction : 0.12 ( 2%) 1377k ( 1%) > callgraph ipa passes : 0.04 ( 1%) 51k ( 0%) > preprocessing : 0.01 ( 0%) 1044k ( 0%) > parser (global) : 0.02 ( 0%) 10M ( 4%) > parser struct body : 0.02 ( 0%) 4272k ( 2%) > parser inl. meth. body : 0.01 ( 0%) 1392k ( 1%) > template instantiation : 0.40 ( 8%) 134M ( 57%) > constant expression evaluation : 0.05 ( 1%) 41k ( 0%) > symout : 4.08 ( 85%) 80M ( 34%) > TOTAL : 4.82 235M > > Results if I use hardcoded false instead of above && condition: > Time variable wall GGC > phase setup : 0.00 ( 0%) 2015k ( 1%) > phase parsing : 0.06 ( 1%) 14M ( 7%) > phase lang. deferred : 0.42 ( 9%) 116M ( 58%) > phase opt and generate : 4.21 ( 90%) 66M ( 33%) > |name lookup : 0.01 ( 0%) 1884k ( 1%) > |overload resolution : 0.02 ( 0%) 3277k ( 2%) > garbage collection : 0.10 ( 2%) 0 ( 0%) > callgraph construction : 0.12 ( 2%) 1377k ( 1%) > callgraph ipa passes : 0.03 ( 1%) 51k ( 0%) > preprocessing : 0.01 ( 0%) 1044k ( 1%) > parser (global) : 0.02 ( 0%) 10M ( 5%) > parser struct body : 0.01 ( 0%) 4272k ( 2%) > parser inl. meth. body : 0.01 ( 0%) 1392k ( 1%) > template instantiation : 0.30 ( 6%) 98M ( 49%) > constant expression evaluation : 0.01 ( 0%) 41k ( 0%) > symout : 4.10 ( 87%) 80M ( 40%) > TOTAL : 4.69 199M > > > > >> >> > }; >> > >> > // _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 >> > >> > > >
