From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4360 invoked by alias); 8 Feb 2017 17:28:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 4293 invoked by uid 89); 8 Feb 2017 17:28:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=toolchain, tromey, Tromey, difficulty 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; Wed, 08 Feb 2017 17:28:15 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F3CC57FB75; Wed, 8 Feb 2017 17:28:14 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v18HSDxQ009076; Wed, 8 Feb 2017 12:28:13 -0500 Subject: Re: [RFA 1/5] Remove some ui_out-related cleanups from Python To: Trevor Saunders , Tom Tromey References: <20170115134253.24018-1-tom@tromey.com> <20170115134253.24018-2-tom@tromey.com> <20170116113021.sar3yh5ivykpqmbw@ball> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Wed, 08 Feb 2017 17:28:00 -0000 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: <20170116113021.sar3yh5ivykpqmbw@ball> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-02/txt/msg00197.txt.bz2 On 01/16/2017 11:30 AM, Trevor Saunders wrote: >> +++ b/gdb/common/gdb_option.h > > might be nice to put it in include/ but fine to do that later when > something else actually wants it. I've been thinking about putting all these utilities and later-standards replacements we're coming up with in its own directory/namespace. I had thought of "gtl", for "gdb template library", or "gnu template library" if other projects want to reuse it. (I think "gnu" is kind of taken by gnulib / libgnu.a) One difficulty with putting such a thing at the top level is that gdb relies on gnulib headers (that exist under gdb/) while other toolchain components don't, yet, at least. >> > + /* 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. */ > I think you can make do most of it, but fair enough. > Back around I backported C++17's optional to C++11. I still have that branch somewhere locally... /me finds it and pushes to github. Here: https://github.com/palves/gdb/commits/palves/gdb-import-gcc-optional This contains the local changes I had made to to port it to C++11. I was surprised that other than portable type traits stuff that's missing in C++11, the only thing that C++11 loses is constexpr-ness in some cases. Now, the result is of course much more code than what Tromey is proposing. It probably does make sense to start with something simpler and upgrade when/if we find a need. OTOH, looks like the current patch doesn't have accessors for the wrapped value, so it seems like we'll at least need to be extend it in that direction in no time. >> + /* True if the object was ever emplaced. */ >> + bool m_instantiated; >> + >> + /* The object. */ >> + union >> + { >> + struct { } m_dummy; >> + T m_item; >> + }; > > It doesn't matter yet, but space wise it would be better to put the bool > last right? For example if sizeof(T) is 6, or if the optional is in some > larger structure with a bool next. Agreed. Seems like that's what gcc's optional does. I also agree with Simon's suggestion for renaming the file. Patch LGTM with Trevor's and Simon's nits addressed. I wondered about making m_instantiated a char, so that optional would pack even better when sizeof or alignof T is small, and thus ends up being no cost in those cases space-wise. Though maybe that ends up being worse / not so efficient generated code -wise, or GCC would do it too? In any case, since GCC doesn't do that, if/when we ever move to C++17, we'd lose that again anyway. Thanks, Pedro Alves