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

[Jonathan Wakely]

On 29/09/20 19:35 +0300, Ville Voutilainen via Libstdc++ wrote:
>On Tue, 29 Sep 2020 at 14:20, Jonathan Wakely <jwakely@redhat.com> wrote:
>> I think this is what we want:
>>
>>    template<typename _Tp, typename... _Types>
>>      constexpr inline __same_types = (is_same_v<_Tp, _Types> && ...);
>>
>> is_same_v is very cheap, it uses the built-in directly, so you don't
>> need to instantiate any class templates at all.
>>
>> >+
>> >+  template <unsigned long int _Idx, class _Visitor, class _Variant>
>>
>> typename not class please.
>>
>> >+    decltype(auto) __check_visitor_result(_Visitor&& __vis,
>>
>> New line after the decltype(auto) please, not in the middle of the
>> parameter list.
>
>Aye.

>diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
>index dd8847cf829..6f647d622c4 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,22 @@ namespace __variant
> 
>       static constexpr auto
>       _S_apply()
>-      { return _Array_type{&__visit_invoke}; }
>+      {
>+	constexpr bool __visit_ret_type_mismatch =
>+	  _Array_type::__result_is_deduced::value
>+	  && !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};
>+      }
>     };
> 
>   template<typename _Result_type, typename _Visitor, typename... _Variants>
>@@ -1692,6 +1707,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::forward<_Visitor>(__vis)(
>+        std::get<_Idx>(std::forward<_Variant>(__variant)));

Looks good, the new error is nice.

git apply warns about some whitespace errors:

/dev/shm/pr95904.diff:51: indent with spaces.
         std::get<_Idx>(std::forward<_Variant>(__variant)));
/dev/shm/pr95904.diff:73: indent with spaces.
         {
/dev/shm/pr95904.diff:77: indent with spaces.
                 std::variant_size<remove_reference_t<_Variants>...>::value>());
/dev/shm/pr95904.diff:92: indent with spaces.
           std::forward<_Visitor>(__visitor),
warning: 4 lines add whitespace errors.

OK for trunk with those leading spaces switched to tab.

Thanks!