On Sat, Dec 6, 2025 at 1:33 PM Jonathan Wakely <[email protected]> wrote:
> On Sat, 06 Dec 2025 at 12:30 +0000, Jonathan Wakely wrote:
> >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 :-(
>
This has a more far fetching consequence, to replicate the bad properties of
the C++17 ABI, the compiler cannot know that the object of the same type
as base class can be stored inside a variant. The simplest way to achieve
that
is to store the object inside a char buffer (make it nested within).
However,
to make variant usable at compile time (constexpr) we need to be able to
placement new a member of an union, and for that the compiler needs to know
that we store an object of type (T), which is in direct conflict with the
above.
So as consequence of the C++17 code, we are currently not conforming,
and the global variable v is initialized dynamically in following example
struct X {
constexpr X() {}
constexpr X(X&&) {}
~X() {}
};
std::variant<X> v{std::in_place_index<0>};
You can see that __static_initialization_and_destruction_0() in C++17 mode
contains
the dynamic initialization of v. It seem like these constructors where made
constexpr
since C++17, as
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2231r1.html
updated only destructor.
So we have the following option:
A) Change C++20 ABI to match C++17. This will make us non-conforming,
to the standard, and be regression for C++20 since GCC-12. We will not
support
constexpr usage for non-trivial object, until we get ability to do
placement new
into char buffer, with support as extension.
B) Keep C++17/C++20 ABI different forever. Remain non-coforming for C++17
mode with above example.
C) Change C++17 ABI in GCC-16.
The option A does not seem to be acceptable:
1) It causes API break for existing programs in C++20 since GCC-12. What,
I mean by API break, is that it will turn static initialization into
dynamic
initialization, for examples like mine above.
2) This affects all variant with non-trivial types, options B,C are limited
only to cases when you have an object of the same type as both base,
and stored as an alternative, inside the variant member.
Now between options B,C. We are choosing between:
1) making a single break of C++17 ABI in GCC-16 (C) versus keeping
the C++17 and C++20 ABI incompatible forever
2) keeping C++17 mode non-conforming forever.
>
> Or maybe we just fix it and rely on the fact that the C++17 behaviour
> is not helpful to anybody, and is probably quite rare (you have to
> have a std::variant member containing the same type as an empty base
> class) and that the only people affected by that probably don't want
> that behaviour.
>
Given that C++17 behavior is not helpful, but also makes us non-conforming
(in a way that affects any non-trivial type), I belive that changing C++17
ABI would be
the right choice here. But I do not have much experience with how
harmful such
breaks are.
>
> >
> >>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
> >>
> >>
>
>
