From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 52247 invoked by alias); 11 Oct 2016 10:23:42 -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 51742 invoked by uid 89); 11 Oct 2016 10:23:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=bake, theirs, clarifies, Uses 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, 11 Oct 2016 10:23:31 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (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 5BCD3C05490A; Tue, 11 Oct 2016 10:23:30 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9BANSZ3005365; Tue, 11 Oct 2016 06:23:29 -0400 Subject: Re: [PATCH 1/3] Introduce gdb::unique_ptr To: "Metzger, Markus T" , "gdb-patches@sourceware.org" References: <1476117992-5689-1-git-send-email-palves@redhat.com> <1476117992-5689-2-git-send-email-palves@redhat.com> From: Pedro Alves Message-ID: Date: Tue, 11 Oct 2016 10:23: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: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00244.txt.bz2 On 10/11/2016 07:47 AM, Metzger, Markus T wrote: >> -----Original Message----- >> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- >> owner@sourceware.org] On Behalf Of Pedro Alves >> Sent: Monday, October 10, 2016 6:47 PM >> To: gdb-patches@sourceware.org >> Subject: [PATCH 1/3] Introduce gdb::unique_ptr > > Hello Pedro, Hi Markus, > >> Many make_cleanup uses in the code base are best eliminated by using a >> "owning" smart pointer to manage ownership of the resource >> automatically. >> >> The question is _which_ smart pointer. >> >> We have std::auto_ptr in C++03, but, as is collective wisdom by now, >> that's too easy to misuse, and has therefore been deprecated in C++11 >> and finally removed in C++17. > > Would it make sense to switch to C++11, instead? I think it would make a lot of sense to switch to C++11. I'd love that. rvalue references support, move-aware containers, "auto", std::unique_ptr and std::shared_ptr would be all very nice to have. The only question in my mind is -- are people OK with requiring gcc 4.8 or later? I think gcc 4.8 or newer were available in Fedora 20. I believe Ubuntu 12.04 had it available as an option. On older RHEL systems, it's available in DTS. On older systems, you'd need to compile a newer gcc or clang first before building gdb. Would people find that acceptable? Or put another way, would anyone find that unacceptable? But meanwhile, I don't want C++ conversion to be blocked by that. Hence this new shim. Even if we don't require C++11, I think it'd make sense to compile in C++11 mode if we have a capable compiler. I.e., if you have gcc 4.8 or newer, pass -std=gnu++11/-std=gnu++0x if necessary. > Or delay this work until we can make this move? No, I don't think so. I think we need _some_ owning smart pointer. An alternative is some separate RAII object that destroys a pointer when leaving a scope: template struct cleanup_pointer { explicit cleanup_pointer(T *p) : m_ptr (p) {} cleanup_pointer { if (m_ptr != NULL) delete m_ptr; } void discard () { m_ptr = NULL; } T *m_ptr; }; // Document that this returns a new object that must // be released with "delete". struct foo *function_returning_new_ptr (); // Document that this uses P, ownership is still with the caller. // May throw. void foo (struct foo *p); // Document that P is no longer owned by the caller after this. // May throw. void bar (struct foo *p); void some_function () { struct foo *ptr = function_returning_new_ptr (); cleanup_pointer cleanup (ptr); ptr->foo = .....; bar (ptr); // whoops, bar now owns the pointer, but // cleanup_pointer will delete it anyway // once we leave this scope // would need to remember to call this. // cleanup.discard (); } That looks very much like our current cleanups, except we no longer need the "do_cleanups" call. But it's far from idiomatic C++. And, ends up being more code to write/maintain than using a smart pointer. We can do better. E.g.,: // (gdb::unique_ptr is like std::unique_ptr and // gdb::move is like std::move for gdb::unique_ptr) // prototype clearly tells you this returns a new object. gdb::unique_ptr function_returning_new_ptr (); // Uses P, ownership is still with the caller. void foo (struct foo *p); // Clearly tells you this transfers ownership. void bar (gdb::unique_ptr p); void some_function () { // struct foo *ptr = function_returning_new_ptr (); // would not compile. gdb::unique_ptr ptr = function_returning_new_ptr (); // OK, ownership clearly transferred to caller. ptr->foo = .....; foo (ptr.get ()); // clearly foo borrows the pointer. bar (gdb::move (ptr)); // clearly transfers ownership. } The other use case a smart pointer clarifies is making a structure/class be the owner of some pointer. Like: struct foo { gdb::unique_ptr ptr; }; When a "foo" is destroyed, then ptr is destroyed automatically as well. Without the smart pointer, we'd have to explicitly write delete ptr in a destructor: struct foo { ~foo() { delete ptr; } struct foo *ptr; }; Most C++03 projects past a size/history will bake their own custom smart pointers for the use cases above. Most large projects will have invented theirs before std::unique_ptr existed though, so they'll differ in API details. However in our case, std::unique_ptr is already there, so I think it makes a lot of sense to reuse the same API, and take advantage of newer compilers as "lints", since that's what people will be using for development anyway. Once we do require C++11, then we won't have to relearn a new smart pointer API. Even if we were to use std::auto_ptr only, I would still think that that would be an improvement over raw pointers, since with raw pointers it's even easier to confuse "borrowing pointers" from "owning pointers", since you have 0 zero help from the type system. The potential problems with std::auto_ptr stem from its "move semantics", and are of the form: std::auto_ptr ptr = function_returning_type (); std::auto_ptr ptr2 = ptr; // this moves instead of // copying. I.e., ptr is NULL after this. and: void function (std::auto_ptr ptr); function (ptr); // This moves instead of copying. ptr is NULL after this. ptr->foo = 1; // crash. In the latter case, the function prototype indicates that the caller wants to transfer ownership of the pointer to the callee. But that's not clear at the call site. So in C++03 mode, gdb::unique_ptr "allows" that too. I.e., the compiler won't complain. However, that _won't_ compile in C++11 mode, forcing you to write instead: gdb::unique_ptr ptr2 = gdb::move (ptr); // OK, now move is clearly // what was intended. function (gdb::move (ptr2)); // Likewise. ptr2->foo = 1; // crash, but, obviously incorrect. And of course the C++03 gdb::unique_ptr version supports this as well. There's also the advantage that gdb::unique_ptr works with malloced objects, and arrays (for 'new some_type[];'). We'd need _some_ smart pointer for those, so std::auto_ptr alone won't be sufficient anyway... WDYT? Thanks, Pedro Alves