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 :-(
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.
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
