From patchwork Thu Nov 4 09:42:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 47039 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 96C423857C6B for ; Thu, 4 Nov 2021 09:46:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 96C423857C6B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636019183; bh=oMHV9CVYW91LxpIWZDs1GRk/P5AKRddqCC1wttLfkCE=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=oniDIhIbf7oQ76TEalYfhE2uop7oZGzxkgTBAmrQkNkxu8texuiY1MGuRXh5Zk5+D +2lRx8bj91wKsKSSVDCKrYE536EWmtFjC/Oot1caGdb/XH2+DFNKhsmBXILHbDb99q AryCA6MlQ7vpPvayXDBrQoTAZiPr2kQwSiGBHlb4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id BCF7D3857C43 for ; Thu, 4 Nov 2021 09:42:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BCF7D3857C43 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-100-hiDY1cpWNTaqSJgAauqb_w-1; Thu, 04 Nov 2021 05:42:14 -0400 X-MC-Unique: hiDY1cpWNTaqSJgAauqb_w-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 84104875048; Thu, 4 Nov 2021 09:42:13 +0000 (UTC) Received: from localhost (unknown [10.33.36.17]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3035419723; Thu, 4 Nov 2021 09:42:13 +0000 (UTC) To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed 3/3] libstdc++: Refactor emplace-like functions in std::variant Date: Thu, 4 Nov 2021 09:42:09 +0000 Message-Id: <20211104094209.1930691-3-jwakely@redhat.com> In-Reply-To: <20211104094209.1930691-1-jwakely@redhat.com> References: <20211104094209.1930691-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-14.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Tested powerpc64le-linux, pushed to trunk. libstdc++-v3/ChangeLog: * include/std/variant (__detail::__variant::__emplace): New function template. (_Copy_assign_base::operator=): Reorder conditions to match bulleted list of effects in the standard. Use __emplace instead of _M_reset followed by _Construct. (_Move_assign_base::operator=): Likewise. (__construct_by_index): Remove. (variant::emplace): Use __emplace instead of _M_reset followed by __construct_by_index. (variant::swap): Hoist valueless cases out of visitor. Use __emplace to replace _M_reset followed by _Construct. --- libstdc++-v3/include/std/variant | 179 ++++++++++++++----------------- 1 file changed, 82 insertions(+), 97 deletions(-) diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant index dc3d032c543..c4c307b7bb2 100644 --- a/libstdc++-v3/include/std/variant +++ b/libstdc++-v3/include/std/variant @@ -590,6 +590,19 @@ namespace __variant __index_type _M_index; }; + // Implementation of v.emplace(args...). + template + _GLIBCXX20_CONSTEXPR + inline void + __emplace(_Variant_storage<_Triv, _Types...>& __v, _Args&&... __args) + { + __v._M_reset(); + auto* __addr = std::__addressof(__variant::__get_n<_Np>(__v._M_u)); + std::_Construct(__addr, std::forward<_Args>(__args)...); + // Construction didn't throw, so can set the new index now: + __v._M_index = _Np; + } + template using _Variant_storage_alias = _Variant_storage<_Traits<_Types...>::_S_trivial_dtor, _Types...>; @@ -655,6 +668,7 @@ namespace __variant }, __variant_cast<_Types...>(std::move(__rhs))); this->_M_index = __rhs._M_index; } + _Move_ctor_base(const _Move_ctor_base&) = default; _Move_ctor_base& operator=(const _Move_ctor_base&) = default; _Move_ctor_base& operator=(_Move_ctor_base&&) = default; @@ -685,36 +699,24 @@ namespace __variant __variant::__raw_idx_visit( [this](auto&& __rhs_mem, auto __rhs_index) mutable { - if constexpr (__rhs_index != variant_npos) + constexpr size_t __j = __rhs_index; + if constexpr (__j == variant_npos) + this->_M_reset(); // Make *this valueless. + else if (this->_M_index == __j) + __variant::__get<__j>(*this) = __rhs_mem; + else { - if (this->_M_index == __rhs_index) - __variant::__get<__rhs_index>(*this) = __rhs_mem; + using _Tj = typename _Nth_type<__j, _Types...>::type; + if constexpr (is_nothrow_copy_constructible_v<_Tj> + || !is_nothrow_move_constructible_v<_Tj>) + __variant::__emplace<__j>(*this, __rhs_mem); else { - using __rhs_type = __remove_cvref_t; - if constexpr (is_nothrow_copy_constructible_v<__rhs_type> - || !is_nothrow_move_constructible_v<__rhs_type>) - { - // The standard says emplace<__rhs_index>(__rhs_mem) - // should be used here, but this is equivalent. Either - // copy construction doesn't throw, so we have strong - // exception safety guarantee, or both copy construction - // and move construction can throw, so emplace only - // gives basic exception safety anyway. - this->_M_reset(); - std::_Construct(std::__addressof(this->_M_u), - in_place_index<__rhs_index>, - __rhs_mem); - this->_M_index = __rhs_index; - } - else - __variant_cast<_Types...>(*this) - = variant<_Types...>(std::in_place_index<__rhs_index>, - __rhs_mem); + using _Variant = variant<_Types...>; + _Variant& __self = __variant_cast<_Types...>(*this); + __self = _Variant(in_place_index<__j>, __rhs_mem); } } - else - this->_M_reset(); }, __variant_cast<_Types...>(__rhs)); return *this; } @@ -749,13 +751,23 @@ namespace __variant __variant::__raw_idx_visit( [this](auto&& __rhs_mem, auto __rhs_index) mutable { - if constexpr (__rhs_index != variant_npos) + constexpr size_t __j = __rhs_index; + if constexpr (__j != variant_npos) { - if (this->_M_index == __rhs_index) - __variant::__get<__rhs_index>(*this) = std::move(__rhs_mem); + if (this->_M_index == __j) + __variant::__get<__j>(*this) = std::move(__rhs_mem); else - __variant_cast<_Types...>(*this) - .template emplace<__rhs_index>(std::move(__rhs_mem)); + { + using _Tj = typename _Nth_type<__j, _Types...>::type; + if constexpr (is_nothrow_move_constructible_v<_Tj>) + __variant::__emplace<__j>(*this, std::move(__rhs_mem)); + else + { + using _Variant = variant<_Types...>; + _Variant& __self = __variant_cast<_Types...>(*this); + __self.template emplace<__j>(std::move(__rhs_mem)); + } + } } else this->_M_reset(); @@ -1164,17 +1176,6 @@ namespace __variant >; } - template - _GLIBCXX20_CONSTEXPR - inline void - __construct_by_index(_Variant& __v, _Args&&... __args) - { - std::_Construct(std::__addressof(__variant::__get<_Np>(__v)), - std::forward<_Args>(__args)...); - // Construction didn't throw, so can set the new index now: - __v._M_index = _Np; - } - } // namespace __variant } // namespace __detail @@ -1415,11 +1416,6 @@ namespace __variant friend _GLIBCXX20_CONSTEXPR decltype(auto) __variant_cast(_Tp&&); - template - friend _GLIBCXX20_CONSTEXPR void - __detail::__variant::__construct_by_index(_Variant& __v, - _Args&&... __args); - static_assert(sizeof...(_Types) > 0, "variant must have at least one alternative"); static_assert(!(std::is_reference_v<_Types> || ...), @@ -1589,17 +1585,14 @@ namespace __variant // to avoid becoming valueless. if constexpr (is_nothrow_constructible_v) { - this->_M_reset(); - __variant::__construct_by_index<_Np>(*this, - std::forward<_Args>(__args)...); + __variant::__emplace<_Np>(*this, std::forward<_Args>(__args)...); } else if constexpr (is_scalar_v) { // This might invoke a potentially-throwing conversion operator: const type __tmp(std::forward<_Args>(__args)...); - // But these steps won't throw: - this->_M_reset(); - __variant::__construct_by_index<_Np>(*this, __tmp); + // But this won't throw: + __variant::__emplace<_Np>(*this, __tmp); } else if constexpr (__variant::_Never_valueless_alt() && _Traits::_S_move_assign) @@ -1614,9 +1607,7 @@ namespace __variant { // This case only provides the basic exception-safety guarantee, // i.e. the variant can become valueless. - this->_M_reset(); - __variant::__construct_by_index<_Np>(*this, - std::forward<_Args>(__args)...); + __variant::__emplace<_Np>(*this, std::forward<_Args>(__args)...); } return std::get<_Np>(*this); } @@ -1636,9 +1627,8 @@ namespace __variant initializer_list<_Up>&, _Args...>) { - this->_M_reset(); - __variant::__construct_by_index<_Np>(*this, __il, - std::forward<_Args>(__args)...); + __variant::__emplace<_Np>(*this, __il, + std::forward<_Args>(__args)...); } else if constexpr (__variant::_Never_valueless_alt() && _Traits::_S_move_assign) @@ -1653,9 +1643,8 @@ namespace __variant { // This case only provides the basic exception-safety guarantee, // i.e. the variant can become valueless. - this->_M_reset(); - __variant::__construct_by_index<_Np>(*this, __il, - std::forward<_Args>(__args)...); + __variant::__emplace<_Np>(*this, __il, + std::forward<_Args>(__args)...); } return std::get<_Np>(*this); } @@ -1686,61 +1675,57 @@ namespace __variant noexcept((__is_nothrow_swappable<_Types>::value && ...) && is_nothrow_move_constructible_v) { - __detail::__variant::__raw_idx_visit( + static_assert((is_move_constructible_v<_Types> && ...)); + + // Handle this here to simplify the visitation. + if (__rhs.valueless_by_exception()) [[__unlikely__]] + { + if (!this->valueless_by_exception()) [[__likely__]] + __rhs.swap(*this); + return; + } + + namespace __variant = __detail::__variant; + + __variant::__raw_idx_visit( [this, &__rhs](auto&& __rhs_mem, auto __rhs_index) mutable { - if constexpr (__rhs_index != variant_npos) + constexpr size_t __j = __rhs_index; + if constexpr (__j != variant_npos) { - if (this->index() == __rhs_index) + if (this->index() == __j) { - auto& __this_mem = - std::get<__rhs_index>(*this); using std::swap; - swap(__this_mem, __rhs_mem); + swap(std::get<__j>(*this), __rhs_mem); } else { - constexpr size_t __j = __rhs_index; - if (!this->valueless_by_exception()) [[__likely__]] - { - auto __tmp(std::move(__rhs_mem)); - __rhs = std::move(*this); - this->_M_reset(); - std::_Construct(std::__addressof(this->_M_u), - in_place_index<__j>, - std::move(__tmp)); - this->_M_index = __j; - } + auto __tmp(std::move(__rhs_mem)); + + if constexpr (_Traits::_S_trivial_move_assign) + __rhs = std::move(*this); else - { - this->_M_reset(); - std::_Construct(std::__addressof(this->_M_u), - in_place_index<__j>, - std::move(__rhs_mem)); - this->_M_index = __j; - __rhs._M_reset(); - } - } - } - else - { - if (!this->valueless_by_exception()) [[__likely__]] - { - __rhs = std::move(*this); - this->_M_reset(); + __variant::__raw_idx_visit( + [&__rhs](auto&& __this_mem, auto __this_index) mutable + { + constexpr size_t __k = __this_index; + if constexpr (__k != variant_npos) + __variant::__emplace<__k>(__rhs, + std::move(__this_mem)); + }, *this); + + __variant::__emplace<__j>(*this, std::move(__tmp)); } } }, __rhs); } - private: - #if defined(__clang__) && __clang_major__ <= 7 public: using _Base::_M_u; // See https://bugs.llvm.org/show_bug.cgi?id=31852 - private: #endif + private: template friend constexpr decltype(auto) __detail::__variant::__get(_Vp&& __v) noexcept;