diff mbox series

[committed] libstdc++: Allow visiting inherited variants [PR 90943]

Message ID YVdko00+wrQ1d3TL@redhat.com
State Committed
Commit c46ecb0112e91c80ee111439e79a58a953e4479d
Headers
Series [committed] libstdc++: Allow visiting inherited variants [PR 90943] |

Commit Message

Jonathan Wakely Oct. 1, 2021, 7:42 p.m. UTC 
  Implement the changes from P2162R2 (as a DR for C++17).

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

libstdc++-v3/ChangeLog:

	PR libstdc++/90943
	* include/std/variant (__cpp_lib_variant): Update value.
	(__detail::__variant::__as): New helpers implementing the
	as-variant exposition-only function templates.
	(visit, visit<R>): Use __as to upcast the variant parameters.
	* include/std/version (__cpp_lib_variant): Update value.
	* testsuite/20_util/variant/visit_inherited.cc: New test.

Tested powerpc64le-linux. Committed to trunk.
commit c46ecb0112e91c80ee111439e79a58a953e4479d
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Apr 19 14:49:12 2021

    libstdc++: Allow visiting inherited variants [PR 90943]
    
    Implement the changes from P2162R2 (as a DR for C++17).
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/90943
            * include/std/variant (__cpp_lib_variant): Update value.
            (__detail::__variant::__as): New helpers implementing the
            as-variant exposition-only function templates.
            (visit, visit<R>): Use __as to upcast the variant parameters.
            * include/std/version (__cpp_lib_variant): Update value.
            * testsuite/20_util/variant/visit_inherited.cc: New test.

Comments

Daniel Krügler Oct. 2, 2021, 12:49 p.m. UTC | #1 
Am Fr., 1. Okt. 2021 um 21:57 Uhr schrieb Jonathan Wakely via
Libstdc++ <libstdc++@gcc.gnu.org>:
>
> Implement the changes from P2162R2 (as a DR for C++17).
>
> Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/90943
>         * include/std/variant (__cpp_lib_variant): Update value.
>         (__detail::__variant::__as): New helpers implementing the
>         as-variant exposition-only function templates.
>         (visit, visit<R>): Use __as to upcast the variant parameters.
>         * include/std/version (__cpp_lib_variant): Update value.
>         * testsuite/20_util/variant/visit_inherited.cc: New test.
>
> Tested powerpc64le-linux. Committed to trunk.
>

I'm wondering why the first __as overload is not noexcept as well (or
asking it the other way around: Why different exception-specifications
are used for the different overloads):

+  // The __as function templates implement the exposition-only "as-variant"
+
+  template<typename... _Types>
+    constexpr std::variant<_Types...>&
+    __as(std::variant<_Types...>& __v)
+    { return __v; }

- Daniel
Jonathan Wakely Oct. 3, 2021, 9:07 a.m. UTC | #2 
On Sat, 2 Oct 2021, 13:50 Daniel Krügler via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

> Am Fr., 1. Okt. 2021 um 21:57 Uhr schrieb Jonathan Wakely via
> Libstdc++ <libstdc++@gcc.gnu.org>:
> >
> > Implement the changes from P2162R2 (as a DR for C++17).
> >
> > Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
> >
> > libstdc++-v3/ChangeLog:
> >
> >         PR libstdc++/90943
> >         * include/std/variant (__cpp_lib_variant): Update value.
> >         (__detail::__variant::__as): New helpers implementing the
> >         as-variant exposition-only function templates.
> >         (visit, visit<R>): Use __as to upcast the variant parameters.
> >         * include/std/version (__cpp_lib_variant): Update value.
> >         * testsuite/20_util/variant/visit_inherited.cc: New test.
> >
> > Tested powerpc64le-linux. Committed to trunk.
> >
>
> I'm wondering why the first __as overload is not noexcept as well (or
> asking it the other way around: Why different exception-specifications
> are used for the different overloads):
>
> +  // The __as function templates implement the exposition-only
> "as-variant"
> +
> +  template<typename... _Types>
> +    constexpr std::variant<_Types...>&
> +    __as(std::variant<_Types...>& __v)
> +    { return __v; }
>

