From 9d89b73c06c84ef461da270830f18a7703503e09 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 9 Apr 2019 19:50:48 +0100 Subject: [PATCH] Add comments and style fixes to * include/std/variant: Adjust whitespace. Add comments. (_Multi_array): Leave primary template undefined. (_Multi_array<_Tp>): Define partial specialization for base case of recursion. (__gen_vtable_impl, __gen_vtable): Remove redundant && from type which is always a reference. (__gen_vtable::_S_apply()): Remove function, inline body into default member initializer. * testsuite/20_util/variant/visit.cc: Test with noncopyable types. From-SVN: r270238 --- libstdc++-v3/ChangeLog | 10 ++ libstdc++-v3/include/std/variant | 129 +++++++++++------- .../testsuite/20_util/variant/visit.cc | 22 +++ 3 files changed, 109 insertions(+), 52 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 8fea9b6ccab..98a69cffcb9 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,15 @@ 2019-04-09 Jonathan Wakely + * include/std/variant: Adjust whitespace. Add comments. + (_Multi_array): Leave primary template undefined. + (_Multi_array<_Tp>): Define partial specialization for base case of + recursion. + (__gen_vtable_impl, __gen_vtable): Remove redundant && from type + which is always a reference. + (__gen_vtable::_S_apply()): Remove function, inline body into + default member initializer. + * testsuite/20_util/variant/visit.cc: Test with noncopyable types. + * include/std/variant (__variant_idx_cookie): Add member type. (__visitor_result_type): Remove. (__do_visit): Use invoke_result instead of __visitor_result_type. diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant index 43e8d1d1706..22b0c3d5c22 100644 --- a/libstdc++-v3/include/std/variant +++ b/libstdc++-v3/include/std/variant @@ -145,7 +145,8 @@ namespace __variant __do_visit(_Visitor&& __visitor, _Variants&&... __variants); template - decltype(auto) __variant_cast(_Tp&& __rhs) + decltype(auto) + __variant_cast(_Tp&& __rhs) { if constexpr (is_lvalue_reference_v<_Tp>) { @@ -197,9 +198,10 @@ namespace __variant struct _Uninitialized<_Type, true> { template - constexpr _Uninitialized(in_place_index_t<0>, _Args&&... __args) - : _M_storage(std::forward<_Args>(__args)...) - { } + constexpr + _Uninitialized(in_place_index_t<0>, _Args&&... __args) + : _M_storage(std::forward<_Args>(__args)...) + { } constexpr const _Type& _M_get() const & { return _M_storage; } @@ -220,11 +222,12 @@ namespace __variant struct _Uninitialized<_Type, false> { template - constexpr _Uninitialized(in_place_index_t<0>, _Args&&... __args) - { - ::new ((void*)std::addressof(_M_storage)) - _Type(std::forward<_Args>(__args)...); - } + constexpr + _Uninitialized(in_place_index_t<0>, _Args&&... __args) + { + ::new ((void*)std::addressof(_M_storage)) + _Type(std::forward<_Args>(__args)...); + } const _Type& _M_get() const & { return *_M_storage._M_ptr(); } @@ -360,15 +363,14 @@ namespace __variant struct _Variant_storage; template - using __select_index = - typename __select_int::_Select_int_base::type::value_type; + using __select_index = + typename __select_int::_Select_int_base::type::value_type; template struct _Variant_storage { - constexpr _Variant_storage() : _M_index(variant_npos) { } template @@ -387,7 +389,7 @@ namespace __variant std::_Destroy(std::__addressof(__this_mem)); return {}; }, __variant_cast<_Types...>(*this)); - } + } void _M_reset() { @@ -472,7 +474,7 @@ namespace __variant -> __detail::__variant::__variant_cookie { __variant_construct_single(std::forward<_Tp>(__lhs), - std::forward(__rhs_mem)); + std::forward( __rhs_mem)); return {}; }, __variant_cast<_Types...>(std::forward(__rhs))); } @@ -573,6 +575,7 @@ namespace __variant __variant_construct_single(*this, std::forward<_Up>(__rhs)); } + template void _M_destructive_copy(unsigned short __rhs_index, const _Up& __rhs) { @@ -708,8 +711,7 @@ namespace __variant template using _Move_assign_alias = - _Move_assign_base<_Traits<_Types...>::_S_trivial_move_assign, - _Types...>; + _Move_assign_base<_Traits<_Types...>::_S_trivial_move_assign, _Types...>; template struct _Variant_base : _Move_assign_alias<_Types...> @@ -812,9 +814,13 @@ namespace __variant && !_Variant_never_valueless<__remove_cvref_t<_Variant>>::value; }; - // Used for storing multi-dimensional vtable. + // Used for storing a multi-dimensional vtable. template - struct _Multi_array + struct _Multi_array; + + // Partial specialization with rank zero, stores a single _Tp element. + template + struct _Multi_array<_Tp> { constexpr const _Tp& _M_access() const @@ -823,6 +829,7 @@ namespace __variant _Tp _M_data; }; + // Partial specialization with rank >= 1. template::type; + static constexpr int __do_cookie = _Extra_visit_slot_needed<_Ret, _Variant>::value ? 1 : 0; + using _Tp = _Ret(*)(_Visitor, _Variants...); + template constexpr const _Tp& _M_access(size_t __first_index, _Args... __rest_indices) const - { return _M_arr[__first_index + __do_cookie]._M_access(__rest_indices...); } + { + return _M_arr[__first_index + __do_cookie] + ._M_access(__rest_indices...); + } _Multi_array<_Tp, __rest...> _M_arr[__first + __do_cookie]; }; // Creates a multi-dimensional vtable recursively. // + // The __same_return_types non-type template parameter specifies whether + // to enforce that all visitor invocations return the same type. This is + // required by std::visit but not std::visit. + // // For example, // visit([](auto, auto){}, // variant(), // typedef'ed as V1 // variant()) // typedef'ed as V2 // will trigger instantiations of: - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<0>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<0, 0>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<0, 1>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<0, 2>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<1>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<1, 0>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<1, 1>> - // __gen_vtable_impl<_Multi_array, + // __gen_vtable_impl, // tuple, std::index_sequence<1, 2>> // The returned multi-dimensional vtable can be fast accessed by the visitor // using index calculation. @@ -874,6 +892,13 @@ namespace __variant typename _Array_type, typename _Variant_tuple, typename _Index_seq> struct __gen_vtable_impl; + // Defines the _S_apply() member that returns a _Multi_array populated + // with function pointers that perform the visitation expressions e(m) + // for each valid pack of indexes into the variant types _Variants. + // + // This partial specialization builds up the index sequences by recursively + // calling _S_apply() on the next specialization of __gen_vtable_impl. + // The base case of the recursion defines the actual function pointers. template @@ -940,6 +965,9 @@ namespace __variant } }; + // This partial specialization is the base case for the recursion. + // It populates a _Multi_array element with the address of a function + // that invokes the visitor with the alternatives specified by __indices. template @@ -949,7 +977,7 @@ namespace __variant tuple<_Variants...>, std::index_sequence<__indices...>> { using _Array_type = - _Multi_array<_Result_type (*)(_Visitor&&, _Variants...)>; + _Multi_array<_Result_type (*)(_Visitor, _Variants...)>; template static constexpr decltype(auto) @@ -964,20 +992,22 @@ namespace __variant static constexpr decltype(auto) __visit_invoke_impl(_Visitor&& __visitor, _Variants... __vars) { - if constexpr (is_same_v<_Result_type, __variant_idx_cookie>) - return std::__invoke(std::forward<_Visitor>(__visitor), - __element_by_index_or_cookie<__indices>( - std::forward<_Variants>(__vars))..., - integral_constant()...); - else if constexpr (!__same_return_types && + // For raw visitation using indices, pass the indices to the visitor: + if constexpr (is_same_v<_Result_type, __variant_idx_cookie>) + return std::__invoke(std::forward<_Visitor>(__visitor), + __element_by_index_or_cookie<__indices>( + std::forward<_Variants>(__vars))..., + integral_constant()...); + // For std::visit, cast the result to void: + else if constexpr (!__same_return_types && std::is_void_v<_Result_type>) return (void)std::__invoke(std::forward<_Visitor>(__visitor), - __element_by_index_or_cookie<__indices>( - std::forward<_Variants>(__vars))...); + __element_by_index_or_cookie<__indices>( + std::forward<_Variants>(__vars))...); else return std::__invoke(std::forward<_Visitor>(__visitor), - __element_by_index_or_cookie<__indices>( - std::forward<_Variants>(__vars))...); + __element_by_index_or_cookie<__indices>( + std::forward<_Variants>(__vars))...); } static constexpr decltype(auto) @@ -987,6 +1017,7 @@ namespace __variant std::forward<_Variants>(__vars)...); } + // Perform the implicit conversion to _Result_type for std::visit. static constexpr _Result_type __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars) { @@ -1014,20 +1045,14 @@ namespace __variant typename _Result_type, typename _Visitor, typename... _Variants> struct __gen_vtable { - using _Func_ptr = _Result_type (*)(_Visitor&&, _Variants...); using _Array_type = - _Multi_array<_Func_ptr, + _Multi_array<_Result_type (*)(_Visitor, _Variants...), variant_size_v>...>; - static constexpr _Array_type - _S_apply() - { - return __gen_vtable_impl<__same_return_types, - _Array_type, tuple<_Variants...>, - std::index_sequence<>>::_S_apply(); - } - - static constexpr auto _S_vtable = _S_apply(); + static constexpr _Array_type _S_vtable + = __gen_vtable_impl<__same_return_types, + _Array_type, tuple<_Variants...>, + std::index_sequence<>>::_S_apply(); }; template diff --git a/libstdc++-v3/testsuite/20_util/variant/visit.cc b/libstdc++-v3/testsuite/20_util/variant/visit.cc index 5bd5b3d11f7..ff7cf56b4a9 100644 --- a/libstdc++-v3/testsuite/20_util/variant/visit.cc +++ b/libstdc++-v3/testsuite/20_util/variant/visit.cc @@ -66,8 +66,30 @@ test01() VERIFY( res == 35 ); } +void +test02() +{ + struct NoCopy + { + NoCopy() { } + NoCopy(const NoCopy&) = delete; + NoCopy(NoCopy&&) = delete; + ~NoCopy() { } + + int operator()(int i) { return i; } + int operator()(const NoCopy&) { return 0; } + }; + + std::variant v{1}; + NoCopy f; + // Visit should not need arguments to be copyable: + int res = std::visit(f, v); + VERIFY( res == 1 ); +} + int main() { test01(); + test02(); } -- 2.30.2