Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]

[Jonathan Wakely <jwakely@redhat.com>]

On 05/10/20 22:35 +0300, Ville Voutilainen via Libstdc++ wrote:
>On Mon, 5 Oct 2020 at 01:15, Ville Voutilainen
><ville.voutilainen@gmail.com> wrote:
>> The patch is borked, doesn't pass tests, fixing...
>
>Unborked, ok for trunk if full testsuite passes?
>
>2020-10-05  Ville Voutilainen  <ville.voutilainen@gmail.com>
>
>    PR libstdc++/95904
>    * include/std/variant (__deduce_visit_result): Add a nested ::type.
>    (__gen_vtable_impl</*base case*/>::_S_apply):
>    Check the visitor return type.
>    (__same_types): New.
>    (__check_visitor_result): Likewise.
>    (__check_visitor_results): Likewise.
>    (visit(_Visitor&&, _Variants&&...)): Use __check_visitor_results
>    in case we're visiting just one variant.
>    * testsuite/20_util/variant/visit_neg.cc: Adjust.

>diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
>index dd8847cf829..b32e564fd41 100644
>--- a/libstdc++-v3/include/std/variant
>+++ b/libstdc++-v3/include/std/variant
>@@ -182,7 +182,7 @@ namespace __variant
>   // used for raw visitation with indices passed in
>   struct __variant_idx_cookie { using type = __variant_idx_cookie; };
>   // Used to enable deduction (and same-type checking) for std::visit:
>-  template<typename> struct __deduce_visit_result { };
>+  template<typename _Tp> struct __deduce_visit_result { using type = _Tp; };
> 
>   // Visit variants that might be valueless.
>   template<typename _Visitor, typename... _Variants>
>@@ -1017,7 +1017,26 @@ namespace __variant
> 
>       static constexpr auto
>       _S_apply()
>-      { return _Array_type{&__visit_invoke}; }
>+      {
>+	if constexpr (_Array_type::__result_is_deduced::value)
>+	  {
>+	    constexpr bool __visit_ret_type_mismatch =
>+	      !is_same_v<typename _Result_type::type,
>+			 decltype(__visit_invoke(std::declval<_Visitor>(),
>+				    std::declval<_Variants>()...))>;
>+	    if constexpr (__visit_ret_type_mismatch)
>+	      {
>+		static_assert(!__visit_ret_type_mismatch,
>+		  "std::visit requires the visitor to have the same "
>+		  "return type for all alternatives of a variant");
>+		return __nonesuch{};
>+	      }
>+	    else
>+	      return _Array_type{&__visit_invoke};
>+	  }
>+	else
>+	  return _Array_type{&__visit_invoke};
>+      }
>     };
> 
>   template<typename _Result_type, typename _Visitor, typename... _Variants>
>@@ -1692,6 +1711,26 @@ namespace __variant
> 			   std::forward<_Variants>(__variants)...);
>     }
> 
>+  template<typename _Tp, typename... _Types>
>+     constexpr inline bool __same_types = (is_same_v<_Tp, _Types> && ...);
>+
>+  template <unsigned long int _Idx, typename _Visitor, typename _Variant>
>+    decltype(auto)
>+    __check_visitor_result(_Visitor&& __vis, _Variant&& __variant)
>+    {
>+      return std::__invoke(std::forward<_Visitor>(__vis),
>+			   std::get<_Idx>(std::forward<_Variant>(__variant)));
>+    }
>+
>+  template <typename _Visitor, typename _Variant, unsigned long int... _Idxs>

index_sequence uses size_t not unsigned long. This parameter pack
needs to be size_t... _Idxs, and the NTTP for __check_visitor_result
should be size_t _Idx.