Probably just an oversight, I'll check again and fix it. Thanks!
Jonathan Wakely Oct. 4, 2021, 2:26 p.m. UTC | #3 
On Sun, 3 Oct 2021 at 10:07, Jonathan Wakely wrote:
>
>
>
> On Sat, 2 Oct 2021, 13:50 Daniel Krügler via Libstdc++, <libstdc++@gcc.gnu.org> wrote:
>>
>> Am Fr., 1. Okt. 2021 um 21:57 Uhr schrieb Jonathan Wakely via
>> Libstdc++ <libstdc++@gcc.gnu.org>:
>> >
>> > Implement the changes from P2162R2 (as a DR for C++17).
>> >
>> > Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>> >
>> > libstdc++-v3/ChangeLog:
>> >
>> >         PR libstdc++/90943
>> >         * include/std/variant (__cpp_lib_variant): Update value.
>> >         (__detail::__variant::__as): New helpers implementing the
>> >         as-variant exposition-only function templates.
>> >         (visit, visit<R>): Use __as to upcast the variant parameters.
>> >         * include/std/version (__cpp_lib_variant): Update value.
>> >         * testsuite/20_util/variant/visit_inherited.cc: New test.
>> >
>> > Tested powerpc64le-linux. Committed to trunk.
>> >
>>
>> I'm wondering why the first __as overload is not noexcept as well (or
>> asking it the other way around: Why different exception-specifications
>> are used for the different overloads):
>>
>> +  // The __as function templates implement the exposition-only "as-variant"
>> +
>> +  template<typename... _Types>
>> +    constexpr std::variant<_Types...>&
>> +    __as(std::variant<_Types...>& __v)
>> +    { return __v; }
>
>
> Probably just an oversight, I'll check again and fix it. Thanks!

Fixed by the attached patch, thanks again.

Tested powerpc64le-linux, pushed to trunk.
commit 728e639d82099035fdfe69b716a54717ae7050e0
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Oct 4 10:21:58 2021

    libstdc++: Add missing noexcept to std::variant helper
    
    libstdc++-v3/ChangeLog:
    
            * include/std/variant (__detail::__variant::__as): Add missing
            noexcept to first overload.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index d50c6b7de1d..6377b6731ea 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -206,7 +206,7 @@ namespace __variant
 
   template<typename... _Types>
     constexpr std::variant<_Types...>&
-    __as(std::variant<_Types...>& __v)
+    __as(std::variant<_Types...>& __v) noexcept
     { return __v; }
 
   template<typename... _Types>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 6383cf4e502..c651326ead9 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -71,7 +71,7 @@  namespace __variant
 } // namespace __variant
 } // namespace __detail
 
-#define __cpp_lib_variant 201606L
+#define __cpp_lib_variant 202102L
 
   template<typename... _Types> class tuple;
   template<typename... _Types> class variant;
@@ -202,6 +202,28 @@  namespace __variant
 	  std::forward<_Variants>(__variants)...);
     }
 
+  // The __as function templates implement the exposition-only "as-variant"
+
+  template<typename... _Types>
+    constexpr std::variant<_Types...>&
+    __as(std::variant<_Types...>& __v)
+    { return __v; }
+
+  template<typename... _Types>
+    constexpr const std::variant<_Types...>&
+    __as(const std::variant<_Types...>& __v) noexcept
+    { return __v; }
+
+  template<typename... _Types>
+    constexpr std::variant<_Types...>&&
+    __as(std::variant<_Types...>&& __v) noexcept
+    { return std::move(__v); }
+
+  template<typename... _Types>
+    constexpr const std::variant<_Types...>&&
+    __as(const std::variant<_Types...>&& __v) noexcept
+    { return std::move(__v); }
+
   // _Uninitialized<T> is guaranteed to be a trivially destructible type,
   // even if T is not.
   template<typename _Type, bool = std::is_trivially_destructible_v<_Type>>
@@ -1063,8 +1085,12 @@  namespace __variant
 			      std::index_sequence<__indices...>>
     : _Base_dedup<__indices, __poison_hash<remove_const_t<_Types>>>... { };
 
-  template<size_t _Np, typename _Variant>
-    using __get_t = decltype(std::get<_Np>(std::declval<_Variant>()));
+  // Equivalent to decltype(get<_Np>(as-variant(declval<_Variant>())))
+  template<size_t _Np, typename _Variant,
+      typename _AsV = decltype(__variant::__as(std::declval<_Variant>())),
+      typename _Tp = variant_alternative_t<_Np, remove_reference_t<_AsV>>>
+    using __get_t
+      = conditional_t<is_lvalue_reference_v<_Variant>, _Tp&, _Tp&&>;
 
   // Return type of std::visit.
   template<typename _Visitor, typename... _Variants>
