From patchwork Wed Jul 24 19:52:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1964481 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YfyKftj+; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WTl7h6sz5z1yZw for ; Thu, 25 Jul 2024 05:53:36 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 269BF3858289 for ; Wed, 24 Jul 2024 19:53:35 +0000 (GMT) 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 ESMTP id CA25E3858C35 for ; Wed, 24 Jul 2024 19:53:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CA25E3858C35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CA25E3858C35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721850795; cv=none; b=wNfPmwKYn8CvXoBTQwX+O3SINKvkM7tezhz55D7bIAkUeMYtojcnIeZoILtVdoFUAThLIF1/WY3/m5tiyK/wwT9Qe/UqE9wEJMj82yebJuY2aHqiYsHUZ9y30DXSJIN70JTP5VIAElJCBAfaEC34fQBAcK5wKtvYymiRlSuDG9Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721850795; c=relaxed/simple; bh=+WK8cPgKBIRtM4/8R8hrTMg5Uc0B+7Ns60bIIQ4skW0=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=RP/GjtR0aYC+MLnMhfyQkWM3bB3nGLzjL5gqP1pDs+YeOx3OvBB9jdDnL0z6JH22ZVezN/z3nIGT5+Y3BK53vUXTuGMuTeTISOfX0G77gJnKYTB8gIh9n/9Rx0C9VXtKQ1Vr69swrjWwou9QWga24xMbwNHwZAcAGepczwRDhpE= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1721850792; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=VNwjwHGLO1bmwLq+a3Oc9A/cGH15x0fTCF4fJvs/law=; b=YfyKftj+Z1u2fJG9OfJ3t2OU0Tz48NoEqOVCeGylifM0GocvVNWUNQJ3k9OF2gOdijYMK6 UudWuHbIsvjgP9z4KOlRYkYd43K43er+LIM1HCoXhtkIJmmIgOHiHWdPjFOX7je0QrL/GP ixCogK/Ty7l4l/JGpuVB8fhUdSq7W34= Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-186-Uf8AHwvuMdGa3fKSREkUVw-1; Wed, 24 Jul 2024 15:53:09 -0400 X-MC-Unique: Uf8AHwvuMdGa3fKSREkUVw-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 528E219560B3; Wed, 24 Jul 2024 19:53:08 +0000 (UTC) Received: from localhost (unknown [10.42.28.14]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 65FA619560AE; Wed, 24 Jul 2024 19:53:07 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH] libstdc++: Use concepts to simplify std::optional base classes Date: Wed, 24 Jul 2024 20:52:52 +0100 Message-ID: <20240724195305.4007818-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.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_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org Tested x86_64-linux. -- >8 -- In C++20 mode we can simplify some of the std::optional base class hierarchy using concepts. We can overload the destructor and copy constructor and move constructor with a trivial defaulted version and a constrained non-trivial version. This allows us to remove some class template partial specializations that were used to conditionally define those special members as trivial or non-trivial. This should not change any semantics, but should be less work for the compiler, due to not needing to match partial specializations, and completely removing one level of the inheritance hierarchy. libstdc++-v3/ChangeLog: * include/std/optional (_Optional_payload_base::_Storage) [C++20]: Define constrained non-trivial destructor. (_Optional_payload_base::_Storage) [C++20]: Do not define partial specialization when primary template has constrained destructor. (_Optional_base) [C++20]: Define constrained trivial copy and move cons and move constructors. Define payload accessors here instead of inheriting them from _Optional_base_impl. (_Optional_base_impl, _Optional_base) (_Optional_base, _Optional_base) [C++20]: Do not define. --- libstdc++-v3/include/std/optional | 216 ++++++++++++++++++++---------- 1 file changed, 146 insertions(+), 70 deletions(-) diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 9ed7ab50140..344be5e44d3 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -226,10 +226,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { } #endif +#if __cpp_concepts >= 202002L // Conditionally trivial special member functions + ~_Storage() = default; + + // User-provided destructor is needed when _Up has non-trivial dtor. + _GLIBCXX20_CONSTEXPR + ~_Storage() requires (!is_trivially_destructible_v<_Up>) + { } + + _Storage(const _Storage&) = default; + _Storage(_Storage&&) = default; + _Storage& operator=(const _Storage&) = default; + _Storage& operator=(_Storage&&) = default; +#endif + _Empty_byte _M_empty; _Up _M_value; }; +#if __cpp_concepts < 202002L template union _Storage<_Up, false> { @@ -259,9 +274,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // User-provided destructor is needed when _Up has non-trivial dtor. _GLIBCXX20_CONSTEXPR ~_Storage() { } + _Storage(const _Storage&) = default; + _Storage(_Storage&&) = default; + _Storage& operator=(const _Storage&) = default; + _Storage& operator=(_Storage&&) = default; + _Empty_byte _M_empty; _Up _M_value; }; +#endif _Storage<_Stored_type> _M_payload; @@ -438,8 +459,128 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX20_CONSTEXPR ~_Optional_payload() { this->_M_reset(); } }; + /** + * @brief Class template that provides copy/move constructors of optional. + * + * Such a separate base class template is necessary in order to + * conditionally make copy/move constructors trivial. + * + * When the contained value is trivially copy/move constructible, + * the copy/move constructors of _Optional_base will invoke the + * trivial copy/move constructor of _Optional_payload. Otherwise, + * they will invoke _Optional_payload(bool, const _Optional_payload&) + * or _Optional_payload(bool, _Optional_payload&&) to initialize + * the contained value, if copying/moving an engaged optional. + * + * Whether the other special members are trivial is determined by the + * _Optional_payload<_Tp> specialization used for the _M_payload member. + * + * @see optional, _Enable_special_members + */ + template, + bool = is_trivially_move_constructible_v<_Tp>> + struct _Optional_base + { + // Constructors for disengaged optionals. + constexpr _Optional_base() = default; + + // Constructors for engaged optionals. + template, bool> = false> + constexpr explicit + _Optional_base(in_place_t, _Args&&... __args) + : _M_payload(in_place, std::forward<_Args>(__args)...) + { } + + template&, + _Args...>, bool> = false> + constexpr explicit + _Optional_base(in_place_t, + initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(in_place, __il, std::forward<_Args>(__args)...) + { } + + // Copy and move constructors. + constexpr + _Optional_base(const _Optional_base& __other) + noexcept(is_nothrow_copy_constructible_v<_Tp>) + : _M_payload(__other._M_payload._M_engaged, __other._M_payload) + { } + + constexpr + _Optional_base(_Optional_base&& __other) + noexcept(is_nothrow_move_constructible_v<_Tp>) + : _M_payload(__other._M_payload._M_engaged, + std::move(__other._M_payload)) + { } + +#if __cpp_concepts >= 202002L // Conditionally trivial special member functions + // Define these in the primary template if possible, so that we don't + // need to use partial specializations of this class template. + constexpr _Optional_base(const _Optional_base&) + requires is_trivially_copy_constructible_v<_Tp> = default; + + constexpr _Optional_base(_Optional_base&&) + requires is_trivially_move_constructible_v<_Tp> = default; +#endif + + // Assignment operators. + _Optional_base& operator=(const _Optional_base&) = default; + _Optional_base& operator=(_Optional_base&&) = default; + + _Optional_payload<_Tp> _M_payload; + + protected: + // For the primary template, we define these functions here. + using _Stored_type = remove_const_t<_Tp>; + + // The _M_construct operation has !_M_engaged as a precondition + // while _M_destruct has _M_engaged as a precondition. + template + constexpr void + _M_construct(_Args&&... __args) + noexcept(is_nothrow_constructible_v<_Stored_type, _Args...>) + { + _M_payload._M_construct(std::forward<_Args>(__args)...); + } + + constexpr void + _M_destruct() noexcept + { _M_payload._M_destroy(); } + + // _M_reset is a 'safe' operation with no precondition. + constexpr void + _M_reset() noexcept + { _M_payload._M_reset(); } + + constexpr bool _M_is_engaged() const noexcept + { return _M_payload._M_engaged; } + + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept + { return _M_payload._M_get(); } + + constexpr const _Tp& + _M_get() const noexcept + { return _M_payload._M_get(); } + }; + +#if __cpp_concepts < 202002L + // If P0848R3 "Conditionally Trivial Special Member Functions" is not + // supported (as determined from the __cpp_concepts macro value), the + // _Optional_base primary template only has non-trivial copy and move + // constructors. Use partial specializations of _Optional_base + // that have a trivial copy and/or move constructor. + // Common base class for _Optional_base to avoid repeating these - // member functions in each specialization. + // member functions in each partial specialization. + // Only used if P0848R3 "Conditionally Trivial Special Member Functions" + // is not supported, as indicated by the __cpp_concepts value. template class _Optional_base_impl { @@ -479,74 +620,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return static_cast(this)->_M_payload._M_get(); } }; - /** - * @brief Class template that provides copy/move constructors of optional. - * - * Such a separate base class template is necessary in order to - * conditionally make copy/move constructors trivial. - * - * When the contained value is trivially copy/move constructible, - * the copy/move constructors of _Optional_base will invoke the - * trivial copy/move constructor of _Optional_payload. Otherwise, - * they will invoke _Optional_payload(bool, const _Optional_payload&) - * or _Optional_payload(bool, _Optional_payload&&) to initialize - * the contained value, if copying/moving an engaged optional. - * - * Whether the other special members are trivial is determined by the - * _Optional_payload<_Tp> specialization used for the _M_payload member. - * - * @see optional, _Enable_special_members - */ - template, - bool = is_trivially_move_constructible_v<_Tp>> - struct _Optional_base - : _Optional_base_impl<_Tp, _Optional_base<_Tp>> - { - // Constructors for disengaged optionals. - constexpr _Optional_base() = default; - - // Constructors for engaged optionals. - template, bool> = false> - constexpr explicit - _Optional_base(in_place_t, _Args&&... __args) - : _M_payload(in_place, std::forward<_Args>(__args)...) - { } - - template&, - _Args...>, bool> = false> - constexpr explicit - _Optional_base(in_place_t, - initializer_list<_Up> __il, - _Args&&... __args) - : _M_payload(in_place, __il, std::forward<_Args>(__args)...) - { } - - // Copy and move constructors. - constexpr - _Optional_base(const _Optional_base& __other) - : _M_payload(__other._M_payload._M_engaged, __other._M_payload) - { } - - constexpr - _Optional_base(_Optional_base&& __other) - noexcept(is_nothrow_move_constructible_v<_Tp>) - : _M_payload(__other._M_payload._M_engaged, - std::move(__other._M_payload)) - { } - - // Assignment operators. - _Optional_base& operator=(const _Optional_base&) = default; - _Optional_base& operator=(_Optional_base&&) = default; - - _Optional_payload<_Tp> _M_payload; - }; - template - struct _Optional_base<_Tp, false, true> + struct _Optional_base<_Tp, false, true> // trivial move ctor : _Optional_base_impl<_Tp, _Optional_base<_Tp>> { // Constructors for disengaged optionals. @@ -586,7 +661,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; template - struct _Optional_base<_Tp, true, false> + struct _Optional_base<_Tp, true, false> // trivial copy ctor : _Optional_base_impl<_Tp, _Optional_base<_Tp>> { // Constructors for disengaged optionals. @@ -629,7 +704,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; template - struct _Optional_base<_Tp, true, true> + struct _Optional_base<_Tp, true, true> // trivial copy and move ctors : _Optional_base_impl<_Tp, _Optional_base<_Tp>> { // Constructors for disengaged optionals. @@ -664,6 +739,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Optional_payload<_Tp> _M_payload; }; +#endif // __cpp_concepts template class optional;