From patchwork Wed Jul 24 19:48:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1964479 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=RhIEEY1s; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; 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 [8.43.85.97]) (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 4WTl5G3PS3z1yZw for ; Thu, 25 Jul 2024 05:51:27 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 007493858294 for ; Wed, 24 Jul 2024 19:51:26 +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.129.124]) by sourceware.org (Postfix) with ESMTP id F14693858C48 for ; Wed, 24 Jul 2024 19:50:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F14693858C48 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 F14693858C48 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721850659; cv=none; b=A7NM/w/8lYzdDibPxUSUvaumzW1RHh5mkM6rWdkO62RUFyaikhZaHbc4AsbQacC/0M7zCn3OdXsLaabISY6mJwl5EQhvVupr9e+zOy/J1tpG85YicBI6sycpiww7pz43MnpNJNLmMBpMv8F1IO1dMPNocb/Aubnf0NXj61T4RVM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721850659; c=relaxed/simple; bh=ibK34Rx/O5uQCvXH8mLb3aAFkCbaAvBGN0YaP7r1gyc=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=U8z7cXNGKIuW5ATrDE5RmNxM7uilDjB19OH2+QptIqnc0BMhr5rBjHBvCw5Zj/FxQ+81oEM9YwGJ9CxxI6oDX3CX7/gABB9vX9Dg21Hi4EYnLctU6tn/DYPzxDS8Amk8xUYbE3C+2bkh/dLv10Tdb2vlKE0s0ttPJ8ztpK/ud74= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1721850657; 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=cp7C1HYgy+AXGOHZIF+j5hKz6rqxfcNJt2WO0ZGs1PI=; b=RhIEEY1s6nuy1BBN05JYcaG4udNgcOCfuHIhJVc+W0gEu8d8/bgNpAsAO8E8D2qO2uMbRL RqjlfFQ6vGQCpeE6ZOrBi46d1t8/fczfS88l9xldM0w+UeVAjc4C4hT2QbsEu4A4joUP03 jzjVc85QkNUqvS+0WN5thO2KrdoSavk= Received: from mx-prod-mc-02.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-621-Zriiz6iINuCk2rXPNO20Lw-1; Wed, 24 Jul 2024 15:50:54 -0400 X-MC-Unique: Zriiz6iINuCk2rXPNO20Lw-1 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (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-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 0AC5D1955D45; Wed, 24 Jul 2024 19:50:53 +0000 (UTC) Received: from localhost (unknown [10.42.28.14]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 504681955F40; Wed, 24 Jul 2024 19:50:51 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH 1/2] libstdc++: Move std::optional assertions out of _M_get() Date: Wed, 24 Jul 2024 20:48:35 +0100 Message-ID: <20240724195050.4006283-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 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_H3, 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. Any reason not to do this? I don't think the assertions are useful to catch implementation bugs where we access the contained value without checking it - we should use tests for that. -- >8 -- Currently we implement the precondition for accessing the contained value of a std::optional in the _M_get() accessor in the base class. This means that we always check the assertions even in internal functions that have an explicit check for a contained value being present, such as value() and value_or(U&&). Although those redundant assertions should get optimized out in most cases, they might hurt inliner heuristics and generally give the compiler more work to do. And they won't be optimized out at all for non-optimized builds. The current assertions also result in repeated invalid bug reports, such as PR 91281, PR 101659, PR 102712, and PR 107894. We can move the assertions from the internal accessors to the public member functions where the preconditions are specified. libstdc++-v3/ChangeLog: * include/std/optional (_Optional_base_impl::_M_get()): Move assertions to ... (optional::operator->, optional::operator*): ... here. --- libstdc++-v3/include/std/optional | 40 ++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 48e0f3d36f2..af72004645e 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -472,17 +472,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // The _M_get operations have _M_engaged as a precondition. constexpr _Tp& _M_get() noexcept - { - __glibcxx_assert(this->_M_is_engaged()); - return static_cast<_Dp*>(this)->_M_payload._M_get(); - } + { return static_cast<_Dp*>(this)->_M_payload._M_get(); } constexpr const _Tp& _M_get() const noexcept - { - __glibcxx_assert(this->_M_is_engaged()); - return static_cast(this)->_M_payload._M_get(); - } + { return static_cast(this)->_M_payload._M_get(); } }; /** @@ -958,27 +952,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Observers. constexpr const _Tp* operator->() const noexcept - { return std::__addressof(this->_M_get()); } + { + __glibcxx_assert(this->_M_is_engaged()); + return std::__addressof(this->_M_get()); + } constexpr _Tp* operator->() noexcept - { return std::__addressof(this->_M_get()); } + { + __glibcxx_assert(this->_M_is_engaged()); + return std::__addressof(this->_M_get()); + } constexpr const _Tp& operator*() const& noexcept - { return this->_M_get(); } + { + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_get(); + } constexpr _Tp& operator*()& noexcept - { return this->_M_get(); } + { + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_get(); + } constexpr _Tp&& operator*()&& noexcept - { return std::move(this->_M_get()); } + { + __glibcxx_assert(this->_M_is_engaged()); + return std::move(this->_M_get()); + } constexpr const _Tp&& operator*() const&& noexcept - { return std::move(this->_M_get()); } + { + __glibcxx_assert(this->_M_is_engaged()); + return std::move(this->_M_get()); + } constexpr explicit operator bool() const noexcept { return this->_M_is_engaged(); } From patchwork Wed Jul 24 19:48:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1964480 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=DA9Qp5/e; 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 4WTl6D3kYtz1yZw for ; Thu, 25 Jul 2024 05:52:20 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C55F63858410 for ; Wed, 24 Jul 2024 19:52:18 +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 BA9083858C35 for ; Wed, 24 Jul 2024 19:51:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BA9083858C35 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 BA9083858C35 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=1721850662; cv=none; b=SRUsB2nCD7AAxoQlu3qkUlHpIQas/9VHclvsLNAznjhL6OuTuqHRaEWc0c1UZe3K+VxXB7islbvzwneKknIRZdrOcXxd+1VX47WqTiBCmYJbu4wc92WggtkxJ225n40ecfgux8tFmPtzC62KHng0AcNqO4+EuSW2T6H/h7frCPE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721850662; c=relaxed/simple; bh=wgw3NuyMZzA8I84d6qpiAGHlYWFus20baQJ0XvxLXvI=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=i2uMK+YO3vC+I/aMR0a2XXaX4CGSwGxs924C6L6ez2TEssCqAs1MwQzzEuGZoU40ukDvChJjXgA7CB6pl+OLsU5vYg5TQd96ahcLQR4McrxNmPN/IN07ApniaFPOYBzvaRkPdxJAD/iiv28vYByJizbLnR90y/kyuPvC3QK+suk= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1721850660; 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: in-reply-to:in-reply-to:references:references; bh=NtPK+jkl0yUd2k58PDfAtsJ3By3F5bQECKdrr8VPIJo=; b=DA9Qp5/eX+dLvc/K0X+H00MwKQGyqfuxs8dKt5WUpuR8UJkvoMtBevWiDeeS45JLpuNbQv zQ8VG1KE/gB9VvZakC5n7DiJPXKQGFmT4d+adTS/KOOJvOonIUqmituV4v8mXEUV6bMYPa 6Cd+L9IODgbUEZsp/OAk6NDZDDvDZbY= 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-586-d1uJZpM-OhmXvez_6DYBjw-1; Wed, 24 Jul 2024 15:50:58 -0400 X-MC-Unique: d1uJZpM-OhmXvez_6DYBjw-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (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 9883A19560B0; Wed, 24 Jul 2024 19:50:55 +0000 (UTC) Received: from localhost (unknown [10.42.28.14]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id B904F195605A; Wed, 24 Jul 2024 19:50:54 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH 2/2] libstdc++: Use _M_get() in std::optional internals Date: Wed, 24 Jul 2024 20:48:36 +0100 Message-ID: <20240724195050.4006283-2-jwakely@redhat.com> In-Reply-To: <20240724195050.4006283-1-jwakely@redhat.com> References: <20240724195050.4006283-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 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=ham 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 -- Now that _base::_M_get() doesn't check the precondition, we can use _M_get() instead of operator*() for the internal uses where we've already checked the precondition holds. Add a using-declaration so that we don't need to lookup _M_get in the dependent base class, and make optional a friend so that the converting constructors and assignment operators can use the parameter's _M_get member. libstdc++-v3/ChangeLog: * include/std/optional (optional): Add using-declaraction for _Base::_M_get and declare optional as friend. (optional(const optional&)): Use _M_get instead of operator*. (optional(optional&&)): Likewise. (operator=(const optional&)): Likewise. (operator=(optional&&)): Likewise. (and_then, tansform): Likewise. --- libstdc++-v3/include/std/optional | 38 ++++++++++++++++--------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index af72004645e..9ed7ab50140 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -760,7 +760,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION noexcept(is_nothrow_constructible_v<_Tp, const _Up&>) { if (__t) - emplace(*__t); + emplace(__t._M_get()); } template) { if (__t) - emplace(*__t); + emplace(__t._M_get()); } template) { if (__t) - emplace(std::move(*__t)); + emplace(std::move(__t._M_get())); } template) { if (__t) - emplace(std::move(*__t)); + emplace(std::move(__t._M_get())); } template_M_is_engaged()) - this->_M_get() = *__u; + this->_M_get() = __u._M_get(); else - this->_M_construct(*__u); + this->_M_construct(__u._M_get()); } else { @@ -889,9 +889,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (__u) { if (this->_M_is_engaged()) - this->_M_get() = std::move(*__u); + this->_M_get() = std::move(__u._M_get()); else - this->_M_construct(std::move(*__u)); + this->_M_construct(std::move(__u._M_get())); } else { @@ -1056,7 +1056,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return static_cast<_Tp>(std::forward<_Up>(__u)); } -#if __cpp_lib_optional >= 202110L +#if __cpp_lib_optional >= 202110L // C++23 // [optional.monadic] template @@ -1068,7 +1068,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION "the function passed to std::optional::and_then " "must return a std::optional"); if (has_value()) - return std::__invoke(std::forward<_Fn>(__f), **this); + return std::__invoke(std::forward<_Fn>(__f), _M_get()); else return _Up(); } @@ -1082,7 +1082,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION "the function passed to std::optional::and_then " "must return a std::optional"); if (has_value()) - return std::__invoke(std::forward<_Fn>(__f), **this); + return std::__invoke(std::forward<_Fn>(__f), _M_get()); else return _Up(); } @@ -1096,7 +1096,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION "the function passed to std::optional::and_then " "must return a std::optional"); if (has_value()) - return std::__invoke(std::forward<_Fn>(__f), std::move(**this)); + return std::__invoke(std::forward<_Fn>(__f), std::move(_M_get())); else return _Up(); } @@ -1110,7 +1110,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION "the function passed to std::optional::and_then " "must return a std::optional"); if (has_value()) - return std::__invoke(std::forward<_Fn>(__f), std::move(**this)); + return std::__invoke(std::forward<_Fn>(__f), std::move(_M_get())); else return _Up(); } @@ -1121,7 +1121,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { using _Up = remove_cv_t>; if (has_value()) - return optional<_Up>(_Optional_func<_Fn>{__f}, **this); + return optional<_Up>(_Optional_func<_Fn>{__f}, _M_get()); else return optional<_Up>(); } @@ -1132,7 +1132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { using _Up = remove_cv_t>; if (has_value()) - return optional<_Up>(_Optional_func<_Fn>{__f}, **this); + return optional<_Up>(_Optional_func<_Fn>{__f}, _M_get()); else return optional<_Up>(); } @@ -1143,7 +1143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { using _Up = remove_cv_t>; if (has_value()) - return optional<_Up>(_Optional_func<_Fn>{__f}, std::move(**this)); + return optional<_Up>(_Optional_func<_Fn>{__f}, std::move(_M_get())); else return optional<_Up>(); } @@ -1154,7 +1154,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { using _Up = remove_cv_t>; if (has_value()) - return optional<_Up>(_Optional_func<_Fn>{__f}, std::move(**this)); + return optional<_Up>(_Optional_func<_Fn>{__f}, std::move(_M_get())); else return optional<_Up>(); } @@ -1193,9 +1193,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX20_CONSTEXPR void reset() noexcept { this->_M_reset(); } private: -#if __cplusplus >= 202002L + using _Base::_M_get; + template friend class optional; +#if __cpp_lib_optional >= 202110L // C++23 template explicit constexpr optional(_Optional_func<_Fn> __f, _Value&& __v)