This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] Introduce gdb::unique_ptr


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<typename T>
struct default_delete
{
  void operator () (T *ptr) const { delete ptr; }
};

/* Specialization for arrays.  Uses delete[].  */

template<typename T>
struct default_delete<T[]>
{
  void operator () (T *ptr) const { delete [] ptr; }
};

/* Type used to support assignment from NULL:

     gdb::unique_ptr<foo> 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<T, D> and unique_ptr<T[], D>.  */

template<typename T, typename D>
class unique_ptr_base
{
public:
  typedef T *pointer;
  typedef T element_type;
  typedef D deleter_type;

  template <typename T1>
  struct unique_ptr_base_ref
  {
    T1 *m_ptr;

    explicit unique_ptr_base_ref (T1 *p): m_ptr (p) {}
  };

  typedef unique_ptr_base_ref<T> 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<Derived>  func_returning_unique_ptr (.....);
      ...
      unique_ptr<Base> ptr = func_returning_unique_ptr (.....);
   */
  unique_ptr_base (unique_ptr_base_ref<element_type> ref) throw()
    : m_ptr (ref.m_ptr) {}

  unique_ptr_base &
  operator= (unique_ptr_base_ref<element_type> ref) throw()
  {
    if (ref.m_ptr != this->get())
      {
	call_deleter ();
	m_ptr = ref.m_ptr;
      }
    return *this;
  }

  template<typename T1>
  operator unique_ptr_base_ref<T1> () throw()
  { return unique_ptr_base_ref<T1> (this->release ()); }

  template<typename T1, typename D1>
  operator unique_ptr_base<T1, D1> () throw()
  { return unique_ptr_base<T1, D1> (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 <typename T, typename D = default_delete<T> >
class unique_ptr : public unique_ptr_base<T, D>
{
  typedef unique_ptr_base<T, D> 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 <typename T, typename D>
class unique_ptr<T[], D> : public unique_ptr_base<T, D>
{
  typedef unique_ptr_base<T, D> base_type;

  DEFINE_UNIQUE_PTR (unique_ptr)

  /* Indexing operator.  */
  T& operator[] (size_t i) const
  { return this->get ()[i]; }
};

/* Comparison operators.  */

template <typename T, typename D,
	  typename U, typename E>
inline bool
operator== (const unique_ptr_base<T, D>& x,
	    const unique_ptr_base<U, E>& y)
{ return x.get() == y.get(); }

template <typename T, typename D,
	  typename U, typename E>
inline bool
operator!= (const unique_ptr_base<T, D>& x,
	    const unique_ptr_base<U, E>& 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<typename T, typename D>
unique_ptr_base<T, D>
move (unique_ptr_base<T, D> 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 <typename T>
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 <typename T>
class unique_malloc_ptr : public unique_ptr<T, xfree_deleter<T> >
{
  typedef unique_ptr<T, xfree_deleter<T> > base_type;

  DEFINE_UNIQUE_PTR (unique_malloc_ptr)
};

} /* namespace gdb */

#endif /* GDB_UNIQUE_PTR_H */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]