From patchwork Wed Jul 20 23:00:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Li, Pan2 via Gcc-patches" X-Patchwork-Id: 86 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e252:0:0:0:0:0 with SMTP id bl18csp1178236wrb; Wed, 20 Jul 2022 16:03:11 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tZIqDfgHVD6cB58DNXTNJIvIUsaSUh0nFiLFeRKCZY3iQel3L43cOz2lZg1Q2CcUbWwHKy X-Received: by 2002:a17:907:6e02:b0:72b:9f16:1bc5 with SMTP id sd2-20020a1709076e0200b0072b9f161bc5mr38129247ejc.676.1658358191253; Wed, 20 Jul 2022 16:03:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1658358191; cv=none; d=google.com; s=arc-20160816; b=VR+LS6v8xzCKcH5WVkuG+/2FKweoCv1xAN8/s8rcqtM2oG3DdK2ra1HA5mQB0CfpCY tExSEm5OoGnEwb9fu85QrnqFfwnoLp98kupUqHsWnwMQ/leKlCqwNvkfiEDhTf7GRgZH ATuotzN0f6jzlasX3rYvdblbTLbuxJPCoNzw63rZxYFmNRdf7XKHnamQ0WZFg7Sn6NtZ axe6wH7WsNaRAYD+LUg1tXKgaevrUTJCEUQz8ZRuD3j+S91tNDjjOWUZwSaTr6ak70T4 zHwd9p/zrqsPKgpKwW+UbgrAv7FvnhwaZYVkOrubHF/NGG7WaebVmd7Hk+6oNa/Xodlo sJ0w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:dmarc-filter:delivered-to:dkim-signature :dkim-filter; bh=kmyW7L4sCHjK0twVqJFQa+fsM1iZjCx6WduzvIfVqfA=; b=nLAOoPEO+PhXqvDNH2NjXH2WguHeRnh6+UurntI1fMNcalubpowN4p8IIgEmOMbXSc eSFrlDRu/WFoV+0gK+fP6oGk+/tVVN4z+CDJIHetJ8pgpiWAR7NNqNfvJHOe8sU6pne9 sPrr3zHvJJufAFcvYkx8SIDM66V8QHCadMIjHFQ3RqNQu4j+z8dMUwLrY4qtNxrrqi// 1yQA2oOLro/BG3hxiD0cMy+IXX/lkKpkqRcyNcNVtQnensjBCley4R+XN67mYj6wkDBd 5dFW9bApZmesat8GJiWRH/0EmMh31rWFhRWk/8Td35vGjNTGvUWwQI6g4d5+zwVi8mTo Hsag== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=o5KYLolX; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id q17-20020a056402519100b0043586d19cf9si340809edd.465.2022.07.20.16.03.11 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Jul 2022 16:03:11 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=o5KYLolX; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1C3433851C21 for ; Wed, 20 Jul 2022 23:02:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1C3433851C21 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1658358133; bh=kmyW7L4sCHjK0twVqJFQa+fsM1iZjCx6WduzvIfVqfA=; 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=o5KYLolX8Z5zs4/jsu4JVg801f16UcICOHs9gl5rZHWSGgfJQAYXpagwW7IqdDGhy l27vk/Kto1W3ke1Hu05rLvNNc6buPo7bxkztR1hW8V+rsBokz2tsxEriHw5m91PhJJ 1xC07HLBOAtZoXg3CPifVdcUWXwvE40Z00mvyhzU= 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 983EB3858C83 for ; Wed, 20 Jul 2022 23:00:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 983EB3858C83 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-172-Z300pIbcMC-P8sThLWeIxQ-1; Wed, 20 Jul 2022 19:00:13 -0400 X-MC-Unique: Z300pIbcMC-P8sThLWeIxQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DC0583802B81; Wed, 20 Jul 2022 23:00:12 +0000 (UTC) Received: from localhost (unknown [10.33.36.32]) by smtp.corp.redhat.com (Postfix) with ESMTP id A83362026D64; Wed, 20 Jul 2022 23:00:12 +0000 (UTC) To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed 2/3] libstdc++: Fix std::common_iterator assignment [PR100823] Date: Thu, 21 Jul 2022 00:00:10 +0100 Message-Id: <20220720230011.2732391-2-jwakely@redhat.com> In-Reply-To: <20220720230011.2732391-1-jwakely@redhat.com> References: <20220720230011.2732391-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 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_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, URI_HEX 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.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: "Li, Pan2 via Gcc-patches" Reply-To: Jonathan Wakely Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1738914599102419148?= X-GMAIL-MSGID: =?utf-8?q?1738914599102419148?= Tested powerpc64le-linux, pushed to trunk. -- >8 -- This fixes the following conformance problems reported in the PR: - Move constructor and move assignment should be defined. - Copy assignment from a valueless object should be allowed. Assignment is completely rewritten by this patch, as the previous version had a number of problems. The converting assignment failed to handle the case of assigning a new value to a valueless object, which should work. It only accepted lvalue arguments, so wasn't usable to implement the move assignment operator. Finally, it enforced the precondition that the argument is not valueless, which is correct for the converting assignment but not for the copy assignment. A new _M_assign member is added to handle all cases of assignment (copying from an lvalue, moving from an rvalue, and converting from a different type). The not valueless precondition is checked in the converting assignment before calling _M_assign, so isn't enforced for copy and move assignment. The new function no longer uses a switch, so handles valueless objects as the LHS or RHS of the assignment. libstdc++-v3/ChangeLog: PR libstdc++/100823 * include/bits/stl_iterator.h (common_iterator): Define move constructor and move assignment operator. (common_iterator::_M_assign): New function implementing assignment. (common_iterator::operator=): Use _M_assign. (common_iterator::_S_valueless): New constant. * testsuite/24_iterators/common_iterator/100823.cc: New test. --- libstdc++-v3/include/bits/stl_iterator.h | 126 ++++++++++++------ .../24_iterators/common_iterator/100823.cc | 43 ++++++ 2 files changed, 129 insertions(+), 40 deletions(-) create mode 100644 libstdc++-v3/testsuite/24_iterators/common_iterator/100823.cc diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index fd0ae3aa771..a913c04deaa 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -1908,6 +1908,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION noexcept(_S_noexcept()) : _M_valueless(), _M_index(__x._M_index) { + __glibcxx_assert(__x._M_has_value()); if (_M_index == 0) { if constexpr (is_trivially_default_constructible_v<_It>) @@ -1945,14 +1946,58 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } + constexpr + common_iterator(common_iterator&& __x) + noexcept(_S_noexcept<_It, _Sent>()) + : _M_valueless(), _M_index(__x._M_index) + { + if (_M_index == 0) + { + if constexpr (is_trivially_default_constructible_v<_It>) + _M_it = std::move(__x._M_it); + else + std::construct_at(std::__addressof(_M_it), std::move(__x._M_it)); + } + else if (_M_index == 1) + { + if constexpr (is_trivially_default_constructible_v<_Sent>) + _M_sent = std::move(__x._M_sent); + else + std::construct_at(std::__addressof(_M_sent), + std::move(__x._M_sent)); + } + } + + constexpr common_iterator& + operator=(const common_iterator&) = default; + constexpr common_iterator& operator=(const common_iterator& __x) noexcept(is_nothrow_copy_assignable_v<_It> && is_nothrow_copy_assignable_v<_Sent> && is_nothrow_copy_constructible_v<_It> && is_nothrow_copy_constructible_v<_Sent>) + requires (!is_trivially_copy_assignable_v<_It> + || !is_trivially_copy_assignable_v<_Sent>) { - return this->operator=<_It, _Sent>(__x); + _M_assign(__x); + return *this; + } + + constexpr common_iterator& + operator=(common_iterator&&) = default; + + constexpr common_iterator& + operator=(common_iterator&& __x) + noexcept(is_nothrow_move_assignable_v<_It> + && is_nothrow_move_assignable_v<_Sent> + && is_nothrow_move_constructible_v<_It> + && is_nothrow_move_constructible_v<_Sent>) + requires (!is_trivially_move_assignable_v<_It> + || !is_trivially_move_assignable_v<_Sent>) + { + _M_assign(std::move(__x)); + return *this; } template @@ -1967,49 +2012,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION && is_nothrow_assignable_v<_It&, const _It2&> && is_nothrow_assignable_v<_Sent&, const _Sent2&>) { - switch(_M_index << 2 | __x._M_index) - { - case 0b0000: - _M_it = __x._M_it; - break; - case 0b0101: - _M_sent = __x._M_sent; - break; - case 0b0001: - _M_it.~_It(); - _M_index = -1; - [[fallthrough]]; - case 0b1001: - std::construct_at(std::__addressof(_M_sent), _Sent(__x._M_sent)); - _M_index = 1; - break; - case 0b0100: - _M_sent.~_Sent(); - _M_index = -1; - [[fallthrough]]; - case 0b1000: - std::construct_at(std::__addressof(_M_it), _It(__x._M_it)); - _M_index = 0; - break; - default: - __glibcxx_assert(__x._M_has_value()); - __builtin_unreachable(); - } + __glibcxx_assert(__x._M_has_value()); + _M_assign(__x); return *this; } constexpr ~common_iterator() { - switch (_M_index) - { - case 0: - _M_it.~_It(); - break; - case 1: - _M_sent.~_Sent(); - break; - } + if (_M_index == 0) + _M_it.~_It(); + else if (_M_index == 1) + _M_sent.~_Sent(); } [[nodiscard]] @@ -2167,7 +2181,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION requires (!same_as<_It2, _Sent2>) && copyable<_It2> friend class common_iterator; - constexpr bool _M_has_value() const noexcept { return _M_index < 2; } + constexpr bool + _M_has_value() const noexcept { return _M_index != _S_valueless; } + + template + constexpr void + _M_assign(_CIt&& __x) + { + if (_M_index == __x._M_index) + { + if (_M_index == 0) + _M_it = std::forward<_CIt>(__x)._M_it; + else if (_M_index == 1) + _M_sent = std::forward<_CIt>(__x)._M_sent; + } + else + { + if (_M_index == 0) + _M_it.~_It(); + else if (_M_index == 1) + _M_sent.~_Sent(); + _M_index = _S_valueless; + + if (__x._M_index == 0) + std::construct_at(std::__addressof(_M_it), + std::forward<_CIt>(__x)._M_it); + else if (__x._M_index == 1) + std::construct_at(std::__addressof(_M_sent), + std::forward<_CIt>(__x)._M_sent); + _M_index = __x._M_index; + } + } union { @@ -2175,7 +2219,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Sent _M_sent; unsigned char _M_valueless; }; - unsigned char _M_index; // 0==_M_it, 1==_M_sent, 2==valueless + unsigned char _M_index; // 0 == _M_it, 1 == _M_sent, 2 == valueless + + static constexpr unsigned char _S_valueless{2}; }; template diff --git a/libstdc++-v3/testsuite/24_iterators/common_iterator/100823.cc b/libstdc++-v3/testsuite/24_iterators/common_iterator/100823.cc new file mode 100644 index 00000000000..4f2b23de8cc --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/common_iterator/100823.cc @@ -0,0 +1,43 @@ +// { dg-options "-std=gnu++20 -D_GLIBCXX_ASSERTIONS" } +// { dg-do run { target c++20 } } +#include +#include +#include + +void +test_valueless_assignment() +{ + int x[1] { }; + __gnu_test::test_forward_range r(x); + using Iter = decltype(r.begin()); + using Sent = decltype(r.end()); + + std::common_iterator i; + const std::common_iterator j(r.begin()); + try + { + struct Bomb + { + bool operator==(Iter) const { return true; } + operator Sent() const { throw 1; } + }; + std::common_iterator b{Bomb{}}; + i = b; // Throws, leaving i valueless-by-exception. + VERIFY(false); + } + catch (int) + { + std::common_iterator k(i); + + // PR libstdc++/100823 + k = i; // Valid even though both operands are valueless. + + i = j; // No longer valueless. + } + VERIFY( i == j ); +} + +int main() +{ + test_valueless_assignment(); +}