From patchwork Tue Apr 18 23:00:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 20083 Received: (qmail 36217 invoked by alias); 18 Apr 2017 23:00:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 36197 invoked by uid 89); 18 Apr 2017 23:00:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=stolen, stick, 2046, 20141 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Apr 2017 23:00:56 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 70108C05975B for ; Tue, 18 Apr 2017 23:00:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 70108C05975B Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 70108C05975B Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id F2056183B7 for ; Tue, 18 Apr 2017 23:00:56 +0000 (UTC) Subject: Re: [PATCH 3/7] More gdb::optional features To: GDB Patches References: <1491826948-21874-1-git-send-email-palves@redhat.com> <1491826948-21874-4-git-send-email-palves@redhat.com> From: Pedro Alves Message-ID: <57090eb4-6338-2c04-2ae1-4c6ac0a0903a@redhat.com> Date: Wed, 19 Apr 2017 00:00:56 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1491826948-21874-4-git-send-email-palves@redhat.com> This is what I pushed. It's the same patch, but with commit log tweaked to mention the issue with returning gdb::optional from functions. From 22796e972f18c5601cecb0251222411a352836b6 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 18 Apr 2017 21:39:24 +0100 Subject: [PATCH] More gdb::optional features MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we can't use gdb::optional as function return type, because gdb::optional's copy ctor is deleted. For example, with: gdb::optional function () { gdb::optional opt; .... return opt; we get: src/gdb/foo.c: In function ‘gdb::optional foo()’: src/gdb/foo.c:75:10: error: use of deleted function ‘gdb::optional::optional(const gdb::optional&) [with T = int]’ return opt; ^ In file included from src/gdb/foo.c:68:0: src/gdb/common/gdb_optional.h:53:3: note: declared here optional (const optional &other) = delete; ^ I started by fixing that, and then ran into another missing feature, also fixed by this patch. The next feature I'm missing most from gdb::optional compared to std::optional is construction/move/assignment from a T, instead of having to default construct an gdb::optional and then use optional::emplace(....). For example: gdb::optional function () { gdb::optional opt; std::string str; ... opt.emplace (std::move (str)); return opt; vs gdb::optional function () { std::string str; ... return str; The copy/move ctor/assign methods weren't initialy implemented because std::optional supports construction from a type U if U is convertible to T too, and has rules to decide whether the ctors are explicit/implicit based on that, and rules for whether the ctor should be trivial or not, etc., which leads to a much more complicated implementation. If we stick to supporting copy/move construction/assignment of/to an optional from exactly only optional and T, then all that conversion-related complication disappears, and we still gain convenience in most use cases. The patch also makes emplace return a reference to the constructor object, per C++17 std::optional, and adds a reset method, againt because std::optional has one and it's trivial to support it. These two changes are a requirement of the gdb::optional unit testing patch that will follow. gdb/ChangeLog: 2017-04-18 Pedro Alves * common/gdb_optional.h: Include common/traits.h. (in_place_t): New type. (in_place): New constexpr variable. (optional::optional): Remove member initialization of m_instantiated. (optional::optional(in_place_t...)): New constructor. (optional::~optional): Use reset. (optional::optional(const optional&)): New. (optional::optional(const optional&&)): New. (optional::optional(T &)): New. (optional::optional(T &&)): New. (operator::operator=(const optional &)): New. (operator::operator=(optional &&)): New. (operator::operator= (const T &)) (operator::operator= (T &&)) (operator::emplace (Args &&... args)): Return a T&. Use reset. (operator::reset): New. (operator::m_instantiated):: Add in-class initializer. * common/traits.h: Include . (struct And): New types. --- gdb/ChangeLog | 23 ++++++++ gdb/common/gdb_optional.h | 132 ++++++++++++++++++++++++++++++++++++++++------ gdb/common/traits.h | 26 +++++++++ 3 files changed, 166 insertions(+), 15 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 85ab1ae..f000112 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,28 @@ 2017-04-18 Pedro Alves + * common/gdb_optional.h: Include common/traits.h. + (in_place_t): New type. + (in_place): New constexpr variable. + (optional::optional): Remove member initialization of + m_instantiated. + (optional::optional(in_place_t...)): New constructor. + (optional::~optional): Use reset. + (optional::optional(const optional&)): New. + (optional::optional(const optional&&)): New. + (optional::optional(T &)): New. + (optional::optional(T &&)): New. + (operator::operator=(const optional &)): New. + (operator::operator=(optional &&)): New. + (operator::operator= (const T &)) + (operator::operator= (T &&)) + (operator::emplace (Args &&... args)): Return a T&. Use reset. + (operator::reset): New. + (operator::m_instantiated):: Add in-class initializer. + * common/traits.h: Include . + (struct And): New types. + +2017-04-18 Pedro Alves + * xml-support.c: Include . (scope_level::scope_level(const gdb_xml_element *)) (scope_level::scope_level(scope_level&&)): New. diff --git a/gdb/common/gdb_optional.h b/gdb/common/gdb_optional.h index ad1119f..c30fa5b 100644 --- a/gdb/common/gdb_optional.h +++ b/gdb/common/gdb_optional.h @@ -20,46 +20,141 @@ #ifndef GDB_OPTIONAL_H #define GDB_OPTIONAL_H +#include "common/traits.h" + namespace gdb { +struct in_place_t +{ + explicit in_place_t () = default; +}; + +constexpr gdb::in_place_t in_place {}; + /* This class attempts to be a compatible subset of std::optional, which is slated to be available in C++17. This class optionally holds an object of some type -- by default it is constructed not holding an object, but later the object can be "emplaced". This is similar to using std::unique_ptr, but in-object allocation is - guaranteed. */ + guaranteed. + + Unlike std::optional, we currently only support copy/move + construction/assignment of an optional from either exactly + optional or T. I.e., we don't support copy/move + construction/assignment from optional or U, when U is a type + convertible to T. Making that work depending on the definitions of + T and U is somewhat complicated, and currently the users of this + class don't need it. */ + template class optional { public: constexpr optional () - : m_dummy (), - m_instantiated (false) + : m_dummy () + {} + + template + constexpr optional (in_place_t, Args &&... args) + : m_item (std::forward (args)...), + m_instantiated (true) + {} + + ~optional () + { this->reset (); } + + /* Copy and move constructors. */ + + optional (const optional &other) { + if (other.m_instantiated) + this->emplace (other.get ()); } - ~optional () + optional (optional &&other) + noexcept(std::is_nothrow_move_constructible ()) + { + if (other.m_instantiated) + this->emplace (std::move (other.get ())); + } + + constexpr optional (const T &other) + : m_item (other), + m_instantiated (true) + {} + + constexpr optional (T &&other) + noexcept (std::is_nothrow_move_constructible ()) + : m_item (std::move (other)), + m_instantiated (true) + {} + + /* Assignment operators. */ + + optional & + operator= (const optional &other) + { + if (m_instantiated && other.m_instantiated) + this->get () = other.get (); + else + { + if (other.m_instantiated) + this->emplace (other.get ()); + else + this->reset (); + } + + return *this; + } + + optional & + operator= (optional &&other) + noexcept (And, + std::is_nothrow_move_assignable> ()) + { + if (m_instantiated && other.m_instantiated) + this->get () = std::move (other.get ()); + else + { + if (other.m_instantiated) + this->emplace (std::move (other.get ())); + else + this->reset (); + } + return *this; + } + + optional & + operator= (const T &other) { if (m_instantiated) - destroy (); + this->get () = other; + else + this->emplace (other); + return *this; } - /* These aren't deleted in std::optional, but it was simpler to - delete them here, because currently the users of this class don't - need them, and making them depend on the definition of T is - somewhat complicated. */ - optional (const optional &other) = delete; - optional &operator= (const optional &other) = delete; + optional & + operator= (T &&other) + noexcept (And, + std::is_nothrow_move_assignable> ()) + { + if (m_instantiated) + this->get () = std::move (other); + else + this->emplace (std::move (other)); + return *this; + } template - void emplace (Args &&... args) + T &emplace (Args &&... args) { - if (m_instantiated) - destroy (); + this->reset (); new (&m_item) T (std::forward(args)...); m_instantiated = true; + return this->get (); } /* Observers. */ @@ -87,6 +182,13 @@ public: constexpr bool has_value () const noexcept { return m_instantiated; } + /* 'reset' is a 'safe' operation with no precondition. */ + void reset () noexcept + { + if (m_instantiated) + this->destroy (); + } + private: /* Destroy the object. */ @@ -109,7 +211,7 @@ private: }; /* True if the object was ever emplaced. */ - bool m_instantiated; + bool m_instantiated = false; }; } diff --git a/gdb/common/traits.h b/gdb/common/traits.h index 4b7bac3..c8f29cc 100644 --- a/gdb/common/traits.h +++ b/gdb/common/traits.h @@ -18,6 +18,8 @@ #ifndef COMMON_TRAITS_H #define COMMON_TRAITS_H +#include + namespace gdb { /* Pre C++14-safe (CWG 1558) version of C++17's std::void_t. See @@ -29,6 +31,30 @@ struct make_void { typedef void type; }; template using void_t = typename make_void::type; +/* A few trait helpers, mainly stolen from libstdc++. Uppercase + because "and" is a keyword. */ + +template +struct And; + +template<> +struct And<> : public std::true_type +{}; + +template +struct And : public B1 +{}; + +template +struct And + : public std::conditional::type +{}; + +template +struct And + : public std::conditional, B1>::type +{}; + } #endif /* COMMON_TRAITS_H */