public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Diagnose visitors with different return types for std::visit [PR95904]
@ 2020-09-28 22:12 Ville Voutilainen
  2020-09-29 11:20 ` Jonathan Wakely
  0 siblings, 1 reply; 11+ messages in thread
From: Ville Voutilainen @ 2020-09-28 22:12 UTC (permalink / raw)
  To: gcc-patches List, libstdc++

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

Not completely tested yet. This does fix the problem of converting
incompatible pointer-to-function types, and thus gets rid of the suggestion
that compiling the code with -fpermissive is a possibility. There
is a special-casing for visit() for visitation of a single variant, and there
we don't even instantiate the whole table mechanism. We should really
entertain the possibility of flattening the whole visitation table; then
this check could (at least in theory) be uniformly written as just
an iteration of all table elements, which is not so convenient to do
with the nested multitable. This seems like a worthy incremental
improvement to me.

2020-09-29  Ville Voutilainen  <ville.voutilainen@gmail.com>

    PR libstdc++/95904
    * include/std/variant (__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.
    (__gen_vtable_impl</*base case*/>::_S_apply):
    Check the visitor return type.

[-- Attachment #2: pr95904.diff --]
[-- Type: text/x-patch, Size: 3478 bytes --]

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..56de78407c4 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,27 @@ namespace __variant
 			   std::forward<_Variants>(__variants)...);
     }
 
+  template <class _Tp, class... _Types>
+    struct __same_types : public std::bool_constant<
+    std::__and_<std::is_same<_Tp, _Types>...>::value> {};
+
+  template <unsigned long int _Idx, class _Visitor, class _Variant>
+    decltype(auto) __check_visitor_result(_Visitor&& __vis,
+					  _Variant&& __variant)
+    {
+      return std::forward<_Visitor>(__vis)(
+        std::get<_Idx>(std::forward<_Variant>(__variant)));
+    }
+
+  template <class _Visitor, class _Variant, unsigned long int... _Idxs>
+    constexpr bool __check_visitor_results(std::index_sequence<_Idxs...>)
+    {
+      return __same_types<decltype(__check_visitor_result<_Idxs>(
+	std::declval<_Visitor>(),
+	std::declval<_Variant>()))...>::value;
+    }
+
+
   template<typename _Visitor, typename... _Variants>
     constexpr decltype(auto)
     visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1704,8 +1740,28 @@ namespace __variant
 
       using _Tag = __detail::__variant::__deduce_visit_result<_Result_type>;
 
-      return std::__do_visit<_Tag>(std::forward<_Visitor>(__visitor),
-				   std::forward<_Variants>(__variants)...);
+      if constexpr (sizeof...(_Variants) == 1)
+        {
+	  constexpr bool __visit_rettypes_match =
+	    __check_visitor_results<_Visitor, _Variants...>(
+	      std::make_index_sequence<
+	        std::variant_size<remove_reference_t<_Variants>...>::value>());
+	  if constexpr (!__visit_rettypes_match)
+	    {
+	      static_assert(__visit_rettypes_match,
+			  "std::visit requires the visitor to have the same "
+			  "return type for all alternatives of a variant");
+	      return;
+	    }
+	  else
+	    return std::__do_visit<_Tag>(
+	      std::forward<_Visitor>(__visitor),
+	      std::forward<_Variants>(__variants)...);
+	}
+      else
+	return std::__do_visit<_Tag>(
+          std::forward<_Visitor>(__visitor),
+	  std::forward<_Variants>(__variants)...);
     }
 
 #if __cplusplus > 201703L

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-10-17 17:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 22:12 [PATCH] c++: Diagnose visitors with different return types for std::visit [PR95904] Ville Voutilainen
2020-09-29 11:20 ` Jonathan Wakely
2020-09-29 16:35   ` [PATCH] libstdc++: Diagnose visitors with different return types [PR95904] Ville Voutilainen
2020-10-02 22:14     ` Jonathan Wakely
2020-10-04 22:15       ` Ville Voutilainen
2020-10-05 19:35         ` Ville Voutilainen
2020-10-08 14:27           ` Jonathan Wakely
2020-10-17 17:30             ` Stephan Bergmann
2020-10-17 17:36               ` Ville Voutilainen
2020-10-10 10:52           ` Jonathan Wakely
2020-10-10 11:15             ` Ville Voutilainen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).