On 08/04/19 17:02 +0100, Jonathan Wakely wrote:
+  template<typename _Result_type, typename _Visitor, typename... _Variants,
+          size_t... __indices>
+    struct __gen_vtable_r_impl<
+      _Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
+                  tuple<_Variants...>, std::index_sequence<__indices...>>
+    {
+      using _Array_type =
+         _Multi_array<_Result_type (*)(_Visitor, _Variants...)>;
+
+      template<size_t __index, typename _Variant>
+       static constexpr decltype(auto)
+       __element_by_index_or_cookie(_Variant&& __var)
+        {
+         if constexpr (__index != variant_npos)
+           return __variant::__get<__index>(std::forward<_Variant>(__var));
+         else
+           return __variant_cookie{};
+       }

I don't think we need __element_by_index_or_cookie for the visit<R>
case, so the function above can go, and ...

+      static constexpr _Result_type
+      __visit_r_invoke(_Visitor&& __visitor, _Variants... __vars)
+      {
+       if constexpr (is_void_v<_Result_type>)
+         return (void) std::__invoke(std::forward<_Visitor>(__visitor),
+           __element_by_index_or_cookie<__indices>(

this just becomes __variant::__get<__indices>(

+             std::forward<_Variants>(__vars))...);
+       else
+         return std::__invoke(std::forward<_Visitor>(__visitor),
+           __element_by_index_or_cookie<__indices>(

and here too.

+             std::forward<_Variants>(__vars))...);
+      }
+
+      static constexpr _Array_type
+      _S_apply()
+      { return _Array_type{&__visit_r_invoke}; }
+    };

With the above simplification the visit<R> code is a lot simpler than
the existing __gen_vtable code, let alone the version that adds
visit<R> support to it.

I'll spend some more time thinking about it.


+  template<typename _Result_type, typename _Visitor, typename... _Variants>
+    struct __gen_vtable_r
+    {
+      using _Array_type =
+         _Multi_array<_Result_type (*)(_Visitor&&, _Variants...),
+                      variant_size_v<remove_reference_t<_Variants>>...>;
+
+      static constexpr _Array_type _S_vtable
+       = __gen_vtable_r_impl<_Array_type, tuple<_Variants...>,
+                             std::index_sequence<>>::_S_apply();

N.B. here I just declare a variable and its default initializer. I
don't see any benefit to how the original in __gen_vtable does it:

     static constexpr _Array_type
     _S_apply()
     {
        return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
                                 std::index_sequence<>>::_S_apply();
     }

     static constexpr auto _S_vtable = _S_apply();

We only ever use that function in the default initializer, so might as
well get rid of it.


Reply via email to