From patchwork Fri Mar 17 14:51:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ville Voutilainen X-Patchwork-Id: 740339 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vl7bQ42cpz9ryj for ; Sat, 18 Mar 2017 01:52:01 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="tCUEI2eT"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; q= dns; s=default; b=QaBZ6cgby6xGPG9HTNs37zLoHnTcljbWBV8FX4LPzCn0hV tvOP2Dqlc0um22PB6ScOJm97dO32LpIxKbzoziRRgp2JR6AZyzti7HpQu2IThqPo SMzC8fULzMBRH+CGwYX4EBLY1PAjJBsV/zyvc98bgYPwpAP+01FSHBwz1MNLs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; s= default; bh=qBA3LeIlhYrjXX9D+OHIGsc0PsU=; b=tCUEI2eTSrlaah+UgG0J w9R9kecHbjmX0OV+ThqmJrhSqCCGx/YbmXDU7FdauIhJbDqgftExydCqdM5VbSqW kvX7uUYT3IpOh7vMrDa3eVYordobdxvRnZPF0dwss75V8Q7OfIXjv5hQZn7rtVXJ JkC7jjgbNIlVrkQOh7YhzhM= Received: (qmail 20158 invoked by alias); 17 Mar 2017 14:51:49 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 20121 invoked by uid 89); 17 Mar 2017 14:51:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=engaged, 3.8, see, 32937 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-ot0-f178.google.com Received: from mail-ot0-f178.google.com (HELO mail-ot0-f178.google.com) (74.125.82.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 17 Mar 2017 14:51:44 +0000 Received: by mail-ot0-f178.google.com with SMTP id a12so23997963ota.0; Fri, 17 Mar 2017 07:51:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=HeE/5cx2EN+yRDNyS7Mvk+65xfgMzVXot9yNKf84QAY=; b=uGCceHFeBDGK3BQqR+JGpXXRPuWNAM9uF5NIG/G99Jou+ANw0EJ2ZWug6bq9pRRWCP n4BrMQAr4mwlgME7oK7WLErJH80I3U6PfPoopZqWXg/reWkAcemCwD6OzBpJL0djgErP m4dnANmu7oNEUKOoEhlNVRBKyDTH82U2RzIWrxu3hdRbCqyqxoz7SPcwiUejUEYZFKaH rmSwTmTi0TPmYCNPWB8CsOqlwUUmz27McNy6YG3a14evtJlsGr6dKURc82SR2qfUcJ4I npB2ZPXOljuOJ3mIs0VNlObLXJCjP/82w21M9dPrAz77K+RylwTPYXBPYkHicm45rIR4 9qlA== X-Gm-Message-State: AFeK/H2DMOG5/nm0N/s70+x/CpEy+zkoijIWwWCl8WPpq0WUL506YxWb/Ab0vwt6Ui/dHUhg9K3EmCqLMNc/6Q== X-Received: by 10.202.74.213 with SMTP id x204mr7816301oia.159.1489762303714; Fri, 17 Mar 2017 07:51:43 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.13.22 with HTTP; Fri, 17 Mar 2017 07:51:43 -0700 (PDT) From: Ville Voutilainen Date: Fri, 17 Mar 2017 16:51:43 +0200 Message-ID: Subject: [v3 PATCH] Implement LWG 2900, The copy and move constructors of optional are not constexpr. To: "libstdc++" , "gcc-patches@gcc.gnu.org" Tested on Linux-x64. 2017-03-17 Ville Voutilainen Implement LWG 2900, The copy and move constructors of optional are not constexpr. * include/std/optional (_Optional_payload): New. (_Optional_base): Remove the bool parameter. (_Optional_base<_Tp, false>): Remove. (_Optional_base()): Adjust. (_Optional_base(nullopt_t)): Likewise. (_Optional_base(in_place_t, _Args&&...)): Likewise. (_Optional_base(in_place_t, initializer_list<_Up>, _Args&&...)): Likewise. (_Optional_base(const _Optional_base&)): Likewise. (_Optional_base(_Optional_base&&)): Likewise. (operator=(const _Optional_base&)): Likewise. (operator=(_Optional_base&&)): Likewise. (~_Optional_base()): Remove. (_M_is_engaged()): Adjust. (_M_get()): Likewise. (_M_construct(_Args&&...)): Likewise. (_M_destruct()): Likewise. (_M_reset()): Likewise. (_Optional_base::_Empty_byte): Remove. (_Optional_base::_M_empty): Remove. (_Optional_base::_M_payload): Adjust. * testsuite/20_util/optional/cons/value_neg.cc: Adjust. * testsuite/20_util/optional/constexpr/cons/value.cc: Add tests. diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 3f540ec..e67ba89 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -95,125 +95,127 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __throw_bad_optional_access() { _GLIBCXX_THROW_OR_ABORT(bad_optional_access()); } - /** - * @brief Class template that holds the necessary state for @ref optional - * and that has the responsibility for construction and the special members. - * - * Such a separate base class template is necessary in order to - * conditionally enable the special members (e.g. copy/move constructors). - * Note that this means that @ref _Optional_base implements the - * functionality for copy and move assignment, but not for converting - * assignment. - * - * @see optional, _Enable_special_members - */ - template::value> - class _Optional_base - { - private: - // Remove const to avoid prohibition of reusing object storage for - // const-qualified types in [3.8/9]. This is strictly internal - // and even optional itself is oblivious to it. - using _Stored_type = remove_const_t<_Tp>; - public: + // Payload for constexpr optionals. + template ::value + && is_trivially_move_constructible<_Tp>::value, + bool /*_ShouldProvideDestructor*/ = + is_trivially_destructible<_Tp>::value> + struct _Optional_payload + { + constexpr _Optional_payload() + : _M_empty() {} - // Constructors for disengaged optionals. - constexpr _Optional_base() noexcept - : _M_empty{} { } + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) + {} - constexpr _Optional_base(nullopt_t) noexcept - : _Optional_base{} { } + template + constexpr _Optional_payload(std::initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(__il, std::forward<_Args>(__args)...), + _M_engaged(true) {} + + template struct __ctor_tag {}; + constexpr _Optional_payload(__ctor_tag, + const _Tp& __other) + : _M_payload(__other), + _M_engaged(true) + {} + + constexpr _Optional_payload(__ctor_tag) + : _M_empty() + {} + + constexpr _Optional_payload(__ctor_tag, _Tp&& __other) + : _M_payload(std::move(__other)), + _M_engaged(true) + {} + + constexpr _Optional_payload(bool __engaged, + const _Optional_payload& + __other) + : _Optional_payload(__engaged ? + _Optional_payload(__ctor_tag{}, + __other._M_payload) : + _Optional_payload(__ctor_tag{})) + {} + + constexpr _Optional_payload(bool __engaged, + _Optional_payload&& + __other) + : _Optional_payload(__engaged ? + _Optional_payload(__ctor_tag{}, + std::move(__other._M_payload)) : + _Optional_payload(__ctor_tag{})) + {} - // Constructors for engaged optionals. - template, bool> = false> - constexpr explicit _Optional_base(in_place_t, _Args&&... __args) - : _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { } + using _Stored_type = remove_const_t<_Tp>; + struct _Empty_byte { }; + union { + _Empty_byte _M_empty; + _Stored_type _M_payload; + }; + bool _M_engaged = false; + }; - template&, - _Args&&...>, bool> = false> - constexpr explicit _Optional_base(in_place_t, - initializer_list<_Up> __il, - _Args&&... __args) - : _M_payload(__il, std::forward<_Args>(__args)...), - _M_engaged(true) { } + // Payload for non-constexpr optionals with non-trivial destructor. + template + struct _Optional_payload<_Tp, false, false> + { + constexpr _Optional_payload() + : _M_empty() {} - // Copy and move constructors. - _Optional_base(const _Optional_base& __other) - { - if (__other._M_engaged) - this->_M_construct(__other._M_get()); - } + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) {} - _Optional_base(_Optional_base&& __other) - noexcept(is_nothrow_move_constructible<_Tp>()) + template + constexpr _Optional_payload(std::initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(__il, std::forward<_Args>(__args)...), + _M_engaged(true) {} + constexpr + _Optional_payload(bool __engaged, const _Optional_payload& __other) + : _Optional_payload(__other) + {} + + constexpr + _Optional_payload(bool __engaged, _Optional_payload&& __other) + : _Optional_payload(std::move(__other)) + {} + + constexpr _Optional_payload(const _Optional_payload& __other) { - if (__other._M_engaged) - this->_M_construct(std::move(__other._M_get())); + if (__other._M_engaged) + this->_M_construct(__other._M_payload); } - // Assignment operators. - _Optional_base& - operator=(const _Optional_base& __other) + constexpr _Optional_payload(_Optional_payload&& __other) { - if (this->_M_engaged && __other._M_engaged) - this->_M_get() = __other._M_get(); - else - { - if (__other._M_engaged) - this->_M_construct(__other._M_get()); - else - this->_M_reset(); - } - - return *this; + if (__other._M_engaged) + this->_M_construct(std::move(__other._M_payload)); } - _Optional_base& - operator=(_Optional_base&& __other) - noexcept(__and_, - is_nothrow_move_assignable<_Tp>>()) - { - if (this->_M_engaged && __other._M_engaged) - this->_M_get() = std::move(__other._M_get()); - else - { - if (__other._M_engaged) - this->_M_construct(std::move(__other._M_get())); - else - this->_M_reset(); - } - return *this; - } + using _Stored_type = remove_const_t<_Tp>; + struct _Empty_byte { }; + union { + _Empty_byte _M_empty; + _Stored_type _M_payload; + }; + bool _M_engaged = false; - // Destructor. - ~_Optional_base() + ~_Optional_payload() { - if (this->_M_engaged) - this->_M_payload.~_Stored_type(); + if (_M_engaged) + _M_payload.~_Stored_type(); } - // The following functionality is also needed by optional, hence the - // protected accessibility. - protected: - constexpr bool _M_is_engaged() const noexcept - { return this->_M_engaged; } - - // The _M_get operations have _M_engaged as a precondition. - constexpr _Tp& - _M_get() noexcept - { return _M_payload; } - - constexpr const _Tp& - _M_get() const noexcept - { return _M_payload; } - - // The _M_construct operation has !_M_engaged as a precondition - // while _M_destruct has _M_engaged as a precondition. template void _M_construct(_Args&&... __args) @@ -223,50 +225,102 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Stored_type(std::forward<_Args>(__args)...); this->_M_engaged = true; } + }; - void - _M_destruct() + // Payload for non-constexpr optionals with trivial destructor. + template + struct _Optional_payload<_Tp, false, true> + { + constexpr _Optional_payload() + : _M_empty() {} + + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) {} + + template + constexpr _Optional_payload(std::initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(__il, std::forward<_Args>(__args)...), + _M_engaged(true) {} + constexpr + _Optional_payload(bool __engaged, const _Optional_payload& __other) + : _Optional_payload(__other) + {} + + constexpr + _Optional_payload(bool __engaged, _Optional_payload&& __other) + : _Optional_payload(std::move(__other)) + {} + + constexpr _Optional_payload(const _Optional_payload& __other) { - this->_M_engaged = false; - this->_M_payload.~_Stored_type(); + if (__other._M_engaged) + this->_M_construct(__other._M_payload); } - // _M_reset is a 'safe' operation with no precondition. - void - _M_reset() + constexpr _Optional_payload(_Optional_payload&& __other) { - if (this->_M_engaged) - this->_M_destruct(); + if (__other._M_engaged) + this->_M_construct(std::move(__other._M_payload)); } - private: + using _Stored_type = remove_const_t<_Tp>; struct _Empty_byte { }; union { _Empty_byte _M_empty; _Stored_type _M_payload; }; bool _M_engaged = false; + + template + void + _M_construct(_Args&&... __args) + noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) + { + ::new (std::__addressof(this->_M_payload)) + _Stored_type(std::forward<_Args>(__args)...); + this->_M_engaged = true; + } }; - /// Partial specialization that is exactly identical to the primary template - /// save for not providing a destructor, to fulfill triviality requirements. + /** + * @brief Class template that holds the necessary state for @ref optional + * and that has the responsibility for construction and the special members. + * + * Such a separate base class template is necessary in order to + * conditionally enable the special members (e.g. copy/move constructors). + * Note that this means that @ref _Optional_base implements the + * functionality for copy and move assignment, but not for converting + * assignment. + * + * @see optional, _Enable_special_members + */ template - class _Optional_base<_Tp, false> + class _Optional_base { private: + // Remove const to avoid prohibition of reusing object storage for + // const-qualified types in [3.8/9]. This is strictly internal + // and even optional itself is oblivious to it. using _Stored_type = remove_const_t<_Tp>; public: + + // Constructors for disengaged optionals. constexpr _Optional_base() noexcept - : _M_empty{} { } + { } constexpr _Optional_base(nullopt_t) noexcept - : _Optional_base{} { } + { } + // Constructors for engaged optionals. template, bool> = false> constexpr explicit _Optional_base(in_place_t, _Args&&... __args) - : _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { } + : _M_payload(in_place, + std::forward<_Args>(__args)...) { } template __il, _Args&&... __args) - : _M_payload(__il, std::forward<_Args>(__args)...), - _M_engaged(true) { } + : _M_payload(in_place, + __il, std::forward<_Args>(__args)...) + { } - _Optional_base(const _Optional_base& __other) - { - if (__other._M_engaged) - this->_M_construct(__other._M_get()); - } + // Copy and move constructors. + constexpr _Optional_base(const _Optional_base& __other) + : _M_payload(__other._M_payload._M_engaged, + __other._M_payload) + { } - _Optional_base(_Optional_base&& __other) + constexpr _Optional_base(_Optional_base&& __other) noexcept(is_nothrow_move_constructible<_Tp>()) - { - if (__other._M_engaged) - this->_M_construct(std::move(__other._M_get())); - } + : _M_payload(__other._M_payload._M_engaged, + std::move(__other._M_payload)) + { } + // Assignment operators. _Optional_base& operator=(const _Optional_base& __other) { - if (this->_M_engaged && __other._M_engaged) - this->_M_get() = __other._M_get(); - else + if (this->_M_payload._M_engaged && __other._M_payload._M_engaged) + this->_M_get() = __other._M_get(); + else { - if (__other._M_engaged) + if (__other._M_payload._M_engaged) this->_M_construct(__other._M_get()); else this->_M_reset(); } - return *this; + + return *this; } _Optional_base& @@ -311,65 +367,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION noexcept(__and_, is_nothrow_move_assignable<_Tp>>()) { - if (this->_M_engaged && __other._M_engaged) + if (this->_M_payload._M_engaged && __other._M_payload._M_engaged) this->_M_get() = std::move(__other._M_get()); else { - if (__other._M_engaged) + if (__other._M_payload._M_engaged) this->_M_construct(std::move(__other._M_get())); else this->_M_reset(); } return *this; } - - // Sole difference - // ~_Optional_base() noexcept = default; - + // The following functionality is also needed by optional, hence the + // protected accessibility. protected: constexpr bool _M_is_engaged() const noexcept - { return this->_M_engaged; } + { return this->_M_payload._M_engaged; } + // The _M_get operations have _M_engaged as a precondition. constexpr _Tp& _M_get() noexcept - { return _M_payload; } + { return this->_M_payload._M_payload; } constexpr const _Tp& _M_get() const noexcept - { return _M_payload; } + { return this->_M_payload._M_payload; } + // The _M_construct operation has !_M_engaged as a precondition + // while _M_destruct has _M_engaged as a precondition. template void _M_construct(_Args&&... __args) noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) { - ::new (std::__addressof(this->_M_payload)) + ::new (std::__addressof(this->_M_payload._M_payload)) _Stored_type(std::forward<_Args>(__args)...); - this->_M_engaged = true; + this->_M_payload._M_engaged = true; } void _M_destruct() { - this->_M_engaged = false; - this->_M_payload.~_Stored_type(); + this->_M_payload._M_engaged = false; + this->_M_payload._M_payload.~_Stored_type(); } + // _M_reset is a 'safe' operation with no precondition. void _M_reset() { - if (this->_M_engaged) + if (this->_M_payload._M_engaged) this->_M_destruct(); } private: - struct _Empty_byte { }; - union - { - _Empty_byte _M_empty; - _Stored_type _M_payload; - }; - bool _M_engaged = false; + _Optional_payload<_Tp> _M_payload; }; template diff --git a/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc b/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc index 249f622..ff17b86 100644 --- a/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc +++ b/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc @@ -37,8 +37,8 @@ int main() std::optional> oup2 = new int; // { dg-error "conversion" } struct U { explicit U(std::in_place_t); }; std::optional ou(std::in_place); // { dg-error "no matching" } - // { dg-error "no type" "" { target { *-*-* } } 437 } - // { dg-error "no type" "" { target { *-*-* } } 447 } - // { dg-error "no type" "" { target { *-*-* } } 504 } + // { dg-error "no type" "" { target { *-*-* } } 489 } + // { dg-error "no type" "" { target { *-*-* } } 499 } + // { dg-error "no type" "" { target { *-*-* } } 556 } } } diff --git a/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc b/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc index b289f44..3b183f8 100644 --- a/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc +++ b/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc @@ -66,4 +66,21 @@ int main() static_assert( o, "" ); static_assert( *o == 0x1234ABCD, "" ); } + { + constexpr std::optional o = 42; + constexpr std::optional o2{o}; + constexpr std::optional o3(o); + constexpr std::optional o4 = o; + constexpr std::optional o5; + constexpr std::optional o6{o5}; + constexpr std::optional o7(o5); + constexpr std::optional o8 = o5; + constexpr std::optional o9{std::move(o)}; + constexpr std::optional o10(std::move(o)); + constexpr std::optional o11 = std::move(o); + constexpr std::optional o12; + constexpr std::optional o13{std::move(o5)}; + constexpr std::optional o14(std::move(o5)); + constexpr std::optional o15 = std::move(o5); + } }