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;
     };
 
   // _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

Reply via email to