@@ -1741,7 +1767,9 @@  namespace __variant
     constexpr __detail::__variant::__visit_result_t<_Visitor, _Variants...>
     visit(_Visitor&& __visitor, _Variants&&... __variants)
     {
-      if ((__variants.valueless_by_exception() || ...))
+      namespace __variant = std::__detail::__variant;
+
+      if ((__variant::__as(__variants).valueless_by_exception() || ...))
 	__throw_bad_variant_access("std::visit: variant is valueless");
 
       using _Result_type
@@ -1751,10 +1779,11 @@  namespace __variant
 
       if constexpr (sizeof...(_Variants) == 1)
 	{
+	  using _Vp = decltype(__variant::__as(std::declval<_Variants>()...));
+
 	  constexpr bool __visit_rettypes_match = __detail::__variant::
-	    __check_visitor_results<_Visitor, _Variants...>(
-	      std::make_index_sequence<
-	        std::variant_size<remove_reference_t<_Variants>...>::value>());
+	    __check_visitor_results<_Visitor, _Vp>(
+	      make_index_sequence<variant_size_v<remove_reference_t<_Vp>>>());
 	  if constexpr (!__visit_rettypes_match)
 	    {
 	      static_assert(__visit_rettypes_match,
@@ -1765,12 +1794,12 @@  namespace __variant
 	  else
 	    return std::__do_visit<_Tag>(
 	      std::forward<_Visitor>(__visitor),
-	      std::forward<_Variants>(__variants)...);
+	      static_cast<_Vp>(__variants)...);
 	}
       else
 	return std::__do_visit<_Tag>(
 	  std::forward<_Visitor>(__visitor),
-	  std::forward<_Variants>(__variants)...);
+	  __variant::__as(std::forward<_Variants>(__variants))...);
     }
 
 #if __cplusplus > 201703L
@@ -1778,11 +1807,13 @@  namespace __variant
     constexpr _Res
     visit(_Visitor&& __visitor, _Variants&&... __variants)
     {
-      if ((__variants.valueless_by_exception() || ...))
+      namespace __variant = std::__detail::__variant;
+
+      if ((__variant::__as(__variants).valueless_by_exception() || ...))
 	__throw_bad_variant_access("std::visit<R>: variant is valueless");
 
       return std::__do_visit<_Res>(std::forward<_Visitor>(__visitor),
-				   std::forward<_Variants>(__variants)...);
+	  __variant::__as(std::forward<_Variants>(__variants))...);
     }
 #endif
 
diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index f41004b5911..42f7dfa62ad 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -171,7 +171,7 @@ 
 # define __cpp_lib_to_chars 201611L
 #endif
 #define __cpp_lib_unordered_map_try_emplace 201411
-#define __cpp_lib_variant 201606L
+#define __cpp_lib_variant 202102L
 #endif
 
 #if __cplusplus >= 202002L
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_inherited.cc b/libstdc++-v3/testsuite/20_util/variant/visit_inherited.cc
new file mode 100644
index 00000000000..ade83096475
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_inherited.cc
@@ -0,0 +1,36 @@ 
+// { dg-do compile { target c++17 } }
+
+#include <variant>
+
+// P2062R2 Inheriting from std::variant (resolving LWG 3052)
+
+#if __cpp_lib_variant < 202102L
+#error __cpp_lib_variant has the wrong value in <variant>
+#endif
+
+struct V : std::variant<int> {
+    using std::variant<int>::variant;
+};
+
+constexpr int
+test01()
+{
+  V v = 42;
+  return std::visit([](int&){ return 17; }, v);
+}
+static_assert( test01() == 17 );
+
+constexpr int
+test02()
+{
+  const V c = 77;
+  std::variant<char*, long> x = 88L;
+  return std::visit([](auto&& a, auto&& b) {
+    if constexpr (std::is_same_v<decltype(a), const int&&>)
+      if constexpr (std::is_same_v<decltype(b), long&&>)
+	return 99;
+    return 0;
+  },
+  std::move(c), std::move(x));
+}
+static_assert( test02() == 99 );