From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122730 invoked by alias); 26 Oct 2016 00:41:45 -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 122698 invoked by uid 89); 26 Oct 2016 00:41:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=elt, movable, trade, buried 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, 26 Oct 2016 00:41:33 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (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 C494BC0567A2 for ; Wed, 26 Oct 2016 00:41:32 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9Q0fVYL026348; Tue, 25 Oct 2016 20:41:31 -0400 Subject: Re: C++11 (abridged version) To: Keith Seitz , GDB Patches References: <4300d24a-8711-c5de-79ce-7c530162288c@redhat.com> <580FC09F.6010304@redhat.com> From: Pedro Alves Message-ID: Date: Wed, 26 Oct 2016 00:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <580FC09F.6010304@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00717.txt.bz2 On 10/25/2016 09:29 PM, Keith Seitz wrote: > Hi, Pedro, Hi Keith! Thanks for dialing in. > > I'm normally a "go with the flow" kind of person, and I've pretty much > had my head buried in the compile "sand," but I felt this topic > important enough to say something -- even if it has already been said. > > TL;DR: I'm *all* for moving directly to C++11. I'm already using quite a > bit of it on C++ Compile. How forward-thinking. :-) > However, there is one concern I have, and the recent unique_ptr patch > exemplifies this. We now have a "unique_ptr." In fact, when using a > real C++11 compiler, gdb_unique_ptr.h will actually just alias a real > C++11 std::unique_ptr to gdb::unique_ptr. > > Alas, > >> - Putting gdb::unique_ptr in standard containers is not supported, >> since C++03 containers are not move-aware (and our emulation >> relies on copy actually moving). > > The comment above shows that while the two are similarly named, > gdb::unique_ptr isn't yet really a real unique_ptr. Right, it's a subset of the API. Containers are probably where we actually miss something. Stateful deleters, etc., not so much. > However, for > developers like me who do use a C++11-compliant compiler, that's one > hidden gotcha waiting to be discovered. The problem is really on the C++03 containers side more than anything. There's no way to make C++03 standard containers work correctly with non-copyable and movable types. That's why you can't put old std::auto_ptr in containers either. boost's unique_ptr emulation supports putting their unique_ptr in containers, but only because they re-implement the containers too... I.e., not in standard container either. > > Fortunately, I read the sources and discovered this before I used > unique_ptr in containers on the c++compile branch. I'm (mildly) > concerned that this could too easily lead to yet-another "our own > dialect" of C++(11) scenario(s). Yeah... You just can't do std::vector> m_vec; instead you have to do: /* Vector owns objects. */ std::vector m_vec; and then destroy the vector's elements yourself, usually in the destructor of the class m_vec belongs to. I have examples here (and in other patches of that series): https://sourceware.org/ml/gdb-patches/2016-10/msg00531.html - /* size of array pointed to by expr_list elt. */ - long aexpr_listsize; - long next_aexpr_elt; - struct agent_expr **aexpr_list; + /* Vector owns pointers. */ + std::vector m_aexprs; ... collection_list::~collection_list () { for (int ndx = 0; ndx < m_aexprs.size (); ndx++) free_agent_expr (m_aexprs[ndx]); } So basically you still use gdb::unique_ptr throughout to manage object's lifetime, and then when "moving" the object to a container, you release the object out of the unique_ptr and put the raw pointer in the container. No way out of that with C++03, with any kind of owning smart pointer, not just gdb::unique_ptr. At: https://sourceware.org/ml/gdb-patches/2016-10/msg00537.html see an example of this moving in action: void -collection_list::add_aexpr (struct agent_expr *aexpr) +collection_list::add_aexpr (agent_expr_up aexpr) { - m_aexprs.push_back (aexpr); + m_aexprs.push_back (aexpr.release ()); } Actually strictly speaking that can leak if push_back throws (due to out of memory) A more correct way to write that would be: m_aexprs.push_back (aexpr.get ()); aexpr.release () I didn't bother with that, but maybe I should have. It uglifies the code a bit... I guess we could add a helper template class for that: m_aexprs.push_back (move_to_container (aexpr)); move_to_container would be a function that returns an instance of a class convertible to the raw pointer type, in order to pass the raw pointers to push_back, and, that class would release the smart pointer in its destructor. The temporary's destructor only runs after push_back returns. If we go C++11, the above example can be: std::vector> m_aexprs; the pushing on the vector is then simply: m_aexprs.push_back (std::move (aexpr)); and the collection_list destructor can be completely eliminated.. > > Of course, it is *far* too early to make any concrete assertions about > this. The code has only been in the repository one week. Yeah. I think it's not that bad of a trade off. The container issue is easy to work around, it's still standard C++, and and, a unique_ptr put directly in a container would have to pass code review, while it's very easy to spot. If it does pass review, it's still fixable without any algorithmic redesign. You just have to do more things manually. > >> #3 - whether we should instead switch to require a C++11 compiler >> > > We gotta start somewhere, and IMO, C++11 is *by far* the best "safe" > place to do so. > >> #4 - if so, then what is the policy to accept the next standard >> versions going forward. > > I don't know how comfortable I am with such black-and-white rules, but I > think/hope that maintainers remain *flexible* about decisions like this. > Do what is "right" when it is "right." [That's not a "rule," per se, but > what I'd like the "spirit" of the rule to be.] > > I think your offering on this policy indicates that you and I are in > agreement. *nod* > > I'm already using C++11 features, and I am absolutely in favor of moving > directly to C++11. > > TL;SW* > > Keith > > * Too Long; Stopped Writing :-) > Thanks, Pedro Alves