From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61670 invoked by alias); 11 Oct 2016 13:46:07 -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 61657 invoked by uid 89); 11 Oct 2016 13:46:06 -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,T_FILL_THIS_FORM_SHORT autolearn=no version=3.3.2 spammy=7.1, pasted, buys 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 13:45:56 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 9985D3F216; Tue, 11 Oct 2016 13:45:55 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9BDjqNI010012; Tue, 11 Oct 2016 09:45:54 -0400 Subject: Re: [PATCH 1/3] Introduce gdb::unique_ptr To: Joel Brobecker References: <1476117992-5689-1-git-send-email-palves@redhat.com> <1476117992-5689-2-git-send-email-palves@redhat.com> <20161011121639.GE3813@adacore.com> Cc: "Metzger, Markus T" , "gdb-patches@sourceware.org" From: Pedro Alves Message-ID: <68fc02cb-59bc-012c-d1be-b5ed2076d6a5@redhat.com> Date: Tue, 11 Oct 2016 13:46: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: <20161011121639.GE3813@adacore.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00252.txt.bz2 On 10/11/2016 01:16 PM, Joel Brobecker wrote: >> 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? > > FWIW, I think it is fine to require C++11 if there are sufficient > benefits we can derive from it. This seems like a good example > of something we could simplify greatly if we did? In the particular issue of an owning smart pointer, I think the shim as I'm proposing buys us some time, and unblocks a lot more struct cleanup elimination and simplification of the codebase. I'd still propose going forward with it immediately. It's really not a complicated template, IMO. I've removed generic comments that basically were explaining how all smart pointers behave anyway, and inlined the safe bool idiom into gdb::unique_ptr directly, and removed the C++11 bits, and the result is only around 250 lines of still-commented code. See patch pasted below (just for reference). Of course, this version misses the benefit of using C++11 when available as a "lint", so it's as "unsafe" as std::auto_ptr, because it _is_ actually std::auto_ptr on steroids. Going full C++11 would certainly give us good benefits. It's a much cleaner language overall. Beyond the benefits mentioned above, there's also std::unordered_map as standard type-safe hash table and other containers. Lambda expressions probably came in handy in many places. Likewise std::function. variadic templates would help simplify template code we might want to add -- see Tromey's observers-in-C++ series. For gcc's older than 6.1, we'd need to force-enable c++11 mode, with -std=gnu++11/-std=gnu++0x, and I think that needs to be done by the top level configure, because CXX is passed down to subdirs. Since the top level is shared with gcc, that may not be trivial. It gets particularly more interesting if different project subdirs have different ideas of level of C++11 support they tolerate. So a staged approach would do the "prefer C++11-if-available" first (w/ -std=...), in order to make use of C++11 features when possible and make sure virtually everyone developing gdb is using the "safer" gdb::unique_ptr version. Once that works and is all in place and breaks nothing else, switching to "require C++11", if designed right, should then be an easy change. > > Note that I wouldn't necessarily think in purely in terms of which > version of GCC supports it, but also consider whether want to support > building GDB with non-GCC compilers, particularly on the more exotic > systems out there, where it can be hard to build GDB. Do all these > compilers support C++11? Probably not. Looking at: https://sourceware.org/gdb/wiki/Systems I think we're pretty much down to not-that-exotic hosts nowadays. At least, all hosts there seem to me like should have working gcc or clang ports. The AIX system on the gcc compile farm has gcc 4.8.1 (AIX 7.1). For BSDs, there's clang and gcc. Of course old BSDs won't have new gcc/clang, you'd have to build a new compiler. But that's the same as old GNU/Linux distros. FWIW, building clang/llvm/lldb requires a C++11 compiler too. > This is where it becomes > a judgment call for me. Making it easy to build with other compilers > by limiting which parts of the language we can use helps getting wider > use of GDB; but on the other hand, if it comes at too much of a cost > in terms of the code and its maintenance, then it seems better overall > to increase our list of requirements. Thanks, Pedro Alves #ifndef GDB_UNIQUE_PTR_H #define GDB_UNIQUE_PTR_H 1 namespace gdb { /* Default destruction policy used by gdb::unique_ptr when no deleter is specified. Uses delete. */ template struct default_delete { void operator () (T *ptr) const { delete ptr; } }; /* Specialization for arrays. Uses delete[]. */ template struct default_delete { void operator () (T *ptr) const { delete [] ptr; } }; /* Type used to support assignment from NULL: gdb::unique_ptr ptr (....); ... ptr = NULL; */ struct unique_ptr_nullptr_t { private: struct private_type; public: /* Since null_type is private, the only way to construct this class is by passing a NULL pointer. See unique_ptr_base::operator= further below. */ unique_ptr_nullptr_t (private_type *) {} }; /* Base class of our unique_ptr emulation. Contains code common to both the unique_ptr and unique_ptr. */ template class unique_ptr_base { public: typedef T *pointer; typedef T element_type; typedef D deleter_type; template struct unique_ptr_base_ref { T1 *m_ptr; explicit unique_ptr_base_ref (T1 *p): m_ptr (p) {} }; typedef unique_ptr_base_ref ref_type; explicit unique_ptr_base (element_type *p = NULL) throw() : m_ptr (p) {} /* Even though std::unique_ptr is not copyable, our little simpler emulation allows it (behaves like std::auto_ptr), because RVO/NRVO requires an accessible copy constructor, and also because our move emulation relies on this. */ unique_ptr_base (unique_ptr_base& a) throw() : m_ptr (a.release ()) {} /* Assignment operator. Actually moves. We implement this simply to allow std::swap work without having to provide our own swap implementation. We know that if GDB compiles with real std::unique_ptr, this won't be called "incorrectly". */ unique_ptr_base & operator= (unique_ptr_base &a) throw() { reset (a.release ()); return *this; } /* std::unique_ptr does not allow assignment, except from nullptr. nullptr doesn't exist before C++11, so we allowing assignment from NULL instead: ptr = NULL; */ unique_ptr_base & operator= (const unique_ptr_nullptr_t &) throw() { reset (); return *this; } ~unique_ptr_base () { call_deleter (); } /* "explicit operator bool ()" emulation using the safe bool idiom. */ private: typedef void (unique_ptr_base::*bool_type) () const; void this_type_does_not_support_comparisons () const {} public: operator bool_type () const { return (m_ptr != NULL ? &unique_ptr_base::this_type_does_not_support_comparisons : 0); } element_type *get () const throw() { return m_ptr; } element_type *release () throw() { pointer tmp = m_ptr; m_ptr = NULL; return tmp; } void reset (element_type *p = NULL) throw() { if (p != m_ptr) { call_deleter (); m_ptr = p; } } /* Automatic conversions. These operations convert an unique_ptr_base into and from an unique_ptr_base_ref automatically as needed. This allows constructs such as: unique_ptr func_returning_unique_ptr (.....); ... unique_ptr ptr = func_returning_unique_ptr (.....); */ unique_ptr_base (unique_ptr_base_ref ref) throw() : m_ptr (ref.m_ptr) {} unique_ptr_base & operator= (unique_ptr_base_ref ref) throw() { if (ref.m_ptr != this->get()) { call_deleter (); m_ptr = ref.m_ptr; } return *this; } template operator unique_ptr_base_ref () throw() { return unique_ptr_base_ref (this->release ()); } template operator unique_ptr_base () throw() { return unique_ptr_base (this->release ()); } private: /* Call the deleter. Note we assume the deleter is "stateless". */ void call_deleter () { D d; d (m_ptr); } element_type *m_ptr; }; /* Macro used to create a unique_ptr_base "specialization" -- a subclass that uses a specific deleter. Basically this re-defines the necessary constructors. This is necessary because we can't inherit constructors with "using" without C++11. While at it, we inherit the assignment operator. TYPE is the name of the type being defined. Assumes that 'base_type' is a typedef of the baseclass UNIQUE_PTR is inheriting from. */ #define DEFINE_UNIQUE_PTR(TYPE) \ public: \ explicit TYPE (T *p = NULL) throw() \ : base_type (p) {} \ TYPE (typename base_type::ref_type ref) throw() \ : base_type (ref.m_ptr) {} \ \ using base_type::operator=; /* Define non-array gdb::unique_ptr. */ template > class unique_ptr : public unique_ptr_base { typedef unique_ptr_base base_type; DEFINE_UNIQUE_PTR (unique_ptr) /* Dereferencing. */ T &operator* () const throw() { return *this->get (); } T *operator-> () const throw() { return this->get (); } }; /* gdb::unique_ptr specialization for T[]. */ template class unique_ptr : public unique_ptr_base { typedef unique_ptr_base base_type; DEFINE_UNIQUE_PTR (unique_ptr) /* Indexing operator. */ T& operator[] (size_t i) const { return this->get ()[i]; } }; /* Comparison operators. */ template inline bool operator== (const unique_ptr_base& x, const unique_ptr_base& y) { return x.get() == y.get(); } template inline bool operator!= (const unique_ptr_base& x, const unique_ptr_base& y) { return x.get() != y.get(); } /* std::move "emulation". This is as simple as it can be -- relies on the fact that our std::unique_ptr emulation actually behaves like std::auto_ptr -- copy/assignment actually moves. */ template unique_ptr_base move (unique_ptr_base v) { return v; } /* Define gdb::unique_malloc_ptr, a gdb::unique_ptr that manages malloc'ed memory. */ /* The deleter for gdb::unique_malloc_ptr. Uses xfree. */ template struct xfree_deleter { void operator() (T *ptr) const { xfree (ptr); } }; /* In C++03, we don't have template aliases, so we need to define a subclass instead (and re-define the constructors, because we don't have inheriting constructors either). */ template class unique_malloc_ptr : public unique_ptr > { typedef unique_ptr > base_type; DEFINE_UNIQUE_PTR (unique_malloc_ptr) }; } /* namespace gdb */ #endif /* GDB_UNIQUE_PTR_H */