public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/2] use unique_ptr some
  2017-07-31 23:46 [PATCH 0/2] add unique_ptr class tbsaunde+gcc
  2017-07-31 23:46 ` [PATCH 1/2] add unique_ptr header tbsaunde+gcc
@ 2017-07-31 23:46 ` tbsaunde+gcc
  2017-08-01  0:51 ` [PATCH 0/2] add unique_ptr class David Malcolm
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 34+ messages in thread
From: tbsaunde+gcc @ 2017-07-31 23:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: palves

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc/ChangeLog:

2017-07-31  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* cse.c (find_comparison_args): Make visited a unique_ptr.
	* lto-streamer-out.c (write_global_references): Make data a
	unique_ptr.
	* tree-cfg.c (move_sese_region_to_fn): Make several variables
	unique_ptrs.
---
 gcc/cse.c              |  7 +++----
 gcc/lto-streamer-out.c |  6 +++---
 gcc/tree-cfg.c         | 38 +++++++++++++-------------------------
 3 files changed, 19 insertions(+), 32 deletions(-)

diff --git a/gcc/cse.c b/gcc/cse.c
index 6a968d19788..45da9b2da9d 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "unique-ptr.h"
 #include "backend.h"
 #include "target.h"
 #include "rtl.h"
@@ -2887,7 +2888,7 @@ find_comparison_args (enum rtx_code code, rtx *parg1, rtx *parg2,
 		      machine_mode *pmode1, machine_mode *pmode2)
 {
   rtx arg1, arg2;
-  hash_set<rtx> *visited = NULL;
+  gtl::unique_ptr<hash_set<rtx> > visited;
   /* Set nonzero when we find something of interest.  */
   rtx x = NULL;
 
@@ -2904,7 +2905,7 @@ find_comparison_args (enum rtx_code code, rtx *parg1, rtx *parg2,
       if (x)
 	{
 	  if (!visited)
-	    visited = new hash_set<rtx>;
+	    visited.reset (new hash_set<rtx>);
 	  visited->add (x);
 	  x = 0;
 	}
@@ -3067,8 +3068,6 @@ find_comparison_args (enum rtx_code code, rtx *parg1, rtx *parg2,
   *pmode1 = GET_MODE (arg1), *pmode2 = GET_MODE (arg2);
   *parg1 = fold_rtx (arg1, 0), *parg2 = fold_rtx (arg2, 0);
 
-  if (visited)
-    delete visited;
   return code;
 }
 \f
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 41fba318cb5..61576c30266 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "unique-ptr.h"
 #include "backend.h"
 #include "target.h"
 #include "rtl.h"
@@ -2434,7 +2435,7 @@ write_global_references (struct output_block *ob,
   const uint32_t size = lto_tree_ref_encoder_size (encoder);
 
   /* Write size and slot indexes as 32-bit unsigned numbers. */
-  uint32_t *data = XNEWVEC (uint32_t, size + 1);
+  gtl::unique_ptr<uint32_t[]> data (new uint32_t[size + 1]);
   data[0] = size;
 
   for (index = 0; index < size; index++)
@@ -2447,8 +2448,7 @@ write_global_references (struct output_block *ob,
       data[index + 1] = slot_num;
     }
 
-  lto_write_data (data, sizeof (int32_t) * (size + 1));
-  free (data);
+  lto_write_data (data.get (), sizeof (int32_t) * (size + 1));
 }
 
 
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 733c92fcdd0..604f799926c 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "unique-ptr.h"
 #include "backend.h"
 #include "target.h"
 #include "rtl.h"
@@ -7252,10 +7253,8 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,
 {
   vec<basic_block> bbs, dom_bbs;
   basic_block dom_entry = get_immediate_dominator (CDI_DOMINATORS, entry_bb);
-  basic_block after, bb, *entry_pred, *exit_succ, abb;
+  basic_block after, bb, abb;
   struct function *saved_cfun = cfun;
-  int *entry_flag, *exit_flag;
-  profile_probability *entry_prob, *exit_prob;
   unsigned i, num_entry_edges, num_exit_edges, num_nodes;
   edge e;
   edge_iterator ei;
@@ -7291,9 +7290,10 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,
      EXIT_BB so that we can re-attach them to the new basic block that
      will replace the region.  */
   num_entry_edges = EDGE_COUNT (entry_bb->preds);
-  entry_pred = XNEWVEC (basic_block, num_entry_edges);
-  entry_flag = XNEWVEC (int, num_entry_edges);
-  entry_prob = XNEWVEC (profile_probability, num_entry_edges);
+  gtl::unique_ptr<basic_block[]> entry_pred (new basic_block[num_entry_edges]);
+  gtl::unique_ptr<int[]> entry_flag (new int[num_entry_edges]);
+  gtl::unique_ptr<profile_probability[]> entry_prob
+    (new profile_probability[num_entry_edges]);
   i = 0;
   for (ei = ei_start (entry_bb->preds); (e = ei_safe_edge (ei)) != NULL;)
     {
@@ -7303,12 +7303,15 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,
       remove_edge (e);
     }
 
+  gtl::unique_ptr<basic_block[]> exit_succ;
+  gtl::unique_ptr<int[]> exit_flag;
+  gtl::unique_ptr<profile_probability[]> exit_prob;
   if (exit_bb)
     {
       num_exit_edges = EDGE_COUNT (exit_bb->succs);
-      exit_succ = XNEWVEC (basic_block, num_exit_edges);
-      exit_flag = XNEWVEC (int, num_exit_edges);
-      exit_prob = XNEWVEC (profile_probability, num_exit_edges);
+      exit_succ.reset (new basic_block[num_exit_edges]);
+      exit_flag.reset (new int[num_exit_edges]);
+      exit_prob.reset (new profile_probability[num_exit_edges]);
       i = 0;
       for (ei = ei_start (exit_bb->succs); (e = ei_safe_edge (ei)) != NULL;)
 	{
@@ -7319,12 +7322,7 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,
 	}
     }
   else
-    {
-      num_exit_edges = 0;
-      exit_succ = NULL;
-      exit_flag = NULL;
-      exit_prob = NULL;
-    }
+    num_exit_edges = 0;
 
   /* Switch context to the child function to initialize DEST_FN's CFG.  */
   gcc_assert (dest_cfun->cfg == NULL);
@@ -7534,16 +7532,6 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,
   FOR_EACH_VEC_ELT (dom_bbs, i, abb)
     set_immediate_dominator (CDI_DOMINATORS, abb, bb);
   dom_bbs.release ();
-
-  if (exit_bb)
-    {
-      free (exit_prob);
-      free (exit_flag);
-      free (exit_succ);
-    }
-  free (entry_prob);
-  free (entry_flag);
-  free (entry_pred);
   bbs.release ();
 
   return bb;
-- 
2.11.0

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/2] add unique_ptr header
  2017-07-31 23:46 [PATCH 0/2] add unique_ptr class tbsaunde+gcc
@ 2017-07-31 23:46 ` tbsaunde+gcc
  2017-08-01 14:40   ` David Malcolm
  2017-07-31 23:46 ` [PATCH 2/2] use unique_ptr some tbsaunde+gcc
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 34+ messages in thread
From: tbsaunde+gcc @ 2017-07-31 23:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: palves

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

For most of the history of this see 
https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
The changes are mostly s/gdb/gtl/g

include/ChangeLog:

2017-07-29  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* unique-ptr.h: New file.
---
 include/unique-ptr.h | 386 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 386 insertions(+)
 create mode 100644 include/unique-ptr.h

diff --git a/include/unique-ptr.h b/include/unique-ptr.h
new file mode 100644
index 00000000000..7903a5abefe
--- /dev/null
+++ b/include/unique-ptr.h
@@ -0,0 +1,386 @@
+/* gtl::unique_ptr, a simple std::unique_ptr replacement for C++03.
+
+   Copyright (C) 2007-2016 Free Software Foundation, Inc.
+
+   This file is part of GCC.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* gtl::unique_ptr defines a C++ owning smart pointer that exposes a
+   subset of the std::unique_ptr API.
+
+   In fact, when compiled with a C++11 compiler, gtl::unique_ptr
+   actually _is_ std::unique_ptr.  When compiled with a C++03 compiler
+   OTOH, it's an hand coded std::unique_ptr emulation that assumes
+   code is correct and doesn't try to be too smart.
+
+   This supports custom deleters, but not _stateful_ deleters, so you
+   can't use those in C++11 mode either.  Only the managed pointer is
+   stored in the smart pointer.  That could be changed; it simply
+   wasn't found necessary.
+
+   At the end of the file you'll find a gtl::unique_ptr partial
+   specialization that uses a custom (stateless) deleter:
+   gtl::unique_xmalloc_ptr.  That is used to manage pointers to
+   objects allocated with xmalloc.
+
+   The C++03 version was originally based on GCC 7.0's std::auto_ptr
+   and then heavily customized to behave more like C++11's
+   std::unique_ptr, but at this point, it no longer shares much at all
+   with the original file.  But, that's the history and the reason for
+   the copyright's starting year.
+
+   The C++03 version lets you shoot yourself in the foot, since
+   similarly to std::auto_ptr, the copy constructor and assignment
+   operators actually move.  Also, in the name of simplicity, no
+   effort is spent on using SFINAE to prevent invalid conversions,
+   etc.  This is not really a problem, because the goal here is to
+   allow code that would be correct using std::unique_ptr to be
+   equally correct in C++03 mode, and, just as efficient.  If client
+   code compiles correctly with a C++11 (or newer) compiler, we know
+   we're not doing anything invalid by mistake.
+
+   Usage notes:
+
+   - Putting gtl::unique_ptr in standard containers is not supported,
+     since C++03 containers are not move-aware (and our emulation
+     relies on copy actually moving).
+
+   - Since there's no nullptr in C++03, gtl::unique_ptr allows
+     implicit initialization and assignment from NULL instead.
+
+   - To check whether there's an associated managed object, all these
+     work as expected:
+
+      if (ptr)
+      if (!ptr)
+      if (ptr != NULL)
+      if (ptr == NULL)
+      if (NULL != ptr)
+      if (NULL == ptr)
+*/
+
+#ifndef GTL_UNIQUE_PTR_H
+#define GTL_UNIQUE_PTR_H 1
+
+#include <memory>
+
+namespace gtl
+{
+
+#if __cplusplus >= 201103
+
+/* In C++11 mode, all we need is import the standard
+   std::unique_ptr.  */
+template<typename T> using unique_ptr = std::unique_ptr<T>;
+
+/* Pull in move as well.  */
+using std::move;
+
+#else /* C++11 */
+
+/* Default destruction policy used by gtl::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; }
+};
+
+namespace detail
+{
+/* Type used to support implicit construction from NULL:
+
+     gtl::unique_ptr<foo> func (....)
+     {
+     return NULL;
+     }
+
+   and assignment from NULL:
+
+     gtl::unique_ptr<foo> ptr (....);
+     ...
+     ptr = NULL;
+
+  It is intentionally not defined anywhere.  */
+struct nullptr_t;
+
+/* Base class of our unique_ptr emulation.  Contains code common to
+   both 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;
+
+  /* Takes ownership of a pointer.  P is a pointer to an object of
+     element_type type.  Defaults to NULL.  */
+  explicit unique_ptr_base (element_type *p = NULL) throw () : m_ptr (p) {}
+
+  /* The "move" constructor.  Really a copy constructor that actually
+     moves.  Even though std::unique_ptr is not copyable, our little
+     simpler emulation allows it, because:
+
+       - There are no rvalue references in C++03.  Our move emulation
+       instead relies on copy/assignment moving, like std::auto_ptr.
+       - RVO/NRVO requires an accessible copy constructor
+  */
+  unique_ptr_base (const unique_ptr_base &other) throw ()
+    : m_ptr (const_cast<unique_ptr_base &> (other).release ()) {}
+
+  /* Converting "move" constructor.  Really an lvalue ref converting
+     constructor that actually moves.  This allows constructs such as:
+
+      unique_ptr<Derived> func_returning_unique_ptr (.....);
+      ...
+      unique_ptr<Base> ptr = func_returning_unique_ptr (.....);
+  */
+  template<typename T1, typename D1>
+  unique_ptr_base (const unique_ptr_base<T1, D1> &other) throw ()
+    : m_ptr (const_cast<unique_ptr_base<T1, D1> &> (other).release ()) {}
+
+  /* The "move" assignment operator.  Really an lvalue ref copy
+     assignment operator that actually moves.  See comments above.  */
+  unique_ptr_base &operator= (const unique_ptr_base &other) throw ()
+  {
+    reset (const_cast<unique_ptr_base &> (other).release ());
+    return *this;
+  }
+
+  /* Converting "move" assignment.  Really an lvalue ref converting
+     copy assignment operator that moves.  See comments above.  */
+  template<typename T1, typename D1>
+  unique_ptr_base &operator= (const unique_ptr_base<T1, D1> &other) throw ()
+  {
+    reset (const_cast<unique_ptr_base<T1, D1> &> (other).release ());
+    return *this;
+  }
+
+  /* std::unique_ptr does not allow assignment, except from nullptr.
+     nullptr doesn't exist in C++03, so we allow assignment from NULL
+     instead [ptr = NULL;].
+  */
+  unique_ptr_base &operator= (detail::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::*explicit_operator_bool) () const;
+  void this_type_does_not_support_comparisons () const {}
+
+public:
+  operator explicit_operator_bool () 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;
+      }
+  }
+
+private:
+
+  /* Call the deleter.  Note we assume the deleter is "stateless".  */
+  void call_deleter ()
+  {
+    D d;
+
+    d (m_ptr);
+  }
+
+  element_type *m_ptr;
+};
+
+} /* namespace detail */
+
+/* Macro used to create a unique_ptr_base "partial specialization" --
+   a subclass that uses a specific deleter.  Basically this re-defines
+   the necessary constructors.  This is necessary because C++03
+   doesn't support inheriting constructors with "using".  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 TYPE is inheriting from.  */
+#define DEFINE_GDB_UNIQUE_PTR(TYPE)						\
+public:									\
+  explicit TYPE (T *p = NULL) throw ()					\
+    : base_type (p) {}							\
+									\
+  TYPE (const TYPE &other) throw () : base_type (other) {}		\
+									\
+  TYPE (detail::nullptr_t *) throw () : base_type (NULL) {}		\
+									\
+  template<typename T1, typename D1>					\
+  TYPE (const detail::unique_ptr_base<T1, D1> &other) throw ()		\
+    : base_type (other) {}						\
+									\
+  using base_type::operator=;
+
+/* Define single-object gtl::unique_ptr.  */
+
+template <typename T, typename D = default_delete<T> >
+class unique_ptr : public detail::unique_ptr_base<T, D>
+{
+  typedef detail::unique_ptr_base<T, D> base_type;
+
+  DEFINE_GDB_UNIQUE_PTR (unique_ptr)
+
+public:
+  /* Dereferencing.  */
+  T &operator* () const throw () { return *this->get (); }
+  T *operator-> () const throw () { return this->get (); }
+};
+
+/* Define gtl::unique_ptr specialization for T[].  */
+
+template <typename T, typename D>
+class unique_ptr<T[], D> : public detail::unique_ptr_base<T, D>
+{
+  typedef detail::unique_ptr_base<T, D> base_type;
+
+  DEFINE_GDB_UNIQUE_PTR (unique_ptr)
+
+public:
+  /* 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 detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return x.get() == y.get(); }
+
+template <typename T, typename D,
+	  typename U, typename E>
+inline bool
+operator!= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return x.get() != y.get(); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator< (const detail::unique_ptr_base<T, D> &x,
+	   const detail::unique_ptr_base<U, E> &y)
+{ return x.get() < y.get (); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator<= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return !(y < x); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator> (const detail::unique_ptr_base<T, D> &x,
+	   const detail::unique_ptr_base<U, E> &y)
+{ return y < x; }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator>= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return !(x < y); }
+
+/* std::move "emulation".  This is as simple as it can be -- no
+   attempt is made to emulate rvalue references.  Instead relies on
+   the fact that gtl::unique_ptr has move semantics like
+   std::auto_ptr.  I.e., copy/assignment actually moves.  */
+
+template<typename T, typename D>
+unique_ptr<T, D>
+move (unique_ptr<T, D> v)
+{
+  return v;
+}
+
+#endif /* C++11 */
+
+/* Define gtl::unique_xmalloc_ptr, a gtl::unique_ptr that manages
+   xmalloc'ed memory.  */
+
+/* The deleter for gtl::unique_xmalloc_ptr.  Uses xfree.  */
+template <typename T>
+struct xfree_deleter
+{
+  void operator() (T *ptr) const { xfree (ptr); }
+};
+
+#if __cplusplus >= 201103
+
+/* In C++11, we just import the standard unique_ptr to our namespace
+   with a custom deleter.  */
+
+template<typename T> using unique_xmalloc_ptr
+  = std::unique_ptr<T, xfree_deleter<T>>;
+
+#else /* C++11 */
+
+/* In C++03, we don't have template aliases, so we need to define a
+   subclass instead, and re-define the constructors, because C++03
+   doesn't support inheriting constructors either.  */
+
+template <typename T>
+class unique_xmalloc_ptr : public unique_ptr<T, xfree_deleter<T> >
+{
+  typedef unique_ptr<T, xfree_deleter<T> > base_type;
+
+  DEFINE_GDB_UNIQUE_PTR (unique_xmalloc_ptr)
+};
+
+#endif /* C++11 */
+
+} /* namespace gtl */
+
+#endif /* GTL_UNIQUE_PTR_H */
-- 
2.11.0

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 0/2] add unique_ptr class
@ 2017-07-31 23:46 tbsaunde+gcc
  2017-07-31 23:46 ` [PATCH 1/2] add unique_ptr header tbsaunde+gcc
                   ` (4 more replies)
  0 siblings, 5 replies; 34+ messages in thread
From: tbsaunde+gcc @ 2017-07-31 23:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: palves

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Hi,

I've been saying I'd do this for a long time, but I'm finally getting to
importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
back.  I believe the gtl namespace also comes from Pedro, but GNU template
library seems as reasonable as any other name I can come up with.  I'm not sure
at the moment what outside of gcc may want to use this, but putting it include/
at least allows us to use it in libcpp which may be useful.  I didn't include
too much usage in this series, but I believe other people have wanted this too,
so I'm reasonably confident it will get a fair amount of usage.

patches individually bootstrapped + regtested on ppc64-linux-gnu, ok?

Trev


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-07-31 23:46 [PATCH 0/2] add unique_ptr class tbsaunde+gcc
  2017-07-31 23:46 ` [PATCH 1/2] add unique_ptr header tbsaunde+gcc
  2017-07-31 23:46 ` [PATCH 2/2] use unique_ptr some tbsaunde+gcc
@ 2017-08-01  0:51 ` David Malcolm
  2017-08-04 18:52 ` Jonathan Wakely
  2017-08-04 22:43 ` [PATCH 0/2] add unique_ptr class Martin Sebor
  4 siblings, 0 replies; 34+ messages in thread
From: David Malcolm @ 2017-08-01  0:51 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches; +Cc: palves

On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> 
> Hi,
> 
> I've been saying I'd do this for a long time, but I'm finally getting
> to
> importing the C++98 compatable unique_ptr class Pedro wrote for gdb a
> while
> back.  I believe the gtl namespace also comes from Pedro, but GNU
> template
> library seems as reasonable as any other name I can come up
> with.  I'm not sure
> at the moment what outside of gcc may want to use this, but putting
> it include/
> at least allows us to use it in libcpp which may be useful.  I didn't
> include
> too much usage in this series, but I believe other people have wanted
> this too,

Thanks for posting this.

For example, I could use this in:

  [PATCH 1/3] c-family: add name_hint/deferred_diagnostic
      https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html

e.g. as discussed here:

https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00123.html

Dave

> so I'm reasonably confident it will get a fair amount of usage.
> 
> patches individually bootstrapped + regtested on ppc64-linux-gnu, ok?
> 
> Trev
> 
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] add unique_ptr header
  2017-07-31 23:46 ` [PATCH 1/2] add unique_ptr header tbsaunde+gcc
@ 2017-08-01 14:40   ` David Malcolm
  2017-08-02  3:09     ` Trevor Saunders
  2017-08-02  8:14     ` [PATCH 1/2] add unique_ptr header Richard Biener
  0 siblings, 2 replies; 34+ messages in thread
From: David Malcolm @ 2017-08-01 14:40 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches; +Cc: palves

On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> 
> For most of the history of this see 
> https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
> The changes are mostly s/gdb/gtl/g
> 
> include/ChangeLog:
> 
> 2017-07-29  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
> 
> 	* unique-ptr.h: New file.
> ---
>  include/unique-ptr.h | 386
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 386 insertions(+)
>  create mode 100644 include/unique-ptr.h
> 
> diff --git a/include/unique-ptr.h b/include/unique-ptr.h
> new file mode 100644
> index 00000000000..7903a5abefe
> --- /dev/null
> +++ b/include/unique-ptr.h
> @@ -0,0 +1,386 @@
> +/* gtl::unique_ptr, a simple std::unique_ptr replacement for C++03.
> +
> +   Copyright (C) 2007-2016 Free Software Foundation, Inc.
> +
> +   This file is part of GCC.
> +
> +   This program is free software; you can redistribute it and/or
> modify
> +   it under the terms of the GNU General Public License as published
> by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/license
> s/>.  */
> +
> +/* gtl::unique_ptr defines a C++ owning smart pointer that exposes a
> +   subset of the std::unique_ptr API.
> +
> +   In fact, when compiled with a C++11 compiler, gtl::unique_ptr
> +   actually _is_ std::unique_ptr.  When compiled with a C++03
> compiler
> +   OTOH, it's an hand coded std::unique_ptr emulation that assumes
> +   code is correct and doesn't try to be too smart.

The comments talk about C++03 (presumably due to the gdb heritage of
this code), but gcc's minimum requirement is for C++98.

Does this code work with C++98?  If so, presumably this comment should
state that (presumably by just writing "C++98 or C++03" wherever you
say "C++03").

[...snip...]

Could the patch add a set of selftests for the new code?  (e.g. assert
the number of times that a destructor is called, for various test
classes in various situations).  This would also give a handy way to
run the pertinent code relatively quickly under valgrind (via "make
selftest-valgrind").

I'm not a global reviewer, so treat these as suggestions.

Hope this is helpful
Dave

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] add unique_ptr header
  2017-08-01 14:40   ` David Malcolm
@ 2017-08-02  3:09     ` Trevor Saunders
  2017-08-04 19:55       ` Jonathan Wakely
  2017-08-02  8:14     ` [PATCH 1/2] add unique_ptr header Richard Biener
  1 sibling, 1 reply; 34+ messages in thread
From: Trevor Saunders @ 2017-08-02  3:09 UTC (permalink / raw)
  To: David Malcolm; +Cc: tbsaunde+gcc, gcc-patches, palves

On Tue, Aug 01, 2017 at 10:40:53AM -0400, David Malcolm wrote:
> On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> > 
> > For most of the history of this see 
> > https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
> > The changes are mostly s/gdb/gtl/g
> > 
> > include/ChangeLog:
> > 
> > 2017-07-29  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
> > 
> > 	* unique-ptr.h: New file.
> > ---
> >  include/unique-ptr.h | 386
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 386 insertions(+)
> >  create mode 100644 include/unique-ptr.h
> > 
> > diff --git a/include/unique-ptr.h b/include/unique-ptr.h
> > new file mode 100644
> > index 00000000000..7903a5abefe
> > --- /dev/null
> > +++ b/include/unique-ptr.h
> > @@ -0,0 +1,386 @@
> > +/* gtl::unique_ptr, a simple std::unique_ptr replacement for C++03.
> > +
> > +   Copyright (C) 2007-2016 Free Software Foundation, Inc.
> > +
> > +   This file is part of GCC.
> > +
> > +   This program is free software; you can redistribute it and/or
> > modify
> > +   it under the terms of the GNU General Public License as published
> > by
> > +   the Free Software Foundation; either version 3 of the License, or
> > +   (at your option) any later version.
> > +
> > +   This program is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +   GNU General Public License for more details.
> > +
> > +   You should have received a copy of the GNU General Public License
> > +   along with this program.  If not, see <http://www.gnu.org/license
> > s/>.  */
> > +
> > +/* gtl::unique_ptr defines a C++ owning smart pointer that exposes a
> > +   subset of the std::unique_ptr API.
> > +
> > +   In fact, when compiled with a C++11 compiler, gtl::unique_ptr
> > +   actually _is_ std::unique_ptr.  When compiled with a C++03
> > compiler
> > +   OTOH, it's an hand coded std::unique_ptr emulation that assumes
> > +   code is correct and doesn't try to be too smart.
> 
> The comments talk about C++03 (presumably due to the gdb heritage of
> this code), but gcc's minimum requirement is for C++98.
> 
> Does this code work with C++98?  If so, presumably this comment should
> state that (presumably by just writing "C++98 or C++03" wherever you
> say "C++03").

aiui C++03 is C++98 with a few additions to the stl.  Yes this doesn't
build with -std=c++98 I haven't tried any of the strange c++ compilers
out there, but its not very fancy so I expect its fine.

> Could the patch add a set of selftests for the new code?  (e.g. assert
> the number of times that a destructor is called, for various test
> classes in various situations).  This would also give a handy way to
> run the pertinent code relatively quickly under valgrind (via "make
> selftest-valgrind").

Its possible, but I'm not sure it would provide much value.  I think
we're unlikely to change this code much since it already more or less
completely implements the API it is shimming.  It also is fairly
trivial, so I doubt anyone would want to run valgrind on just the
unique_ptr code.  Of course I may just be being lazy here.

thanks

Trev

> 
> I'm not a global reviewer, so treat these as suggestions.
> 
> Hope this is helpful
> Dave

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] add unique_ptr header
  2017-08-01 14:40   ` David Malcolm
  2017-08-02  3:09     ` Trevor Saunders
@ 2017-08-02  8:14     ` Richard Biener
  1 sibling, 0 replies; 34+ messages in thread
From: Richard Biener @ 2017-08-02  8:14 UTC (permalink / raw)
  To: David Malcolm; +Cc: tbsaunde+gcc, GCC Patches, palves

On Tue, Aug 1, 2017 at 4:40 PM, David Malcolm <dmalcolm@redhat.com> wrote
> On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
>> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>>
>> For most of the history of this see
>> https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
>> The changes are mostly s/gdb/gtl/g
>>
>> include/ChangeLog:
>>
>> 2017-07-29  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>>
>>       * unique-ptr.h: New file.
>> ---
>>  include/unique-ptr.h | 386
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 386 insertions(+)
>>  create mode 100644 include/unique-ptr.h
>>
>> diff --git a/include/unique-ptr.h b/include/unique-ptr.h
>> new file mode 100644
>> index 00000000000..7903a5abefe
>> --- /dev/null
>> +++ b/include/unique-ptr.h
>> @@ -0,0 +1,386 @@
>> +/* gtl::unique_ptr, a simple std::unique_ptr replacement for C++03.
>> +
>> +   Copyright (C) 2007-2016 Free Software Foundation, Inc.
>> +
>> +   This file is part of GCC.
>> +
>> +   This program is free software; you can redistribute it and/or
>> modify
>> +   it under the terms of the GNU General Public License as published
>> by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/license
>> s/>.  */
>> +
>> +/* gtl::unique_ptr defines a C++ owning smart pointer that exposes a
>> +   subset of the std::unique_ptr API.
>> +
>> +   In fact, when compiled with a C++11 compiler, gtl::unique_ptr
>> +   actually _is_ std::unique_ptr.  When compiled with a C++03
>> compiler
>> +   OTOH, it's an hand coded std::unique_ptr emulation that assumes
>> +   code is correct and doesn't try to be too smart.
>
> The comments talk about C++03 (presumably due to the gdb heritage of
> this code), but gcc's minimum requirement is for C++98.

It's actually C++04 but I don't remember the details.  It happens to work
with GCC 3.4 and thus C++98 though.

Richard.

> Does this code work with C++98?  If so, presumably this comment should
> state that (presumably by just writing "C++98 or C++03" wherever you
> say "C++03").
>
> [...snip...]
>
> Could the patch add a set of selftests for the new code?  (e.g. assert
> the number of times that a destructor is called, for various test
> classes in various situations).  This would also give a handy way to
> run the pertinent code relatively quickly under valgrind (via "make
> selftest-valgrind").
>
> I'm not a global reviewer, so treat these as suggestions.
>
> Hope this is helpful
> Dave

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-07-31 23:46 [PATCH 0/2] add unique_ptr class tbsaunde+gcc
                   ` (2 preceding siblings ...)
  2017-08-01  0:51 ` [PATCH 0/2] add unique_ptr class David Malcolm
@ 2017-08-04 18:52 ` Jonathan Wakely
  2017-08-05  5:36   ` Trevor Saunders
  2017-08-05 19:05   ` Pedro Alves
  2017-08-04 22:43 ` [PATCH 0/2] add unique_ptr class Martin Sebor
  4 siblings, 2 replies; 34+ messages in thread
From: Jonathan Wakely @ 2017-08-04 18:52 UTC (permalink / raw)
  To: tbsaunde+gcc; +Cc: gcc-patches, palves

On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
>I've been saying I'd do this for a long time, but I'm finally getting to
>importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
>back.  I believe the gtl namespace also comes from Pedro, but GNU template
>library seems as reasonable as any other name I can come up with.

If it's inspired by "STL" then can we call it something else?

std::unique_ptr is not part of the STL, because the STL is a library
of containers and algorithms from the 1990s. std::unique_ptr is
something much newer that doesn't originate in the STL.

STL != C++ Standard Library

If we want a namespace for GNU utilities, what's wrong with "gnu"?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] add unique_ptr header
  2017-08-02  3:09     ` Trevor Saunders
@ 2017-08-04 19:55       ` Jonathan Wakely
  2017-08-05  5:39         ` Trevor Saunders
  0 siblings, 1 reply; 34+ messages in thread
From: Jonathan Wakely @ 2017-08-04 19:55 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: David Malcolm, tbsaunde+gcc, gcc-patches, palves

On 01/08/17 23:09 -0400, Trevor Saunders wrote:
>aiui C++03 is C++98 with a few additions to the stl.

Again, STL != C++ Standard Library.

C++03 made some important changes to the core language, particularly
the value-initialization rules.

Most of the library changes were small bug fixes, and most were to
locales (which didn't originate in the STL and aren't even templates)
and iostreams (which didn't originate in the STL).

There were also changes to std::auto_ptr (also not from the STL) which
this might rely on (I haven't checked).


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-07-31 23:46 [PATCH 0/2] add unique_ptr class tbsaunde+gcc
                   ` (3 preceding siblings ...)
  2017-08-04 18:52 ` Jonathan Wakely
@ 2017-08-04 22:43 ` Martin Sebor
  2017-08-05  5:32   ` Trevor Saunders
  4 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2017-08-04 22:43 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches; +Cc: palves

On 07/31/2017 05:46 PM, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> Hi,
>
> I've been saying I'd do this for a long time, but I'm finally getting to
> importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
> back.  I believe the gtl namespace also comes from Pedro, but GNU template
> library seems as reasonable as any other name I can come up with.  I'm not sure
> at the moment what outside of gcc may want to use this, but putting it include/
> at least allows us to use it in libcpp which may be useful.  I didn't include
> too much usage in this series, but I believe other people have wanted this too,
> so I'm reasonably confident it will get a fair amount of usage.
>
> patches individually bootstrapped + regtested on ppc64-linux-gnu, ok?

FWIW, I'm a big fan of RAII and I like the idea being able to
rely on a smart pointer in GCC quite a bit.  Although I haven't
reviewed the C++ 03 implementation in any detail I like how the
new API is just transitional until GCC switches from C++ 98 to
C++ 11 when it will be possible to drop it, presumably with no
changes to client code.  (If there's any risk that clients might
come to depend on the C++ 03 "features" or limitations in the
meantime and thus jeopardize this goal I would only suggest to
put some effort into making this harder.)

Other than that, thank you for this nice enhancement!

Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-04 22:43 ` [PATCH 0/2] add unique_ptr class Martin Sebor
@ 2017-08-05  5:32   ` Trevor Saunders
  0 siblings, 0 replies; 34+ messages in thread
From: Trevor Saunders @ 2017-08-05  5:32 UTC (permalink / raw)
  To: Martin Sebor; +Cc: tbsaunde+gcc, gcc-patches, palves

On Fri, Aug 04, 2017 at 04:43:32PM -0600, Martin Sebor wrote:
> On 07/31/2017 05:46 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> > 
> > Hi,
> > 
> > I've been saying I'd do this for a long time, but I'm finally getting to
> > importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
> > back.  I believe the gtl namespace also comes from Pedro, but GNU template
> > library seems as reasonable as any other name I can come up with.  I'm not sure
> > at the moment what outside of gcc may want to use this, but putting it include/
> > at least allows us to use it in libcpp which may be useful.  I didn't include
> > too much usage in this series, but I believe other people have wanted this too,
> > so I'm reasonably confident it will get a fair amount of usage.
> > 
> > patches individually bootstrapped + regtested on ppc64-linux-gnu, ok?
> 
> FWIW, I'm a big fan of RAII and I like the idea being able to
> rely on a smart pointer in GCC quite a bit.  Although I haven't
> reviewed the C++ 03 implementation in any detail I like how the
> new API is just transitional until GCC switches from C++ 98 to
> C++ 11 when it will be possible to drop it, presumably with no
> changes to client code.  (If there's any risk that clients might
> come to depend on the C++ 03 "features" or limitations in the
> meantime and thus jeopardize this goal I would only suggest to
> put some effort into making this harder.)

Given that stage 2 / 3 of a bootstrap is as C++14 I expect that it would
be pretty hard to put in code that works with the shim, but not real
unique_ptr, since in a bootstrap situation both would be used.

> Other than that, thank you for this nice enhancement!

your welcome

Trev

> 
> Martin
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-04 18:52 ` Jonathan Wakely
@ 2017-08-05  5:36   ` Trevor Saunders
  2017-08-07 13:01     ` Jonathan Wakely
  2017-08-05 19:05   ` Pedro Alves
  1 sibling, 1 reply; 34+ messages in thread
From: Trevor Saunders @ 2017-08-05  5:36 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: tbsaunde+gcc, gcc-patches, palves

On Fri, Aug 04, 2017 at 07:52:18PM +0100, Jonathan Wakely wrote:
> On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
> > I've been saying I'd do this for a long time, but I'm finally getting to
> > importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
> > back.  I believe the gtl namespace also comes from Pedro, but GNU template
> > library seems as reasonable as any other name I can come up with.
> 
> If it's inspired by "STL" then can we call it something else?
> 
> std::unique_ptr is not part of the STL, because the STL is a library
> of containers and algorithms from the 1990s. std::unique_ptr is
> something much newer that doesn't originate in the STL.

ok then I forgot or never knew that sorry.  It *is* std::unique_ptr in
C++11 or newer, so I think it would be odd to name it something else.

> STL != C++ Standard Library

Will try to remember that, sorry.

> If we want a namespace for GNU utilities, what's wrong with "gnu"?

I'm fine with s/gtl/gnu/g if that's what people want.

Trev

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] add unique_ptr header
  2017-08-04 19:55       ` Jonathan Wakely
@ 2017-08-05  5:39         ` Trevor Saunders
  2017-10-11 18:52           ` David Malcolm
  0 siblings, 1 reply; 34+ messages in thread
From: Trevor Saunders @ 2017-08-05  5:39 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: David Malcolm, tbsaunde+gcc, gcc-patches, palves

On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > aiui C++03 is C++98 with a few additions to the stl.
> 
> Again, STL != C++ Standard Library.
> 
> C++03 made some important changes to the core language, particularly
> the value-initialization rules.
> 
> Most of the library changes were small bug fixes, and most were to
> locales (which didn't originate in the STL and aren't even templates)
> and iostreams (which didn't originate in the STL).
> 
> There were also changes to std::auto_ptr (also not from the STL) which
> this might rely on (I haven't checked).

I doubt it, as Pedro said in his email it originally copied code from
std::auto_ptr, but it doesn't use the standard libraries definition of
std::auto_ptr anywhere.  However please do feel free to look at the
implementation.

Trev

> 
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-04 18:52 ` Jonathan Wakely
  2017-08-05  5:36   ` Trevor Saunders
@ 2017-08-05 19:05   ` Pedro Alves
  2017-08-11 21:30     ` Jonathan Wakely
  2017-09-05 16:52     ` Manuel López-Ibáñez
  1 sibling, 2 replies; 34+ messages in thread
From: Pedro Alves @ 2017-08-05 19:05 UTC (permalink / raw)
  To: Jonathan Wakely, tbsaunde+gcc; +Cc: gcc-patches

On 08/04/2017 07:52 PM, Jonathan Wakely wrote:
> On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
>> I've been saying I'd do this for a long time, but I'm finally getting to
>> importing the C++98 compatable unique_ptr class Pedro wrote for gdb a
>> while
>> back.  

Thanks a lot for doing this!

> I believe the gtl namespace also comes from Pedro, but GNU template
> library seems as reasonable as any other name I can come up with.

Yes, I had suggested it here:

  https://sourceware.org/ml/gdb-patches/2017-02/msg00197.html

> 
> If it's inspired by "STL" then can we call it something else?
> 
> std::unique_ptr is not part of the STL, because the STL is a library
> of containers and algorithms from the 1990s. std::unique_ptr is
> something much newer that doesn't originate in the STL.
> 
> STL != C++ Standard Library

That I knew, but gtl was not really a reference to the
C++ Standard Library, so I don't see the problem.  It was supposed to
be the name of a library which would contain other C++ utilities
that would be shared by different GNU toolchain components.
Some of those bits would be inspired by, or be straight backports of
more-recent standards, but it'd be more than that.

An option would be to keep "gtl" as acronym, but expand it
to "GNU Toolchain Library" instead.

For example, gdb has been growing C++ utilities under gdb/common/
that might be handy for gcc and other projects too, for example:

 - enum_flags (type-safe enum bit flags)
 - function_view (non-owning reference to callables)
 - offset_type (type safe / distinct integer types to represent offsets
                into anything addressable)
 - optional (C++11 backport of libstdc++'s std::optional)
 - refcounted_object.h (intrusively-refcounted types)
 - scoped_restore (RAII save/restore of globals)
 - an allocator that default-constructs using default-initialization
   instead of value-initialization.

and more.

GCC OTOH has code that might be handy for GDB as well, like for
example the open addressing hash tables (hash_map/hash_table/hash_set
etc.).

Maybe Gold could make use of some of this code too.  There
are some bits in Gold that might be useful for (at least) GDB
too.  For example, its Stringpool class.

I think there's a lot of scope for sharing more code between the
different components of the GNU toolchain, even beyond general
random utilities and data structures, and I'd love to see us
move more in that direction.

> If we want a namespace for GNU utilities, what's wrong with "gnu"?

That'd be an "obvious" choice, and I'm not terribly against it,
though I wonder whether it'd be taking over a name that has a wider
scope than intended?  I.e., GNU is a larger set of projects than the
GNU toolchain.  For example, there's Gnulib, which already compiles
as libgnu.a / -lgnu, which might be confusing.  GCC doesn't currently
use Gnulib, but GDB does, and, there was work going on a while ago to
make GCC use gnulib as well.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-05  5:36   ` Trevor Saunders
@ 2017-08-07 13:01     ` Jonathan Wakely
  0 siblings, 0 replies; 34+ messages in thread
From: Jonathan Wakely @ 2017-08-07 13:01 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: tbsaunde+gcc, gcc-patches, palves

On 05/08/17 01:36 -0400, Trevor Saunders wrote:
>On Fri, Aug 04, 2017 at 07:52:18PM +0100, Jonathan Wakely wrote:
>> On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
>> > I've been saying I'd do this for a long time, but I'm finally getting to
>> > importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
>> > back.  I believe the gtl namespace also comes from Pedro, but GNU template
>> > library seems as reasonable as any other name I can come up with.
>>
>> If it's inspired by "STL" then can we call it something else?
>>
>> std::unique_ptr is not part of the STL, because the STL is a library
>> of containers and algorithms from the 1990s. std::unique_ptr is
>> something much newer that doesn't originate in the STL.
>
>ok then I forgot or never knew that sorry.  It *is* std::unique_ptr in
>C++11 or newer, so I think it would be odd to name it something else.

I was only objecting to "gtl" not to "unique_ptr". I agree that
calling it unique_ptr is the logical choice.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-05 19:05   ` Pedro Alves
@ 2017-08-11 21:30     ` Jonathan Wakely
  2017-09-03  1:24       ` Trevor Saunders
  2017-09-04  9:31       ` Richard Biener
  2017-09-05 16:52     ` Manuel López-Ibáñez
  1 sibling, 2 replies; 34+ messages in thread
From: Jonathan Wakely @ 2017-08-11 21:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: tbsaunde+gcc, gcc-patches

On 05/08/17 20:05 +0100, Pedro Alves wrote:
>On 08/04/2017 07:52 PM, Jonathan Wakely wrote:
>> On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
>>> I've been saying I'd do this for a long time, but I'm finally getting to
>>> importing the C++98 compatable unique_ptr class Pedro wrote for gdb a
>>> while
>>> back.
>
>Thanks a lot for doing this!
>
>> I believe the gtl namespace also comes from Pedro, but GNU template
>> library seems as reasonable as any other name I can come up with.
>
>Yes, I had suggested it here:
>
>  https://sourceware.org/ml/gdb-patches/2017-02/msg00197.html
>
>>
>> If it's inspired by "STL" then can we call it something else?
>>
>> std::unique_ptr is not part of the STL, because the STL is a library
>> of containers and algorithms from the 1990s. std::unique_ptr is
>> something much newer that doesn't originate in the STL.
>>
>> STL != C++ Standard Library
>
>That I knew, but gtl was not really a reference to the
>C++ Standard Library, so I don't see the problem.  It was supposed to
>be the name of a library which would contain other C++ utilities
>that would be shared by different GNU toolchain components.
>Some of those bits would be inspired by, or be straight backports of
>more-recent standards, but it'd be more than that.
>
>An option would be to keep "gtl" as acronym, but expand it
>to "GNU Toolchain Library" instead.

OK, that raises my hackles less. What bothered me was an apparent
analogy to "STL" when that isn't even the right name for the library
that provides the original unique_ptr.

And "template library" assumes we'd never add non-templates to it,
which is unlikely (you already mentioned offset_type, which isn't a
template).

It seems odd to make up a completely new acronym for this though,
rather than naming it after something that exists already in the
toolchain codebases.

>For example, gdb has been growing C++ utilities under gdb/common/
>that might be handy for gcc and other projects too, for example:
>
> - enum_flags (type-safe enum bit flags)
> - function_view (non-owning reference to callables)
> - offset_type (type safe / distinct integer types to represent offsets
>                into anything addressable)
> - optional (C++11 backport of libstdc++'s std::optional)
> - refcounted_object.h (intrusively-refcounted types)
> - scoped_restore (RAII save/restore of globals)
> - an allocator that default-constructs using default-initialization
>   instead of value-initialization.
>
>and more.
>
>GCC OTOH has code that might be handy for GDB as well, like for
>example the open addressing hash tables (hash_map/hash_table/hash_set
>etc.).
>
>Maybe Gold could make use of some of this code too.  There
>are some bits in Gold that might be useful for (at least) GDB
>too.  For example, its Stringpool class.
>
>I think there's a lot of scope for sharing more code between the
>different components of the GNU toolchain, even beyond general
>random utilities and data structures, and I'd love to see us
>move more in that direction.
>
>> If we want a namespace for GNU utilities, what's wrong with "gnu"?
>
>That'd be an "obvious" choice, and I'm not terribly against it,
>though I wonder whether it'd be taking over a name that has a wider
>scope than intended?  I.e., GNU is a larger set of projects than the
>GNU toolchain.  For example, there's Gnulib, which already compiles
>as libgnu.a / -lgnu, which might be confusing.  GCC doesn't currently
>use Gnulib, but GDB does, and, there was work going on a while ago to
>make GCC use gnulib as well.

Good point. "gnutools" might be more accurate, but people might object
to adding 10 extra characters for "gnutools::".

Naming is important, especially for a whole namespace (not just a
single type) so I do think it's worth spending time getting it right.

But I could live with gtl as long as nobody ever says "GTL is the GNU
STL" or mentions "gtl" and STL in the same breath :-)


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-11 21:30     ` Jonathan Wakely
@ 2017-09-03  1:24       ` Trevor Saunders
  2017-09-04  9:31       ` Richard Biener
  1 sibling, 0 replies; 34+ messages in thread
From: Trevor Saunders @ 2017-09-03  1:24 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Pedro Alves, tbsaunde+gcc, gcc-patches

HI,

figured I'd ping this to see if we can come to some concensus, I don't
care much what we choose as a namespace, I just want to get this in so
we can use it more.

On Fri, Aug 11, 2017 at 09:43:21PM +0100, Jonathan Wakely wrote:
> On 05/08/17 20:05 +0100, Pedro Alves wrote:
> > On 08/04/2017 07:52 PM, Jonathan Wakely wrote:
> > > On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
> > > > I've been saying I'd do this for a long time, but I'm finally getting to
> > > > importing the C++98 compatable unique_ptr class Pedro wrote for gdb a
> > > > while
> > > > back.
> > 
> > Thanks a lot for doing this!
> > 
> > > I believe the gtl namespace also comes from Pedro, but GNU template
> > > library seems as reasonable as any other name I can come up with.
> > 
> > Yes, I had suggested it here:
> > 
> >  https://sourceware.org/ml/gdb-patches/2017-02/msg00197.html
> > 
> > > 
> > > If it's inspired by "STL" then can we call it something else?
> > > 
> > > std::unique_ptr is not part of the STL, because the STL is a library
> > > of containers and algorithms from the 1990s. std::unique_ptr is
> > > something much newer that doesn't originate in the STL.
> > > 
> > > STL != C++ Standard Library
> > 
> > That I knew, but gtl was not really a reference to the
> > C++ Standard Library, so I don't see the problem.  It was supposed to
> > be the name of a library which would contain other C++ utilities
> > that would be shared by different GNU toolchain components.
> > Some of those bits would be inspired by, or be straight backports of
> > more-recent standards, but it'd be more than that.
> > 
> > An option would be to keep "gtl" as acronym, but expand it
> > to "GNU Toolchain Library" instead.
> 
> OK, that raises my hackles less. What bothered me was an apparent
> analogy to "STL" when that isn't even the right name for the library
> that provides the original unique_ptr.
> 
> And "template library" assumes we'd never add non-templates to it,
> which is unlikely (you already mentioned offset_type, which isn't a
> template).

Well, "GNU Toolchain Library" doesn't include template anywhere in the
name ;)

> It seems odd to make up a completely new acronym for this though,
> rather than naming it after something that exists already in the
> toolchain codebases.

That's fair, I suppose we could use namespace libiberty, but that's kind
of long.

> > For example, gdb has been growing C++ utilities under gdb/common/
> > that might be handy for gcc and other projects too, for example:
> > 
> > - enum_flags (type-safe enum bit flags)
> > - function_view (non-owning reference to callables)
> > - offset_type (type safe / distinct integer types to represent offsets
> >                into anything addressable)
> > - optional (C++11 backport of libstdc++'s std::optional)
> > - refcounted_object.h (intrusively-refcounted types)
> > - scoped_restore (RAII save/restore of globals)
> > - an allocator that default-constructs using default-initialization
> >   instead of value-initialization.
> > 
> > and more.
> > 
> > GCC OTOH has code that might be handy for GDB as well, like for
> > example the open addressing hash tables (hash_map/hash_table/hash_set
> > etc.).
> > 
> > Maybe Gold could make use of some of this code too.  There
> > are some bits in Gold that might be useful for (at least) GDB
> > too.  For example, its Stringpool class.
> > 
> > I think there's a lot of scope for sharing more code between the
> > different components of the GNU toolchain, even beyond general
> > random utilities and data structures, and I'd love to see us
> > move more in that direction.
> > 
> > > If we want a namespace for GNU utilities, what's wrong with "gnu"?
> > 
> > That'd be an "obvious" choice, and I'm not terribly against it,
> > though I wonder whether it'd be taking over a name that has a wider
> > scope than intended?  I.e., GNU is a larger set of projects than the
> > GNU toolchain.  For example, there's Gnulib, which already compiles
> > as libgnu.a / -lgnu, which might be confusing.  GCC doesn't currently
> > use Gnulib, but GDB does, and, there was work going on a while ago to
> > make GCC use gnulib as well.
> 
> Good point. "gnutools" might be more accurate, but people might object
> to adding 10 extra characters for "gnutools::".

yeah, 3 or 4 characters seems to be the length of namespace names that
other people like, and its still fairly reasonable to use the fully
qualified name then.

> Naming is important, especially for a whole namespace (not just a
> single type) so I do think it's worth spending time getting it right.

True, though as far as I'm aware we wouldn't be promising any kind of
stability so would be free to rename it if we felt the need.

> But I could live with gtl as long as nobody ever says "GTL is the GNU
> STL" or mentions "gtl" and STL in the same breath :-)

heh, well feel free to be annoyed if I do it at least ;)

thanks

Trev

> 
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-11 21:30     ` Jonathan Wakely
  2017-09-03  1:24       ` Trevor Saunders
@ 2017-09-04  9:31       ` Richard Biener
  2017-09-04 10:05         ` Pedro Alves
  1 sibling, 1 reply; 34+ messages in thread
From: Richard Biener @ 2017-09-04  9:31 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Pedro Alves, tbsaunde+gcc, GCC Patches

On Fri, Aug 11, 2017 at 10:43 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 05/08/17 20:05 +0100, Pedro Alves wrote:
>>
>> On 08/04/2017 07:52 PM, Jonathan Wakely wrote:
>>>
>>> On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
>>>>
>>>> I've been saying I'd do this for a long time, but I'm finally getting to
>>>> importing the C++98 compatable unique_ptr class Pedro wrote for gdb a
>>>> while
>>>> back.
>>
>>
>> Thanks a lot for doing this!
>>
>>> I believe the gtl namespace also comes from Pedro, but GNU template
>>> library seems as reasonable as any other name I can come up with.
>>
>>
>> Yes, I had suggested it here:
>>
>>  https://sourceware.org/ml/gdb-patches/2017-02/msg00197.html
>>
>>>
>>> If it's inspired by "STL" then can we call it something else?
>>>
>>> std::unique_ptr is not part of the STL, because the STL is a library
>>> of containers and algorithms from the 1990s. std::unique_ptr is
>>> something much newer that doesn't originate in the STL.
>>>
>>> STL != C++ Standard Library
>>
>>
>> That I knew, but gtl was not really a reference to the
>> C++ Standard Library, so I don't see the problem.  It was supposed to
>> be the name of a library which would contain other C++ utilities
>> that would be shared by different GNU toolchain components.
>> Some of those bits would be inspired by, or be straight backports of
>> more-recent standards, but it'd be more than that.
>>
>> An option would be to keep "gtl" as acronym, but expand it
>> to "GNU Toolchain Library" instead.
>
>
> OK, that raises my hackles less. What bothered me was an apparent
> analogy to "STL" when that isn't even the right name for the library
> that provides the original unique_ptr.
>
> And "template library" assumes we'd never add non-templates to it,
> which is unlikely (you already mentioned offset_type, which isn't a
> template).
>
> It seems odd to make up a completely new acronym for this though,
> rather than naming it after something that exists already in the
> toolchain codebases.
>
>
>> For example, gdb has been growing C++ utilities under gdb/common/
>> that might be handy for gcc and other projects too, for example:
>>
>> - enum_flags (type-safe enum bit flags)
>> - function_view (non-owning reference to callables)
>> - offset_type (type safe / distinct integer types to represent offsets
>>                into anything addressable)
>> - optional (C++11 backport of libstdc++'s std::optional)
>> - refcounted_object.h (intrusively-refcounted types)
>> - scoped_restore (RAII save/restore of globals)
>> - an allocator that default-constructs using default-initialization
>>   instead of value-initialization.
>>
>> and more.
>>
>> GCC OTOH has code that might be handy for GDB as well, like for
>> example the open addressing hash tables (hash_map/hash_table/hash_set
>> etc.).
>>
>> Maybe Gold could make use of some of this code too.  There
>> are some bits in Gold that might be useful for (at least) GDB
>> too.  For example, its Stringpool class.
>>
>> I think there's a lot of scope for sharing more code between the
>> different components of the GNU toolchain, even beyond general
>> random utilities and data structures, and I'd love to see us
>> move more in that direction.
>>
>>> If we want a namespace for GNU utilities, what's wrong with "gnu"?
>>
>>
>> That'd be an "obvious" choice, and I'm not terribly against it,
>> though I wonder whether it'd be taking over a name that has a wider
>> scope than intended?  I.e., GNU is a larger set of projects than the
>> GNU toolchain.  For example, there's Gnulib, which already compiles
>> as libgnu.a / -lgnu, which might be confusing.  GCC doesn't currently
>> use Gnulib, but GDB does, and, there was work going on a while ago to
>> make GCC use gnulib as well.
>
>
> Good point. "gnutools" might be more accurate, but people might object
> to adding 10 extra characters for "gnutools::".
>
> Naming is important, especially for a whole namespace (not just a
> single type) so I do think it's worth spending time getting it right.
>
> But I could live with gtl as long as nobody ever says "GTL is the GNU
> STL" or mentions "gtl" and STL in the same breath :-)

If it should be short use g::.  We can also use gnu:: I guess and I
agree gnutools:: is a little long (similar to libiberty::).  Maybe
gt:: as a short-hand for gnutools.

Richard.

>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-09-04  9:31       ` Richard Biener
@ 2017-09-04 10:05         ` Pedro Alves
  0 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2017-09-04 10:05 UTC (permalink / raw)
  To: Richard Biener, Jonathan Wakely; +Cc: tbsaunde+gcc, GCC Patches

On 09/04/2017 11:31 AM, Richard Biener wrote:
> On Fri, Aug 11, 2017 at 10:43 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 05/08/17 20:05 +0100, Pedro Alves wrote:
>>>
>>> On 08/04/2017 07:52 PM, Jonathan Wakely wrote:
>>>>
>>>> On 31/07/17 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
>>>>>
>>>>> I've been saying I'd do this for a long time, but I'm finally getting to
>>>>> importing the C++98 compatable unique_ptr class Pedro wrote for gdb a
>>>>> while
>>>>> back.
>>>
>>>
>>> Thanks a lot for doing this!
>>>
>>>> I believe the gtl namespace also comes from Pedro, but GNU template
>>>> library seems as reasonable as any other name I can come up with.
>>>
>>>
>>> Yes, I had suggested it here:
>>>
>>>  https://sourceware.org/ml/gdb-patches/2017-02/msg00197.html
>>>
>>>>
>>>> If it's inspired by "STL" then can we call it something else?
>>>>
>>>> std::unique_ptr is not part of the STL, because the STL is a library
>>>> of containers and algorithms from the 1990s. std::unique_ptr is
>>>> something much newer that doesn't originate in the STL.
>>>>
>>>> STL != C++ Standard Library
>>>
>>>
>>> That I knew, but gtl was not really a reference to the
>>> C++ Standard Library, so I don't see the problem.  It was supposed to
>>> be the name of a library which would contain other C++ utilities
>>> that would be shared by different GNU toolchain components.
>>> Some of those bits would be inspired by, or be straight backports of
>>> more-recent standards, but it'd be more than that.
>>>
>>> An option would be to keep "gtl" as acronym, but expand it
>>> to "GNU Toolchain Library" instead.
>>
>>
>> OK, that raises my hackles less. What bothered me was an apparent
>> analogy to "STL" when that isn't even the right name for the library
>> that provides the original unique_ptr.
>>
>> And "template library" assumes we'd never add non-templates to it,
>> which is unlikely (you already mentioned offset_type, which isn't a
>> template).
>>
>> It seems odd to make up a completely new acronym for this though,
>> rather than naming it after something that exists already in the
>> toolchain codebases.
>>
>>
>>> For example, gdb has been growing C++ utilities under gdb/common/
>>> that might be handy for gcc and other projects too, for example:
>>>
>>> - enum_flags (type-safe enum bit flags)
>>> - function_view (non-owning reference to callables)
>>> - offset_type (type safe / distinct integer types to represent offsets
>>>                into anything addressable)
>>> - optional (C++11 backport of libstdc++'s std::optional)
>>> - refcounted_object.h (intrusively-refcounted types)
>>> - scoped_restore (RAII save/restore of globals)
>>> - an allocator that default-constructs using default-initialization
>>>   instead of value-initialization.
>>>
>>> and more.
>>>
>>> GCC OTOH has code that might be handy for GDB as well, like for
>>> example the open addressing hash tables (hash_map/hash_table/hash_set
>>> etc.).
>>>
>>> Maybe Gold could make use of some of this code too.  There
>>> are some bits in Gold that might be useful for (at least) GDB
>>> too.  For example, its Stringpool class.
>>>
>>> I think there's a lot of scope for sharing more code between the
>>> different components of the GNU toolchain, even beyond general
>>> random utilities and data structures, and I'd love to see us
>>> move more in that direction.
>>>
>>>> If we want a namespace for GNU utilities, what's wrong with "gnu"?
>>>
>>>
>>> That'd be an "obvious" choice, and I'm not terribly against it,
>>> though I wonder whether it'd be taking over a name that has a wider
>>> scope than intended?  I.e., GNU is a larger set of projects than the
>>> GNU toolchain.  For example, there's Gnulib, which already compiles
>>> as libgnu.a / -lgnu, which might be confusing.  GCC doesn't currently
>>> use Gnulib, but GDB does, and, there was work going on a while ago to
>>> make GCC use gnulib as well.
>>
>>
>> Good point. "gnutools" might be more accurate, but people might object
>> to adding 10 extra characters for "gnutools::".
>>
>> Naming is important, especially for a whole namespace (not just a
>> single type) so I do think it's worth spending time getting it right.
>>
>> But I could live with gtl as long as nobody ever says "GTL is the GNU
>> STL" or mentions "gtl" and STL in the same breath :-)
> 
> If it should be short use g::.  We can also use gnu:: I guess and I
> agree gnutools:: is a little long (similar to libiberty::).  Maybe
> gt:: as a short-hand for gnutools.

Exactly 3 letters has the nice property of making s/gtl::foo/std::foo/ super
trivial down the road; you don't have to care about reindenting stuff
[1].  Also makes gdb->gtl and gcc->gtl renamings trivial in the same way.
Really a minor thing in the grand scheme of things, but just a FYI that that
factored in a bit in the original motivation for the "gtl" naming back when
I proposed it on the gdb list.

[1] - [PATCH] gdb::{unique_ptr,move} -> std::{unique_ptr,move}:
https://sourceware.org/ml/gdb-patches/2016-11/msg00200.html

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-08-05 19:05   ` Pedro Alves
  2017-08-11 21:30     ` Jonathan Wakely
@ 2017-09-05 16:52     ` Manuel López-Ibáñez
  2017-09-05 17:40       ` Pedro Alves
  1 sibling, 1 reply; 34+ messages in thread
From: Manuel López-Ibáñez @ 2017-09-05 16:52 UTC (permalink / raw)
  To: GCC Patches; +Cc: Pedro Alves, Jonathan Wakely, tbsaunde+gcc

On 05/08/17 20:05, Pedro Alves wrote:
> That'd be an "obvious" choice, and I'm not terribly against it,
> though I wonder whether it'd be taking over a name that has a wider
> scope than intended?  I.e., GNU is a larger set of projects than the
> GNU toolchain.  For example, there's Gnulib, which already compiles
> as libgnu.a / -lgnu, which might be confusing.  GCC doesn't currently
> use Gnulib, but GDB does, and, there was work going on a while ago to
> make GCC use gnulib as well.

Unfortunately, that work was never committed, although there are parts that are 
ready to be committed and the rest of the conversion could be done incrementally:

https://gcc.gnu.org/wiki/replacelibibertywithgnulib

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
  2017-09-05 16:52     ` Manuel López-Ibáñez
@ 2017-09-05 17:40       ` Pedro Alves
  2017-09-06 12:35         ` replace libiberty with gnulib (was: Re: [PATCH 0/2] add unique_ptr class) Manuel López-Ibáñez
  0 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2017-09-05 17:40 UTC (permalink / raw)
  To: Manuel López-Ibáñez, GCC Patches
  Cc: Jonathan Wakely, tbsaunde+gcc

On 09/05/2017 05:52 PM, Manuel López-Ibáñez wrote:
> On 05/08/17 20:05, Pedro Alves wrote:
>> That'd be an "obvious" choice, and I'm not terribly against it,
>> though I wonder whether it'd be taking over a name that has a wider
>> scope than intended?  I.e., GNU is a larger set of projects than the
>> GNU toolchain.  For example, there's Gnulib, which already compiles
>> as libgnu.a / -lgnu, which might be confusing.  GCC doesn't currently
>> use Gnulib, but GDB does, and, there was work going on a while ago to
>> make GCC use gnulib as well.
> 
> Unfortunately, that work was never committed, although there are parts
> that are ready to be committed and the rest of the conversion could be
> done incrementally:
> 
> https://gcc.gnu.org/wiki/replacelibibertywithgnulib

Yeah, ISTR it was close, though there were a couple things
that needed addressing still.

The wiki seems to miss a pointer to following iterations/review
of that patch (mailing list archives don't cross month
boundaries...).  You can find it starting here:
 https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01208.html
I think this was the latest version posted:
 https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01554.html

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 34+ messages in thread

* replace libiberty with gnulib (was: Re: [PATCH 0/2] add unique_ptr class)
  2017-09-05 17:40       ` Pedro Alves
@ 2017-09-06 12:35         ` Manuel López-Ibáñez
  0 siblings, 0 replies; 34+ messages in thread
From: Manuel López-Ibáñez @ 2017-09-06 12:35 UTC (permalink / raw)
  To: Pedro Alves, GCC Patches; +Cc: Jonathan Wakely, tbsaunde+gcc

On 05/09/17 18:40, Pedro Alves wrote:
> On 09/05/2017 05:52 PM, Manuel López-Ibáñez wrote:
> Yeah, ISTR it was close, though there were a couple things
> that needed addressing still.
> 
> The wiki seems to miss a pointer to following iterations/review
> of that patch (mailing list archives don't cross month
> boundaries...).  You can find it starting here:
>   https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01208.html
> I think this was the latest version posted:
>   https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01554.html

Thanks, I have updated the hyperlinks in the wiki.

Unfortunately, Ayush left and there is no one else to finish the work. While 
converting individuals functions from libiberty to gnulib is more or less 
straightforward, the build system of GCC is far too complex for any new or 
less-experienced contributor to finish the job.

I have also updated https://gcc.gnu.org/wiki/SummerOfCode
I don't believe that this was the only project accepted in 2016, but I cannot 
remember the others. Didn't GCC apply this year?

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] add unique_ptr header
  2017-08-05  5:39         ` Trevor Saunders
@ 2017-10-11 18:52           ` David Malcolm
  2017-10-12 14:08             ` Trevor Saunders
  0 siblings, 1 reply; 34+ messages in thread
From: David Malcolm @ 2017-10-11 18:52 UTC (permalink / raw)
  To: Trevor Saunders, Jonathan Wakely, law; +Cc: tbsaunde+gcc, gcc-patches, palves

On Sat, 2017-08-05 at 01:39 -0400, Trevor Saunders wrote:
> On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> > On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > > aiui C++03 is C++98 with a few additions to the stl.
> > 
> > Again, STL != C++ Standard Library.
> > 
> > C++03 made some important changes to the core language,
> > particularly
> > the value-initialization rules.
> > 
> > Most of the library changes were small bug fixes, and most were to
> > locales (which didn't originate in the STL and aren't even
> > templates)
> > and iostreams (which didn't originate in the STL).
> > 
> > There were also changes to std::auto_ptr (also not from the STL)
> > which
> > this might rely on (I haven't checked).
> 
> I doubt it, as Pedro said in his email it originally copied code from
> std::auto_ptr, but it doesn't use the standard libraries definition
> of
> std::auto_ptr anywhere.  However please do feel free to look at the
> implementation.
> 
> Trev
> 

Trevor: did this go anywhere?

Do we have an unique_ptr class we can use within gcc's own
implementation?

(I was hoping to use it for
  https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html
  "[PATCH 1/3] c-family: add name_hint/deferred_diagnostic";
see the discussion at:
  https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00123.html
onwards)

Thanks
Dave

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] add unique_ptr header
  2017-10-11 18:52           ` David Malcolm
@ 2017-10-12 14:08             ` Trevor Saunders
  2017-10-13  3:05               ` [PATCH] Add gnu::unique_ptr David Malcolm
  0 siblings, 1 reply; 34+ messages in thread
From: Trevor Saunders @ 2017-10-12 14:08 UTC (permalink / raw)
  To: David Malcolm; +Cc: Jonathan Wakely, law, tbsaunde+gcc, gcc-patches, palves

On Wed, Oct 11, 2017 at 02:16:38PM -0400, David Malcolm wrote:
> On Sat, 2017-08-05 at 01:39 -0400, Trevor Saunders wrote:
> > On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> > > On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > > > aiui C++03 is C++98 with a few additions to the stl.
> > > 
> > > Again, STL != C++ Standard Library.
> > > 
> > > C++03 made some important changes to the core language,
> > > particularly
> > > the value-initialization rules.
> > > 
> > > Most of the library changes were small bug fixes, and most were to
> > > locales (which didn't originate in the STL and aren't even
> > > templates)
> > > and iostreams (which didn't originate in the STL).
> > > 
> > > There were also changes to std::auto_ptr (also not from the STL)
> > > which
> > > this might rely on (I haven't checked).
> > 
> > I doubt it, as Pedro said in his email it originally copied code from
> > std::auto_ptr, but it doesn't use the standard libraries definition
> > of
> > std::auto_ptr anywhere.  However please do feel free to look at the
> > implementation.
> > 
> > Trev
> > 
> 
> Trevor: did this go anywhere?

I'm sorry, I've been pretty busy starting a new job, and haven't yet
finished the paperwork to get permission to work on gcc stuff.  So I
haven't done anything since my last message in the thread.

> Do we have an unique_ptr class we can use within gcc's own
> implementation?

Not yet :(

> (I was hoping to use it for
>   https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html
>   "[PATCH 1/3] c-family: add name_hint/deferred_diagnostic";
> see the discussion at:
>   https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00123.html
> onwards)

I believe you should be good to take the last version of the patch I
submitted and sed the namespace name to whatever people can agree on.  I
think that was the only issue people had with the patch.  I'd really
like to see this get into gcc8, and will try to get stuff done on my
end, but I'd also be thankful if you felt like doing that work first.

Trev

> 
> Thanks
> Dave

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH] Add gnu::unique_ptr
  2017-10-12 14:08             ` Trevor Saunders
@ 2017-10-13  3:05               ` David Malcolm
  2017-10-13  9:36                 ` Richard Biener
  0 siblings, 1 reply; 34+ messages in thread
From: David Malcolm @ 2017-10-13  3:05 UTC (permalink / raw)
  To: gcc-patches; +Cc: Trevor Saunders, Jonathan Wakely, Richard Biener, Pedro Alves

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

I had a go at updating Trevor's unique_ptr patch from July
( https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02084.html )

One of the sticking points was what to call the namespace; there was
wariness about using "gtl" as the name.

Richi proposed (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00155.html):
> If it should be short use g::.  We can also use gnu:: I guess and I
> agree gnutools:: is a little long (similar to libiberty::).  Maybe
> gt:: as a short-hand for gnutools.  

Pedro noted (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00157.html):
> Exactly 3 letters has the nice property of making s/gtl::foo/std::foo/
> super trivial down the road; you don't have to care about reindenting
> stuff

Hence this version of the patch uses "gnu::" - 3 letters, one of the
ones Richi proposed, and *not* a match for ".tl" (e.g. "gtl");
(FWIW personally "gnu::" is my favorite, followed by "gcc::").

The include/unique-ptr.h in this patch is identical to that posted
by Trevor in July, with the following changes (by me):
- renaming of "gtl" to "gnu" 
- renaming of DEFINE_GDB_UNIQUE_PTR to DEFINE_GNU_UNIQUE_PTR
- renaming of xfree_deleter to xmalloc_deleter, and making it
  use "free" rather than "xfree" (which doesn't exist)

I also went and added a gcc/unique-ptr-tests.cc file containing
selftests (my thinking here is that although std::unique_ptr ought
to already be well-tested, we need to ensure that the fallback
implementation is sane when building with C++ prior to C++11).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
using gcc 4.8 for the initial bootstrap (hence testing both gnu+03
and then gnu++14 in the selftests, for stage 1 and stages 2 and 3
respectively).

I also manually tested selftests with both gcc 4.8 and trunk on
the same hardware (again, to exercise both the with/without C++11
behavior).

Tested with "make selftest-valgrind" (no new issues).

OK for trunk?

Motivation/use-cases:
(a) I have an updated version of the
name_hint/deferred_diagnostic patch kit, which uses this (rather
than the refcounting used by
https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html ); and
(b) having this available ought to allow various other cleanups
e.g. the example ones identified by Trevor here:
  https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02085.html

Thanks
Dave


Blurb from Trevor:

For most of the history of this see
  https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
The changes are mostly s/gdb/gtl/g

include/ChangeLog:

2017-07-29  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* unique-ptr.h: New file.

Combined ChangeLog follows:

gcc/ChangeLog:

	David Malcolm <dmalcolm@redhat.com>

	* Makefile.in (OBJS): Add unique-ptr-tests.o.
	* selftest-run-tests.c (selftest::run_tests): Call
	selftest::unique_ptr_tests_cc_tests.
	* selftest.h (selftest::unique_ptr_tests_cc_tests): New decl.
	* unique-ptr-tests.cc: New file.

include/ChangeLog:

	Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
	David Malcolm <dmalcolm@redhat.com>

	* unique-ptr.h: New file.
---
 gcc/Makefile.in          |   1 +
 gcc/selftest-run-tests.c |   1 +
 gcc/selftest.h           |   1 +
 gcc/unique-ptr-tests.cc  | 177 ++++++++++++++++++++++
 include/unique-ptr.h     | 386 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 566 insertions(+)
 create mode 100644 gcc/unique-ptr-tests.cc
 create mode 100644 include/unique-ptr.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 878ce7b..2809619 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1568,6 +1568,7 @@ OBJS = \
 	tree-vrp.o \
 	tree.o \
 	typed-splay-tree.o \
+	unique-ptr-tests.o \
 	valtrack.o \
 	value-prof.o \
 	var-tracking.o \
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 30e476d..b05e0fc 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -66,6 +66,7 @@ selftest::run_tests ()
   sreal_c_tests ();
   fibonacci_heap_c_tests ();
   typed_splay_tree_c_tests ();
+  unique_ptr_tests_cc_tests ();
 
   /* Mid-level data structures.  */
   input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 96eccac..adc0b68 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -194,6 +194,7 @@ extern void store_merging_c_tests ();
 extern void typed_splay_tree_c_tests ();
 extern void tree_c_tests ();
 extern void tree_cfg_c_tests ();
+extern void unique_ptr_tests_cc_tests ();
 extern void vec_c_tests ();
 extern void wide_int_cc_tests ();
 extern void predict_c_tests ();
diff --git a/gcc/unique-ptr-tests.cc b/gcc/unique-ptr-tests.cc
new file mode 100644
index 0000000..df18467
--- /dev/null
+++ b/gcc/unique-ptr-tests.cc
@@ -0,0 +1,177 @@
+/* Unit tests for unique-ptr.h.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "unique-ptr.h"
+#include "selftest.h"
+
+#if CHECKING_P
+
+namespace selftest {
+
+namespace {
+
+/* A class for counting ctor and dtor invocations.  */
+
+struct stats
+{
+  stats () : ctor_count (0), dtor_count (0) {}
+
+  int ctor_count;
+  int dtor_count;
+};
+
+/* A class that uses "stats" to track its ctor and dtor invocations.  */
+
+class foo
+{
+public:
+  foo (stats &s) : m_s (s) { ++m_s.ctor_count; }
+  ~foo () { ++m_s.dtor_count; }
+
+  int example_method () const { return 42; }
+
+private:
+  foo (const foo&);
+  foo & operator= (const foo &);
+
+private:
+  stats &m_s;
+};
+
+} // anonymous namespace
+
+/* Verify that the default ctor inits ptrs to NULL.  */
+
+static void
+test_null_ptr ()
+{
+  gnu::unique_ptr<void *> p;
+  ASSERT_EQ (NULL, p);
+
+  gnu::unique_xmalloc_ptr<void *> q;
+  ASSERT_EQ (NULL, q);
+}
+
+/* Verify that deletion happens when a unique_ptr goes out of scope.  */
+
+static void
+test_implicit_deletion ()
+{
+  stats s;
+  ASSERT_EQ (0, s.ctor_count);
+  ASSERT_EQ (0, s.dtor_count);
+
+  {
+    gnu::unique_ptr<foo> f (new foo (s));
+    ASSERT_NE (NULL, f);
+    ASSERT_EQ (1, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+  }
+
+  /* Verify that the foo was implicitly deleted.  */
+  ASSERT_EQ (1, s.ctor_count);
+  ASSERT_EQ (1, s.dtor_count);
+}
+
+/* Verify that we can assign to a NULL unique_ptr.  */
+
+static void
+test_overwrite_of_null ()
+{
+  stats s;
+  ASSERT_EQ (0, s.ctor_count);
+  ASSERT_EQ (0, s.dtor_count);
+
+  {
+    gnu::unique_ptr<foo> f;
+    ASSERT_EQ (NULL, f);
+    ASSERT_EQ (0, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+
+    /* Overwrite with a non-NULL value.  */
+    f = gnu::unique_ptr<foo> (new foo (s));
+    ASSERT_EQ (1, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+  }
+
+  /* Verify that the foo is implicitly deleted.  */
+  ASSERT_EQ (1, s.ctor_count);
+  ASSERT_EQ (1, s.dtor_count);
+}
+
+/* Verify that we can assign to a non-NULL unique_ptr.  */
+
+static void
+test_overwrite_of_non_null ()
+{
+  stats s;
+  ASSERT_EQ (0, s.ctor_count);
+  ASSERT_EQ (0, s.dtor_count);
+
+  {
+    gnu::unique_ptr<foo> f (new foo (s));
+    ASSERT_NE (NULL, f);
+    ASSERT_EQ (1, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+
+    /* Overwrite with a different value.  */
+    f = gnu::unique_ptr<foo> (new foo (s));
+    ASSERT_EQ (2, s.ctor_count);
+    ASSERT_EQ (1, s.dtor_count);
+  }
+
+  /* Verify that the 2nd foo was implicitly deleted.  */
+  ASSERT_EQ (2, s.ctor_count);
+  ASSERT_EQ (2, s.dtor_count);
+}
+
+/* Verify that unique_ptr's overloaded ops work.  */
+
+static void
+test_overloaded_ops ()
+{
+  stats s;
+  gnu::unique_ptr<foo> f (new foo (s));
+  ASSERT_EQ (42, f->example_method ());
+  ASSERT_EQ (42, (*f).example_method ());
+  ASSERT_EQ (f, f);
+  ASSERT_NE (NULL, f.get ());
+
+  gnu::unique_ptr<foo> g (new foo (s));
+  ASSERT_NE (f, g);
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+unique_ptr_tests_cc_tests ()
+{
+  test_null_ptr ();
+  test_implicit_deletion ();
+  test_overwrite_of_null ();
+  test_overwrite_of_non_null ();
+  test_overloaded_ops ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/include/unique-ptr.h b/include/unique-ptr.h
new file mode 100644
index 0000000..eddb001
--- /dev/null
+++ b/include/unique-ptr.h
@@ -0,0 +1,386 @@
+/* gnu::unique_ptr, a simple std::unique_ptr replacement for C++03.
+
+   Copyright (C) 2007-2016 Free Software Foundation, Inc.
+
+   This file is part of GCC.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* gnu::unique_ptr defines a C++ owning smart pointer that exposes a
+   subset of the std::unique_ptr API.
+
+   In fact, when compiled with a C++11 compiler, gnu::unique_ptr
+   actually _is_ std::unique_ptr.  When compiled with a C++03 compiler
+   OTOH, it's an hand coded std::unique_ptr emulation that assumes
+   code is correct and doesn't try to be too smart.
+
+   This supports custom deleters, but not _stateful_ deleters, so you
+   can't use those in C++11 mode either.  Only the managed pointer is
+   stored in the smart pointer.  That could be changed; it simply
+   wasn't found necessary.
+
+   At the end of the file you'll find a gnu::unique_ptr partial
+   specialization that uses a custom (stateless) deleter:
+   gnu::unique_xmalloc_ptr.  That is used to manage pointers to
+   objects allocated with xmalloc.
+
+   The C++03 version was originally based on GCC 7.0's std::auto_ptr
+   and then heavily customized to behave more like C++11's
+   std::unique_ptr, but at this point, it no longer shares much at all
+   with the original file.  But, that's the history and the reason for
+   the copyright's starting year.
+
+   The C++03 version lets you shoot yourself in the foot, since
+   similarly to std::auto_ptr, the copy constructor and assignment
+   operators actually move.  Also, in the name of simplicity, no
+   effort is spent on using SFINAE to prevent invalid conversions,
+   etc.  This is not really a problem, because the goal here is to
+   allow code that would be correct using std::unique_ptr to be
+   equally correct in C++03 mode, and, just as efficient.  If client
+   code compiles correctly with a C++11 (or newer) compiler, we know
+   we're not doing anything invalid by mistake.
+
+   Usage notes:
+
+   - Putting gnu::unique_ptr in standard containers is not supported,
+     since C++03 containers are not move-aware (and our emulation
+     relies on copy actually moving).
+
+   - Since there's no nullptr in C++03, gnu::unique_ptr allows
+     implicit initialization and assignment from NULL instead.
+
+   - To check whether there's an associated managed object, all these
+     work as expected:
+
+      if (ptr)
+      if (!ptr)
+      if (ptr != NULL)
+      if (ptr == NULL)
+      if (NULL != ptr)
+      if (NULL == ptr)
+*/
+
+#ifndef GNU_UNIQUE_PTR_H
+#define GNU_UNIQUE_PTR_H 1
+
+#include <memory>
+
+namespace gnu
+{
+
+#if __cplusplus >= 201103
+
+/* In C++11 mode, all we need is import the standard
+   std::unique_ptr.  */
+template<typename T> using unique_ptr = std::unique_ptr<T>;
+
+/* Pull in move as well.  */
+using std::move;
+
+#else /* C++11 */
+
+/* Default destruction policy used by gnu::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; }
+};
+
+namespace detail
+{
+/* Type used to support implicit construction from NULL:
+
+     gnu::unique_ptr<foo> func (....)
+     {
+     return NULL;
+     }
+
+   and assignment from NULL:
+
+     gnu::unique_ptr<foo> ptr (....);
+     ...
+     ptr = NULL;
+
+  It is intentionally not defined anywhere.  */
+struct nullptr_t;
+
+/* Base class of our unique_ptr emulation.  Contains code common to
+   both 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;
+
+  /* Takes ownership of a pointer.  P is a pointer to an object of
+     element_type type.  Defaults to NULL.  */
+  explicit unique_ptr_base (element_type *p = NULL) throw () : m_ptr (p) {}
+
+  /* The "move" constructor.  Really a copy constructor that actually
+     moves.  Even though std::unique_ptr is not copyable, our little
+     simpler emulation allows it, because:
+
+       - There are no rvalue references in C++03.  Our move emulation
+       instead relies on copy/assignment moving, like std::auto_ptr.
+       - RVO/NRVO requires an accessible copy constructor
+  */
+  unique_ptr_base (const unique_ptr_base &other) throw ()
+    : m_ptr (const_cast<unique_ptr_base &> (other).release ()) {}
+
+  /* Converting "move" constructor.  Really an lvalue ref converting
+     constructor that actually moves.  This allows constructs such as:
+
+      unique_ptr<Derived> func_returning_unique_ptr (.....);
+      ...
+      unique_ptr<Base> ptr = func_returning_unique_ptr (.....);
+  */
+  template<typename T1, typename D1>
+  unique_ptr_base (const unique_ptr_base<T1, D1> &other) throw ()
+    : m_ptr (const_cast<unique_ptr_base<T1, D1> &> (other).release ()) {}
+
+  /* The "move" assignment operator.  Really an lvalue ref copy
+     assignment operator that actually moves.  See comments above.  */
+  unique_ptr_base &operator= (const unique_ptr_base &other) throw ()
+  {
+    reset (const_cast<unique_ptr_base &> (other).release ());
+    return *this;
+  }
+
+  /* Converting "move" assignment.  Really an lvalue ref converting
+     copy assignment operator that moves.  See comments above.  */
+  template<typename T1, typename D1>
+  unique_ptr_base &operator= (const unique_ptr_base<T1, D1> &other) throw ()
+  {
+    reset (const_cast<unique_ptr_base<T1, D1> &> (other).release ());
+    return *this;
+  }
+
+  /* std::unique_ptr does not allow assignment, except from nullptr.
+     nullptr doesn't exist in C++03, so we allow assignment from NULL
+     instead [ptr = NULL;].
+  */
+  unique_ptr_base &operator= (detail::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::*explicit_operator_bool) () const;
+  void this_type_does_not_support_comparisons () const {}
+
+public:
+  operator explicit_operator_bool () 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;
+      }
+  }
+
+private:
+
+  /* Call the deleter.  Note we assume the deleter is "stateless".  */
+  void call_deleter ()
+  {
+    D d;
+
+    d (m_ptr);
+  }
+
+  element_type *m_ptr;
+};
+
+} /* namespace detail */
+
+/* Macro used to create a unique_ptr_base "partial specialization" --
+   a subclass that uses a specific deleter.  Basically this re-defines
+   the necessary constructors.  This is necessary because C++03
+   doesn't support inheriting constructors with "using".  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 TYPE is inheriting from.  */
+#define DEFINE_GNU_UNIQUE_PTR(TYPE)						\
+public:									\
+  explicit TYPE (T *p = NULL) throw ()					\
+    : base_type (p) {}							\
+									\
+  TYPE (const TYPE &other) throw () : base_type (other) {}		\
+									\
+  TYPE (detail::nullptr_t *) throw () : base_type (NULL) {}		\
+									\
+  template<typename T1, typename D1>					\
+  TYPE (const detail::unique_ptr_base<T1, D1> &other) throw ()		\
+    : base_type (other) {}						\
+									\
+  using base_type::operator=;
+
+/* Define single-object gnu::unique_ptr.  */
+
+template <typename T, typename D = default_delete<T> >
+class unique_ptr : public detail::unique_ptr_base<T, D>
+{
+  typedef detail::unique_ptr_base<T, D> base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_ptr)
+
+public:
+  /* Dereferencing.  */
+  T &operator* () const throw () { return *this->get (); }
+  T *operator-> () const throw () { return this->get (); }
+};
+
+/* Define gnu::unique_ptr specialization for T[].  */
+
+template <typename T, typename D>
+class unique_ptr<T[], D> : public detail::unique_ptr_base<T, D>
+{
+  typedef detail::unique_ptr_base<T, D> base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_ptr)
+
+public:
+  /* 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 detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return x.get() == y.get(); }
+
+template <typename T, typename D,
+	  typename U, typename E>
+inline bool
+operator!= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return x.get() != y.get(); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator< (const detail::unique_ptr_base<T, D> &x,
+	   const detail::unique_ptr_base<U, E> &y)
+{ return x.get() < y.get (); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator<= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return !(y < x); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator> (const detail::unique_ptr_base<T, D> &x,
+	   const detail::unique_ptr_base<U, E> &y)
+{ return y < x; }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator>= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return !(x < y); }
+
+/* std::move "emulation".  This is as simple as it can be -- no
+   attempt is made to emulate rvalue references.  Instead relies on
+   the fact that gnu::unique_ptr has move semantics like
+   std::auto_ptr.  I.e., copy/assignment actually moves.  */
+
+template<typename T, typename D>
+unique_ptr<T, D>
+move (unique_ptr<T, D> v)
+{
+  return v;
+}
+
+#endif /* C++11 */
+
+/* Define gnu::unique_xmalloc_ptr, a gnu::unique_ptr that manages
+   xmalloc'ed memory.  */
+
+/* The deleter for gnu::unique_xmalloc_ptr.  Uses free.  */
+template <typename T>
+struct xmalloc_deleter
+{
+  void operator() (T *ptr) const { free (ptr); }
+};
+
+#if __cplusplus >= 201103
+
+/* In C++11, we just import the standard unique_ptr to our namespace
+   with a custom deleter.  */
+
+template<typename T> using unique_xmalloc_ptr
+  = std::unique_ptr<T, xmalloc_deleter<T>>;
+
+#else /* C++11 */
+
+/* In C++03, we don't have template aliases, so we need to define a
+   subclass instead, and re-define the constructors, because C++03
+   doesn't support inheriting constructors either.  */
+
+template <typename T>
+class unique_xmalloc_ptr : public unique_ptr<T, xmalloc_deleter<T> >
+{
+  typedef unique_ptr<T, xmalloc_deleter<T> > base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
+};
+
+#endif /* C++11 */
+
+} /* namespace gnu */
+
+#endif /* GNU_UNIQUE_PTR_H */
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Add gnu::unique_ptr
  2017-10-13  3:05               ` [PATCH] Add gnu::unique_ptr David Malcolm
@ 2017-10-13  9:36                 ` Richard Biener
  2017-10-13 12:08                   ` Pedro Alves
  0 siblings, 1 reply; 34+ messages in thread
From: Richard Biener @ 2017-10-13  9:36 UTC (permalink / raw)
  To: David Malcolm; +Cc: GCC Patches, Trevor Saunders, Jonathan Wakely, Pedro Alves

On Fri, Oct 13, 2017 at 2:40 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> I had a go at updating Trevor's unique_ptr patch from July
> ( https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02084.html )
>
> One of the sticking points was what to call the namespace; there was
> wariness about using "gtl" as the name.
>
> Richi proposed (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00155.html):
>> If it should be short use g::.  We can also use gnu:: I guess and I
>> agree gnutools:: is a little long (similar to libiberty::).  Maybe
>> gt:: as a short-hand for gnutools.
>
> Pedro noted (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00157.html):
>> Exactly 3 letters has the nice property of making s/gtl::foo/std::foo/
>> super trivial down the road; you don't have to care about reindenting
>> stuff
>
> Hence this version of the patch uses "gnu::" - 3 letters, one of the
> ones Richi proposed, and *not* a match for ".tl" (e.g. "gtl");
> (FWIW personally "gnu::" is my favorite, followed by "gcc::").
>
> The include/unique-ptr.h in this patch is identical to that posted
> by Trevor in July, with the following changes (by me):
> - renaming of "gtl" to "gnu"
> - renaming of DEFINE_GDB_UNIQUE_PTR to DEFINE_GNU_UNIQUE_PTR
> - renaming of xfree_deleter to xmalloc_deleter, and making it
>   use "free" rather than "xfree" (which doesn't exist)
>
> I also went and added a gcc/unique-ptr-tests.cc file containing
> selftests (my thinking here is that although std::unique_ptr ought
> to already be well-tested, we need to ensure that the fallback
> implementation is sane when building with C++ prior to C++11).
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
> using gcc 4.8 for the initial bootstrap (hence testing both gnu+03
> and then gnu++14 in the selftests, for stage 1 and stages 2 and 3
> respectively).
>
> I also manually tested selftests with both gcc 4.8 and trunk on
> the same hardware (again, to exercise both the with/without C++11
> behavior).
>
> Tested with "make selftest-valgrind" (no new issues).
>
> OK for trunk?

Ok if the gdb folks approve (and will change to this version).

Thanks,
Richard.

> Motivation/use-cases:
> (a) I have an updated version of the
> name_hint/deferred_diagnostic patch kit, which uses this (rather
> than the refcounting used by
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html ); and
> (b) having this available ought to allow various other cleanups
> e.g. the example ones identified by Trevor here:
>   https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02085.html
>
> Thanks
> Dave
>
>
> Blurb from Trevor:
>
> For most of the history of this see
>   https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
> The changes are mostly s/gdb/gtl/g
>
> include/ChangeLog:
>
> 2017-07-29  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
>         * unique-ptr.h: New file.
>
> Combined ChangeLog follows:
>
> gcc/ChangeLog:
>
>         David Malcolm <dmalcolm@redhat.com>
>
>         * Makefile.in (OBJS): Add unique-ptr-tests.o.
>         * selftest-run-tests.c (selftest::run_tests): Call
>         selftest::unique_ptr_tests_cc_tests.
>         * selftest.h (selftest::unique_ptr_tests_cc_tests): New decl.
>         * unique-ptr-tests.cc: New file.
>
> include/ChangeLog:
>
>         Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>         David Malcolm <dmalcolm@redhat.com>
>
>         * unique-ptr.h: New file.
> ---
>  gcc/Makefile.in          |   1 +
>  gcc/selftest-run-tests.c |   1 +
>  gcc/selftest.h           |   1 +
>  gcc/unique-ptr-tests.cc  | 177 ++++++++++++++++++++++
>  include/unique-ptr.h     | 386 +++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 566 insertions(+)
>  create mode 100644 gcc/unique-ptr-tests.cc
>  create mode 100644 include/unique-ptr.h
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 878ce7b..2809619 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1568,6 +1568,7 @@ OBJS = \
>         tree-vrp.o \
>         tree.o \
>         typed-splay-tree.o \
> +       unique-ptr-tests.o \
>         valtrack.o \
>         value-prof.o \
>         var-tracking.o \
> diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
> index 30e476d..b05e0fc 100644
> --- a/gcc/selftest-run-tests.c
> +++ b/gcc/selftest-run-tests.c
> @@ -66,6 +66,7 @@ selftest::run_tests ()
>    sreal_c_tests ();
>    fibonacci_heap_c_tests ();
>    typed_splay_tree_c_tests ();
> +  unique_ptr_tests_cc_tests ();
>
>    /* Mid-level data structures.  */
>    input_c_tests ();
> diff --git a/gcc/selftest.h b/gcc/selftest.h
> index 96eccac..adc0b68 100644
> --- a/gcc/selftest.h
> +++ b/gcc/selftest.h
> @@ -194,6 +194,7 @@ extern void store_merging_c_tests ();
>  extern void typed_splay_tree_c_tests ();
>  extern void tree_c_tests ();
>  extern void tree_cfg_c_tests ();
> +extern void unique_ptr_tests_cc_tests ();
>  extern void vec_c_tests ();
>  extern void wide_int_cc_tests ();
>  extern void predict_c_tests ();
> diff --git a/gcc/unique-ptr-tests.cc b/gcc/unique-ptr-tests.cc
> new file mode 100644
> index 0000000..df18467
> --- /dev/null
> +++ b/gcc/unique-ptr-tests.cc
> @@ -0,0 +1,177 @@
> +/* Unit tests for unique-ptr.h.
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free
> +Software Foundation; either version 3, or (at your option) any later
> +version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "unique-ptr.h"
> +#include "selftest.h"
> +
> +#if CHECKING_P
> +
> +namespace selftest {
> +
> +namespace {
> +
> +/* A class for counting ctor and dtor invocations.  */
> +
> +struct stats
> +{
> +  stats () : ctor_count (0), dtor_count (0) {}
> +
> +  int ctor_count;
> +  int dtor_count;
> +};
> +
> +/* A class that uses "stats" to track its ctor and dtor invocations.  */
> +
> +class foo
> +{
> +public:
> +  foo (stats &s) : m_s (s) { ++m_s.ctor_count; }
> +  ~foo () { ++m_s.dtor_count; }
> +
> +  int example_method () const { return 42; }
> +
> +private:
> +  foo (const foo&);
> +  foo & operator= (const foo &);
> +
> +private:
> +  stats &m_s;
> +};
> +
> +} // anonymous namespace
> +
> +/* Verify that the default ctor inits ptrs to NULL.  */
> +
> +static void
> +test_null_ptr ()
> +{
> +  gnu::unique_ptr<void *> p;
> +  ASSERT_EQ (NULL, p);
> +
> +  gnu::unique_xmalloc_ptr<void *> q;
> +  ASSERT_EQ (NULL, q);
> +}
> +
> +/* Verify that deletion happens when a unique_ptr goes out of scope.  */
> +
> +static void
> +test_implicit_deletion ()
> +{
> +  stats s;
> +  ASSERT_EQ (0, s.ctor_count);
> +  ASSERT_EQ (0, s.dtor_count);
> +
> +  {
> +    gnu::unique_ptr<foo> f (new foo (s));
> +    ASSERT_NE (NULL, f);
> +    ASSERT_EQ (1, s.ctor_count);
> +    ASSERT_EQ (0, s.dtor_count);
> +  }
> +
> +  /* Verify that the foo was implicitly deleted.  */
> +  ASSERT_EQ (1, s.ctor_count);
> +  ASSERT_EQ (1, s.dtor_count);
> +}
> +
> +/* Verify that we can assign to a NULL unique_ptr.  */
> +
> +static void
> +test_overwrite_of_null ()
> +{
> +  stats s;
> +  ASSERT_EQ (0, s.ctor_count);
> +  ASSERT_EQ (0, s.dtor_count);
> +
> +  {
> +    gnu::unique_ptr<foo> f;
> +    ASSERT_EQ (NULL, f);
> +    ASSERT_EQ (0, s.ctor_count);
> +    ASSERT_EQ (0, s.dtor_count);
> +
> +    /* Overwrite with a non-NULL value.  */
> +    f = gnu::unique_ptr<foo> (new foo (s));
> +    ASSERT_EQ (1, s.ctor_count);
> +    ASSERT_EQ (0, s.dtor_count);
> +  }
> +
> +  /* Verify that the foo is implicitly deleted.  */
> +  ASSERT_EQ (1, s.ctor_count);
> +  ASSERT_EQ (1, s.dtor_count);
> +}
> +
> +/* Verify that we can assign to a non-NULL unique_ptr.  */
> +
> +static void
> +test_overwrite_of_non_null ()
> +{
> +  stats s;
> +  ASSERT_EQ (0, s.ctor_count);
> +  ASSERT_EQ (0, s.dtor_count);
> +
> +  {
> +    gnu::unique_ptr<foo> f (new foo (s));
> +    ASSERT_NE (NULL, f);
> +    ASSERT_EQ (1, s.ctor_count);
> +    ASSERT_EQ (0, s.dtor_count);
> +
> +    /* Overwrite with a different value.  */
> +    f = gnu::unique_ptr<foo> (new foo (s));
> +    ASSERT_EQ (2, s.ctor_count);
> +    ASSERT_EQ (1, s.dtor_count);
> +  }
> +
> +  /* Verify that the 2nd foo was implicitly deleted.  */
> +  ASSERT_EQ (2, s.ctor_count);
> +  ASSERT_EQ (2, s.dtor_count);
> +}
> +
> +/* Verify that unique_ptr's overloaded ops work.  */
> +
> +static void
> +test_overloaded_ops ()
> +{
> +  stats s;
> +  gnu::unique_ptr<foo> f (new foo (s));
> +  ASSERT_EQ (42, f->example_method ());
> +  ASSERT_EQ (42, (*f).example_method ());
> +  ASSERT_EQ (f, f);
> +  ASSERT_NE (NULL, f.get ());
> +
> +  gnu::unique_ptr<foo> g (new foo (s));
> +  ASSERT_NE (f, g);
> +}
> +
> +/* Run all of the selftests within this file.  */
> +
> +void
> +unique_ptr_tests_cc_tests ()
> +{
> +  test_null_ptr ();
> +  test_implicit_deletion ();
> +  test_overwrite_of_null ();
> +  test_overwrite_of_non_null ();
> +  test_overloaded_ops ();
> +}
> +
> +} // namespace selftest
> +
> +#endif /* #if CHECKING_P */
> diff --git a/include/unique-ptr.h b/include/unique-ptr.h
> new file mode 100644
> index 0000000..eddb001
> --- /dev/null
> +++ b/include/unique-ptr.h
> @@ -0,0 +1,386 @@
> +/* gnu::unique_ptr, a simple std::unique_ptr replacement for C++03.
> +
> +   Copyright (C) 2007-2016 Free Software Foundation, Inc.
> +
> +   This file is part of GCC.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* gnu::unique_ptr defines a C++ owning smart pointer that exposes a
> +   subset of the std::unique_ptr API.
> +
> +   In fact, when compiled with a C++11 compiler, gnu::unique_ptr
> +   actually _is_ std::unique_ptr.  When compiled with a C++03 compiler
> +   OTOH, it's an hand coded std::unique_ptr emulation that assumes
> +   code is correct and doesn't try to be too smart.
> +
> +   This supports custom deleters, but not _stateful_ deleters, so you
> +   can't use those in C++11 mode either.  Only the managed pointer is
> +   stored in the smart pointer.  That could be changed; it simply
> +   wasn't found necessary.
> +
> +   At the end of the file you'll find a gnu::unique_ptr partial
> +   specialization that uses a custom (stateless) deleter:
> +   gnu::unique_xmalloc_ptr.  That is used to manage pointers to
> +   objects allocated with xmalloc.
> +
> +   The C++03 version was originally based on GCC 7.0's std::auto_ptr
> +   and then heavily customized to behave more like C++11's
> +   std::unique_ptr, but at this point, it no longer shares much at all
> +   with the original file.  But, that's the history and the reason for
> +   the copyright's starting year.
> +
> +   The C++03 version lets you shoot yourself in the foot, since
> +   similarly to std::auto_ptr, the copy constructor and assignment
> +   operators actually move.  Also, in the name of simplicity, no
> +   effort is spent on using SFINAE to prevent invalid conversions,
> +   etc.  This is not really a problem, because the goal here is to
> +   allow code that would be correct using std::unique_ptr to be
> +   equally correct in C++03 mode, and, just as efficient.  If client
> +   code compiles correctly with a C++11 (or newer) compiler, we know
> +   we're not doing anything invalid by mistake.
> +
> +   Usage notes:
> +
> +   - Putting gnu::unique_ptr in standard containers is not supported,
> +     since C++03 containers are not move-aware (and our emulation
> +     relies on copy actually moving).
> +
> +   - Since there's no nullptr in C++03, gnu::unique_ptr allows
> +     implicit initialization and assignment from NULL instead.
> +
> +   - To check whether there's an associated managed object, all these
> +     work as expected:
> +
> +      if (ptr)
> +      if (!ptr)
> +      if (ptr != NULL)
> +      if (ptr == NULL)
> +      if (NULL != ptr)
> +      if (NULL == ptr)
> +*/
> +
> +#ifndef GNU_UNIQUE_PTR_H
> +#define GNU_UNIQUE_PTR_H 1
> +
> +#include <memory>
> +
> +namespace gnu
> +{
> +
> +#if __cplusplus >= 201103
> +
> +/* In C++11 mode, all we need is import the standard
> +   std::unique_ptr.  */
> +template<typename T> using unique_ptr = std::unique_ptr<T>;
> +
> +/* Pull in move as well.  */
> +using std::move;
> +
> +#else /* C++11 */
> +
> +/* Default destruction policy used by gnu::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; }
> +};
> +
> +namespace detail
> +{
> +/* Type used to support implicit construction from NULL:
> +
> +     gnu::unique_ptr<foo> func (....)
> +     {
> +     return NULL;
> +     }
> +
> +   and assignment from NULL:
> +
> +     gnu::unique_ptr<foo> ptr (....);
> +     ...
> +     ptr = NULL;
> +
> +  It is intentionally not defined anywhere.  */
> +struct nullptr_t;
> +
> +/* Base class of our unique_ptr emulation.  Contains code common to
> +   both 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;
> +
> +  /* Takes ownership of a pointer.  P is a pointer to an object of
> +     element_type type.  Defaults to NULL.  */
> +  explicit unique_ptr_base (element_type *p = NULL) throw () : m_ptr (p) {}
> +
> +  /* The "move" constructor.  Really a copy constructor that actually
> +     moves.  Even though std::unique_ptr is not copyable, our little
> +     simpler emulation allows it, because:
> +
> +       - There are no rvalue references in C++03.  Our move emulation
> +       instead relies on copy/assignment moving, like std::auto_ptr.
> +       - RVO/NRVO requires an accessible copy constructor
> +  */
> +  unique_ptr_base (const unique_ptr_base &other) throw ()
> +    : m_ptr (const_cast<unique_ptr_base &> (other).release ()) {}
> +
> +  /* Converting "move" constructor.  Really an lvalue ref converting
> +     constructor that actually moves.  This allows constructs such as:
> +
> +      unique_ptr<Derived> func_returning_unique_ptr (.....);
> +      ...
> +      unique_ptr<Base> ptr = func_returning_unique_ptr (.....);
> +  */
> +  template<typename T1, typename D1>
> +  unique_ptr_base (const unique_ptr_base<T1, D1> &other) throw ()
> +    : m_ptr (const_cast<unique_ptr_base<T1, D1> &> (other).release ()) {}
> +
> +  /* The "move" assignment operator.  Really an lvalue ref copy
> +     assignment operator that actually moves.  See comments above.  */
> +  unique_ptr_base &operator= (const unique_ptr_base &other) throw ()
> +  {
> +    reset (const_cast<unique_ptr_base &> (other).release ());
> +    return *this;
> +  }
> +
> +  /* Converting "move" assignment.  Really an lvalue ref converting
> +     copy assignment operator that moves.  See comments above.  */
> +  template<typename T1, typename D1>
> +  unique_ptr_base &operator= (const unique_ptr_base<T1, D1> &other) throw ()
> +  {
> +    reset (const_cast<unique_ptr_base<T1, D1> &> (other).release ());
> +    return *this;
> +  }
> +
> +  /* std::unique_ptr does not allow assignment, except from nullptr.
> +     nullptr doesn't exist in C++03, so we allow assignment from NULL
> +     instead [ptr = NULL;].
> +  */
> +  unique_ptr_base &operator= (detail::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::*explicit_operator_bool) () const;
> +  void this_type_does_not_support_comparisons () const {}
> +
> +public:
> +  operator explicit_operator_bool () 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;
> +      }
> +  }
> +
> +private:
> +
> +  /* Call the deleter.  Note we assume the deleter is "stateless".  */
> +  void call_deleter ()
> +  {
> +    D d;
> +
> +    d (m_ptr);
> +  }
> +
> +  element_type *m_ptr;
> +};
> +
> +} /* namespace detail */
> +
> +/* Macro used to create a unique_ptr_base "partial specialization" --
> +   a subclass that uses a specific deleter.  Basically this re-defines
> +   the necessary constructors.  This is necessary because C++03
> +   doesn't support inheriting constructors with "using".  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 TYPE is inheriting from.  */
> +#define DEFINE_GNU_UNIQUE_PTR(TYPE)                                            \
> +public:                                                                        \
> +  explicit TYPE (T *p = NULL) throw ()                                 \
> +    : base_type (p) {}                                                 \
> +                                                                       \
> +  TYPE (const TYPE &other) throw () : base_type (other) {}             \
> +                                                                       \
> +  TYPE (detail::nullptr_t *) throw () : base_type (NULL) {}            \
> +                                                                       \
> +  template<typename T1, typename D1>                                   \
> +  TYPE (const detail::unique_ptr_base<T1, D1> &other) throw ()         \
> +    : base_type (other) {}                                             \
> +                                                                       \
> +  using base_type::operator=;
> +
> +/* Define single-object gnu::unique_ptr.  */
> +
> +template <typename T, typename D = default_delete<T> >
> +class unique_ptr : public detail::unique_ptr_base<T, D>
> +{
> +  typedef detail::unique_ptr_base<T, D> base_type;
> +
> +  DEFINE_GNU_UNIQUE_PTR (unique_ptr)
> +
> +public:
> +  /* Dereferencing.  */
> +  T &operator* () const throw () { return *this->get (); }
> +  T *operator-> () const throw () { return this->get (); }
> +};
> +
> +/* Define gnu::unique_ptr specialization for T[].  */
> +
> +template <typename T, typename D>
> +class unique_ptr<T[], D> : public detail::unique_ptr_base<T, D>
> +{
> +  typedef detail::unique_ptr_base<T, D> base_type;
> +
> +  DEFINE_GNU_UNIQUE_PTR (unique_ptr)
> +
> +public:
> +  /* 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 detail::unique_ptr_base<T, D> &x,
> +           const detail::unique_ptr_base<U, E> &y)
> +{ return x.get() == y.get(); }
> +
> +template <typename T, typename D,
> +         typename U, typename E>
> +inline bool
> +operator!= (const detail::unique_ptr_base<T, D> &x,
> +           const detail::unique_ptr_base<U, E> &y)
> +{ return x.get() != y.get(); }
> +
> +template<typename T, typename D,
> +        typename U, typename E>
> +inline bool
> +operator< (const detail::unique_ptr_base<T, D> &x,
> +          const detail::unique_ptr_base<U, E> &y)
> +{ return x.get() < y.get (); }
> +
> +template<typename T, typename D,
> +        typename U, typename E>
> +inline bool
> +operator<= (const detail::unique_ptr_base<T, D> &x,
> +           const detail::unique_ptr_base<U, E> &y)
> +{ return !(y < x); }
> +
> +template<typename T, typename D,
> +        typename U, typename E>
> +inline bool
> +operator> (const detail::unique_ptr_base<T, D> &x,
> +          const detail::unique_ptr_base<U, E> &y)
> +{ return y < x; }
> +
> +template<typename T, typename D,
> +        typename U, typename E>
> +inline bool
> +operator>= (const detail::unique_ptr_base<T, D> &x,
> +           const detail::unique_ptr_base<U, E> &y)
> +{ return !(x < y); }
> +
> +/* std::move "emulation".  This is as simple as it can be -- no
> +   attempt is made to emulate rvalue references.  Instead relies on
> +   the fact that gnu::unique_ptr has move semantics like
> +   std::auto_ptr.  I.e., copy/assignment actually moves.  */
> +
> +template<typename T, typename D>
> +unique_ptr<T, D>
> +move (unique_ptr<T, D> v)
> +{
> +  return v;
> +}
> +
> +#endif /* C++11 */
> +
> +/* Define gnu::unique_xmalloc_ptr, a gnu::unique_ptr that manages
> +   xmalloc'ed memory.  */
> +
> +/* The deleter for gnu::unique_xmalloc_ptr.  Uses free.  */
> +template <typename T>
> +struct xmalloc_deleter
> +{
> +  void operator() (T *ptr) const { free (ptr); }
> +};
> +
> +#if __cplusplus >= 201103
> +
> +/* In C++11, we just import the standard unique_ptr to our namespace
> +   with a custom deleter.  */
> +
> +template<typename T> using unique_xmalloc_ptr
> +  = std::unique_ptr<T, xmalloc_deleter<T>>;
> +
> +#else /* C++11 */
> +
> +/* In C++03, we don't have template aliases, so we need to define a
> +   subclass instead, and re-define the constructors, because C++03
> +   doesn't support inheriting constructors either.  */
> +
> +template <typename T>
> +class unique_xmalloc_ptr : public unique_ptr<T, xmalloc_deleter<T> >
> +{
> +  typedef unique_ptr<T, xmalloc_deleter<T> > base_type;
> +
> +  DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
> +};
> +
> +#endif /* C++11 */
> +
> +} /* namespace gnu */
> +
> +#endif /* GNU_UNIQUE_PTR_H */
> --
> 1.8.5.3
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Add gnu::unique_ptr
  2017-10-13  9:36                 ` Richard Biener
@ 2017-10-13 12:08                   ` Pedro Alves
  2017-10-14  0:35                     ` [PATCH] Implement unique_xmalloc_ptr<T[]> and add more selftests David Malcolm
  0 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2017-10-13 12:08 UTC (permalink / raw)
  To: Richard Biener, David Malcolm
  Cc: GCC Patches, Trevor Saunders, Jonathan Wakely

On 10/13/2017 10:26 AM, Richard Biener wrote:
> On Fri, Oct 13, 2017 at 2:40 AM, David Malcolm <dmalcolm@redhat.com> wrote:
>> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>>
>> I had a go at updating Trevor's unique_ptr patch from July
>> ( https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02084.html )
>>
>> One of the sticking points was what to call the namespace; there was
>> wariness about using "gtl" as the name.
>>
>> Richi proposed (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00155.html):
>>> If it should be short use g::.  We can also use gnu:: I guess and I
>>> agree gnutools:: is a little long (similar to libiberty::).  Maybe
>>> gt:: as a short-hand for gnutools.
>>
>> Pedro noted (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00157.html):
>>> Exactly 3 letters has the nice property of making s/gtl::foo/std::foo/
>>> super trivial down the road; you don't have to care about reindenting
>>> stuff
>>
>> Hence this version of the patch uses "gnu::" - 3 letters, one of the
>> ones Richi proposed, and *not* a match for ".tl" (e.g. "gtl");
>> (FWIW personally "gnu::" is my favorite, followed by "gcc::").
>>
>> The include/unique-ptr.h in this patch is identical to that posted
>> by Trevor in July, with the following changes (by me):
>> - renaming of "gtl" to "gnu"
>> - renaming of DEFINE_GDB_UNIQUE_PTR to DEFINE_GNU_UNIQUE_PTR
>> - renaming of xfree_deleter to xmalloc_deleter, and making it
>>   use "free" rather than "xfree" (which doesn't exist)
>>
>> I also went and added a gcc/unique-ptr-tests.cc file containing
>> selftests (my thinking here is that although std::unique_ptr ought
>> to already be well-tested, we need to ensure that the fallback
>> implementation is sane when building with C++ prior to C++11).
>>
>> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
>> using gcc 4.8 for the initial bootstrap (hence testing both gnu+03
>> and then gnu++14 in the selftests, for stage 1 and stages 2 and 3
>> respectively).
>>
>> I also manually tested selftests with both gcc 4.8 and trunk on
>> the same hardware (again, to exercise both the with/without C++11
>> behavior).
>>
>> Tested with "make selftest-valgrind" (no new issues).
>>
>> OK for trunk?
> 
> Ok if the gdb folks approve 

The new tests looks good to me.  [ The class too, obviously. :-) ]

(and will change to this version).

GDB used this emulation/shim in master for a period, but it
no longer does, because GDB switched to requiring
C++11 (gcc >= 4.8) instead meanwhile...  I.e., GDB uses
standard std::unique_ptr nowadays.

GDB still uses gdb::unique_xmalloc_ptr though.  Might make
sense to switch to using gnu::unique_xmalloc_ptr instead.

GDB does have an xfree function that we call instead
of free throughout (that's where the reference David fixed
comes from).  We can most probably just switch to free too nowadays.

Also, gdb nowadays also has a gdb::unique_xmalloc_ptr<T[]> specialization
that this patch is missing (because the specialization was added
after switching to C++11):

  [pushed] Allow gdb::unique_xmalloc_ptr<T[]>
  https://sourceware.org/ml/gdb-patches/2017-08/msg00212.html

So we'd need that before switching.  I don't recall off hand whether
it's easy to make that work with the C++03 version.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH] Implement unique_xmalloc_ptr<T[]> and add more selftests
  2017-10-13 12:08                   ` Pedro Alves
@ 2017-10-14  0:35                     ` David Malcolm
  2017-10-16 10:53                       ` Pedro Alves
  0 siblings, 1 reply; 34+ messages in thread
From: David Malcolm @ 2017-10-14  0:35 UTC (permalink / raw)
  To: Pedro Alves, Richard Biener
  Cc: GCC Patches, Trevor Saunders, Jonathan Wakely, David Malcolm

On Fri, 2017-10-13 at 13:01 +0100, Pedro Alves wrote:
> On 10/13/2017 10:26 AM, Richard Biener wrote:
> > On Fri, Oct 13, 2017 at 2:40 AM, David Malcolm <dmalcolm@redhat.com
> > > wrote:
> > > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> > > 
> > > I had a go at updating Trevor's unique_ptr patch from July
> > > ( https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02084.html )
> > > 
> > > One of the sticking points was what to call the namespace; there
> > > was
> > > wariness about using "gtl" as the name.
> > > 
> > > Richi proposed (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg001
> > > 55.html):
> > > > If it should be short use g::.  We can also use gnu:: I guess
> > > > and I
> > > > agree gnutools:: is a little long (similar to
> > > > libiberty::).  Maybe
> > > > gt:: as a short-hand for gnutools.
> > > 
> > > Pedro noted (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00157.
> > > html):
> > > > Exactly 3 letters has the nice property of making
> > > > s/gtl::foo/std::foo/
> > > > super trivial down the road; you don't have to care about
> > > > reindenting
> > > > stuff
> > > 
> > > Hence this version of the patch uses "gnu::" - 3 letters, one of
> > > the
> > > ones Richi proposed, and *not* a match for ".tl" (e.g. "gtl");
> > > (FWIW personally "gnu::" is my favorite, followed by "gcc::").
> > > 
> > > The include/unique-ptr.h in this patch is identical to that
> > > posted
> > > by Trevor in July, with the following changes (by me):
> > > - renaming of "gtl" to "gnu"
> > > - renaming of DEFINE_GDB_UNIQUE_PTR to DEFINE_GNU_UNIQUE_PTR
> > > - renaming of xfree_deleter to xmalloc_deleter, and making it
> > >   use "free" rather than "xfree" (which doesn't exist)
> > > 
> > > I also went and added a gcc/unique-ptr-tests.cc file containing
> > > selftests (my thinking here is that although std::unique_ptr
> > > ought
> > > to already be well-tested, we need to ensure that the fallback
> > > implementation is sane when building with C++ prior to C++11).
> > > 
> > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
> > > using gcc 4.8 for the initial bootstrap (hence testing both
> > > gnu+03
> > > and then gnu++14 in the selftests, for stage 1 and stages 2 and 3
> > > respectively).
> > > 
> > > I also manually tested selftests with both gcc 4.8 and trunk on
> > > the same hardware (again, to exercise both the with/without C++11
> > > behavior).
> > > 
> > > Tested with "make selftest-valgrind" (no new issues).
> > > 
> > > OK for trunk?
> > 
> > Ok if the gdb folks approve 
> 
> The new tests looks good to me.  [ The class too, obviously. :-) ]
> 
> (and will change to this version).
> 
> GDB used this emulation/shim in master for a period, but it
> no longer does, because GDB switched to requiring
> C++11 (gcc >= 4.8) instead meanwhile...  I.e., GDB uses
> standard std::unique_ptr nowadays.
> 
> GDB still uses gdb::unique_xmalloc_ptr though.  Might make
> sense to switch to using gnu::unique_xmalloc_ptr instead.
> 
> GDB does have an xfree function that we call instead
> of free throughout (that's where the reference David fixed
> comes from).  We can most probably just switch to free too nowadays.
> 
> Also, gdb nowadays also has a gdb::unique_xmalloc_ptr<T[]>
> specialization
> that this patch is missing (because the specialization was added
> after switching to C++11):
> 
>   [pushed] Allow gdb::unique_xmalloc_ptr<T[]>
>   https://sourceware.org/ml/gdb-patches/2017-08/msg00212.html
> 
> So we'd need that before switching.  I don't recall off hand whether
> it's easy to make that work with the C++03 version.
> 
> Thanks,
> Pedro Alves

Pedro: it appears based on Richard's email that you (or another
gdb hacker) are responsible for approving this gcc change.
(presumably in the hope of sharing this header between gcc and gdb?)

(though gdb's copy doesn't seem to have a lot left in it:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/common/gdb_unique_ptr.h;hb=HEAD
is only 52 lines, including comments).

As far as I can tell from your mail, the one issue that blocks that
is the lack of gdb::unique_xmalloc_ptr<T[]>.

So here's an patch on top of the previous one which adds the
xmalloc_deleter<T[]> (taken from gdb, but changing "xfree" to
"free", and adding support for pre-C++11 dialects), along with
selftests for unique_ptr<T[]>, unique_xmalloc_ptr<T> and
unique_xmalloc_ptr<T[]>.

As before, successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
using gcc 4.8 for the initial bootstrap (hence testing both gnu++03
then gnu++14 in the selftests, for stage 1 and stages 2 and 3
respectively).

Hand-tested with "make selftest-valgrind" with both gnu++03 and
gnu++14.

Also tested stage1 on powerpc-ibm-aix7.1.3.0 ("gcc111" in the
compile farm; gcc 4.8 i.e. gnu++03)

Is this OK?

Thanks
Dave

gcc/ChangeLog:
	* unique-ptr-tests.cc (struct selftest::has_default_ctor): New
	struct.
	(struct selftest::dummy): New struct.
	(selftest::test_array_new): New function.
	(selftest::test_xmalloc): New function.
	(selftest::test_xmalloc_array): New function.
	(selftest::unique_ptr_tests_cc_tests): Call the new functions.

include/ChangeLog:
	* unique-ptr.h (struct xmalloc_deleter<T[]>): New.
	[!__cplusplus >= 201103] (class unique_xmalloc_ptr<T[]>): New.
---
 gcc/unique-ptr-tests.cc | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/unique-ptr.h    | 17 +++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/gcc/unique-ptr-tests.cc b/gcc/unique-ptr-tests.cc
index df18467..f051a77 100644
--- a/gcc/unique-ptr-tests.cc
+++ b/gcc/unique-ptr-tests.cc
@@ -57,6 +57,21 @@ private:
   stats &m_s;
 };
 
+/* A struct for testing unique_ptr<T[]>.  */
+
+struct has_default_ctor
+{
+  has_default_ctor () : m_field (42) {}
+  int m_field;
+};
+
+/* A dummy struct for testing unique_xmalloc_ptr.  */
+
+struct dummy
+{
+  int field;
+};
+
 } // anonymous namespace
 
 /* Verify that the default ctor inits ptrs to NULL.  */
@@ -160,6 +175,45 @@ test_overloaded_ops ()
   ASSERT_NE (f, g);
 }
 
+/* Verify that the gnu::unique_ptr specialization for T[] works.  */
+
+static void
+test_array_new ()
+{
+  const int num = 10;
+  gnu::unique_ptr<has_default_ctor[]> p (new has_default_ctor[num]);
+  ASSERT_NE (NULL, p.get ());
+  /* Verify that operator[] works, and that the default ctor was called
+     on each element.  */
+  for (int i = 0; i < num; i++)
+    ASSERT_EQ (42, p[i].m_field);
+}
+
+/* Verify that gnu::unique_malloc_ptr works.  */
+
+static void
+test_xmalloc ()
+{
+  gnu::unique_xmalloc_ptr<dummy> p (XNEW (dummy));
+  ASSERT_NE (NULL, p.get ());
+}
+
+/* Verify the gnu::unique_malloc_ptr specialization for T[].  */
+
+static void
+test_xmalloc_array ()
+{
+  const int num = 10;
+  gnu::unique_xmalloc_ptr<dummy[]> p (XNEWVEC (dummy, num));
+  ASSERT_NE (NULL, p.get ());
+
+  /* Verify that operator[] works.  */
+  for (int i = 0; i < num; i++)
+    p[i].field = 42;
+  for (int i = 0; i < num; i++)
+    ASSERT_EQ (42, p[i].field);
+}
+
 /* Run all of the selftests within this file.  */
 
 void
@@ -170,6 +224,9 @@ unique_ptr_tests_cc_tests ()
   test_overwrite_of_null ();
   test_overwrite_of_non_null ();
   test_overloaded_ops ();
+  test_array_new ();
+  test_xmalloc ();
+  test_xmalloc_array ();
 }
 
 } // namespace selftest
diff --git a/include/unique-ptr.h b/include/unique-ptr.h
index eddb001..c5ca031 100644
--- a/include/unique-ptr.h
+++ b/include/unique-ptr.h
@@ -357,6 +357,13 @@ struct xmalloc_deleter
   void operator() (T *ptr) const { free (ptr); }
 };
 
+/* Same, for arrays.  */
+template <typename T>
+struct xmalloc_deleter<T[]>
+{
+  void operator() (T *ptr) const { free (ptr); }
+};
+
 #if __cplusplus >= 201103
 
 /* In C++11, we just import the standard unique_ptr to our namespace
@@ -379,6 +386,16 @@ class unique_xmalloc_ptr : public unique_ptr<T, xmalloc_deleter<T> >
   DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
 };
 
+/* Define gnu::unique_xmalloc_ptr specialization for T[].  */
+
+template <typename T>
+class unique_xmalloc_ptr<T[]> : public unique_ptr<T[], xmalloc_deleter<T[]> >
+{
+  typedef unique_ptr<T[], xmalloc_deleter<T[]> > base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
+};
+
 #endif /* C++11 */
 
 } /* namespace gnu */
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Implement unique_xmalloc_ptr<T[]> and add more selftests
  2017-10-14  0:35                     ` [PATCH] Implement unique_xmalloc_ptr<T[]> and add more selftests David Malcolm
@ 2017-10-16 10:53                       ` Pedro Alves
  2017-10-16 21:04                         ` [committed] Add gnu::unique_ptr David Malcolm
  0 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2017-10-16 10:53 UTC (permalink / raw)
  To: David Malcolm, Richard Biener
  Cc: GCC Patches, Trevor Saunders, Jonathan Wakely

On 10/14/2017 12:35 AM, David Malcolm wrote:

> As far as I can tell from your mail, the one issue that blocks that
> is the lack of gdb::unique_xmalloc_ptr<T[]>.
> 
> So here's an patch on top of the previous one which adds the
> xmalloc_deleter<T[]> (taken from gdb, but changing "xfree" to
> "free", and adding support for pre-C++11 dialects), along with
> selftests for unique_ptr<T[]>, unique_xmalloc_ptr<T> and
> unique_xmalloc_ptr<T[]>.

Thanks much!

> As before, successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
> using gcc 4.8 for the initial bootstrap (hence testing both gnu++03
> then gnu++14 in the selftests, for stage 1 and stages 2 and 3
> respectively).
> 

Excellent.

> Hand-tested with "make selftest-valgrind" with both gnu++03 and
> gnu++14.
> 
> Also tested stage1 on powerpc-ibm-aix7.1.3.0 ("gcc111" in the
> compile farm; gcc 4.8 i.e. gnu++03)
> 
> Is this OK?

Looks great to me.

> +/* Verify that gnu::unique_malloc_ptr works.  */

Typo: malloc -> xmalloc.  Appears in other comments too.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [committed] Add gnu::unique_ptr
  2017-10-16 10:53                       ` Pedro Alves
@ 2017-10-16 21:04                         ` David Malcolm
  2017-10-19 17:27                           ` Gerald Pfeifer
  0 siblings, 1 reply; 34+ messages in thread
From: David Malcolm @ 2017-10-16 21:04 UTC (permalink / raw)
  To: Pedro Alves, Richard Biener; +Cc: GCC Patches, Trevor Saunders, Jonathan Wakely

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

On Mon, 2017-10-16 at 11:47 +0100, Pedro Alves wrote:
> On 10/14/2017 12:35 AM, David Malcolm wrote:
> 
> > As far as I can tell from your mail, the one issue that blocks that
> > is the lack of gdb::unique_xmalloc_ptr<T[]>.
> > 
> > So here's an patch on top of the previous one which adds the
> > xmalloc_deleter<T[]> (taken from gdb, but changing "xfree" to
> > "free", and adding support for pre-C++11 dialects), along with
> > selftests for unique_ptr<T[]>, unique_xmalloc_ptr<T> and
> > unique_xmalloc_ptr<T[]>.
> 
> Thanks much!
> 
> > As before, successfully bootstrapped & regrtested on x86_64-pc-
> > linux-gnu,
> > using gcc 4.8 for the initial bootstrap (hence testing both gnu++03
> > then gnu++14 in the selftests, for stage 1 and stages 2 and 3
> > respectively).
> > 
> 
> Excellent.
> 
> > Hand-tested with "make selftest-valgrind" with both gnu++03 and
> > gnu++14.
> > 
> > Also tested stage1 on powerpc-ibm-aix7.1.3.0 ("gcc111" in the
> > compile farm; gcc 4.8 i.e. gnu++03)
> > 
> > Is this OK?
> 
> Looks great to me.
> 
> > +/* Verify that gnu::unique_malloc_ptr works.  */
> 
> Typo: malloc -> xmalloc.  Appears in other comments too.
> 
> Thanks,
> Pedro Alves

Thanks.  I've fixed those typos, re-tested, and committed it to
gcc trunk as r253797.

For reference, here's what I committed:


This is a version of the patch posted by Trevor Saunders on 2017-07-31,
for which he wrote:
> For most of the history of this see
>   https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
> The changes are mostly s/gdb/gtl/g

This version was updated by me (dmalcolm) adding these changes:
- renaming of "gtl" to "gnu" (3 letters, and one of the ones Richi
  proposed, and not a match for "*tl")
- renaming of DEFINE_GDB_UNIQUE_PTR to DEFINE_GNU_UNIQUE_PTR
- renaming of xfree_deleter to xmalloc_deleter, and making it
  use "free" rather than "xfree" (which doesn't exist)
- added a gcc/unique-ptr-tests.cc
- implement unique_xmalloc_ptr<T[]> (taken from gdb, but changing
  "xfree" to "free", and adding support for pre-C++-11)

gcc/ChangeLog:

	David Malcolm <dmalcolm@redhat.com>

	* Makefile.in (OBJS): Add unique-ptr-tests.o.
	* selftest-run-tests.c (selftest::run_tests): Call
	selftest::unique_ptr_tests_cc_tests.
	* selftest.h (selftest::unique_ptr_tests_cc_tests): New decl.
	* unique-ptr-tests.cc: New file.

include/ChangeLog:

	Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
	David Malcolm <dmalcolm@redhat.com>

	* unique-ptr.h: New file.
---
 gcc/Makefile.in          |   1 +
 gcc/selftest-run-tests.c |   1 +
 gcc/selftest.h           |   1 +
 gcc/unique-ptr-tests.cc  | 234 +++++++++++++++++++++++++++
 include/unique-ptr.h     | 403 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 640 insertions(+)
 create mode 100644 gcc/unique-ptr-tests.cc
 create mode 100644 include/unique-ptr.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 878ce7b..2809619 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1568,6 +1568,7 @@ OBJS = \
 	tree-vrp.o \
 	tree.o \
 	typed-splay-tree.o \
+	unique-ptr-tests.o \
 	valtrack.o \
 	value-prof.o \
 	var-tracking.o \
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 30e476d..b05e0fc 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -66,6 +66,7 @@ selftest::run_tests ()
   sreal_c_tests ();
   fibonacci_heap_c_tests ();
   typed_splay_tree_c_tests ();
+  unique_ptr_tests_cc_tests ();
 
   /* Mid-level data structures.  */
   input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 96eccac..adc0b68 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -194,6 +194,7 @@ extern void store_merging_c_tests ();
 extern void typed_splay_tree_c_tests ();
 extern void tree_c_tests ();
 extern void tree_cfg_c_tests ();
+extern void unique_ptr_tests_cc_tests ();
 extern void vec_c_tests ();
 extern void wide_int_cc_tests ();
 extern void predict_c_tests ();
diff --git a/gcc/unique-ptr-tests.cc b/gcc/unique-ptr-tests.cc
new file mode 100644
index 0000000..f5b72a8
--- /dev/null
+++ b/gcc/unique-ptr-tests.cc
@@ -0,0 +1,234 @@
+/* Unit tests for unique-ptr.h.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "unique-ptr.h"
+#include "selftest.h"
+
+#if CHECKING_P
+
+namespace selftest {
+
+namespace {
+
+/* A class for counting ctor and dtor invocations.  */
+
+struct stats
+{
+  stats () : ctor_count (0), dtor_count (0) {}
+
+  int ctor_count;
+  int dtor_count;
+};
+
+/* A class that uses "stats" to track its ctor and dtor invocations.  */
+
+class foo
+{
+public:
+  foo (stats &s) : m_s (s) { ++m_s.ctor_count; }
+  ~foo () { ++m_s.dtor_count; }
+
+  int example_method () const { return 42; }
+
+private:
+  foo (const foo&);
+  foo & operator= (const foo &);
+
+private:
+  stats &m_s;
+};
+
+/* A struct for testing unique_ptr<T[]>.  */
+
+struct has_default_ctor
+{
+  has_default_ctor () : m_field (42) {}
+  int m_field;
+};
+
+/* A dummy struct for testing unique_xmalloc_ptr.  */
+
+struct dummy
+{
+  int field;
+};
+
+} // anonymous namespace
+
+/* Verify that the default ctor inits ptrs to NULL.  */
+
+static void
+test_null_ptr ()
+{
+  gnu::unique_ptr<void *> p;
+  ASSERT_EQ (NULL, p);
+
+  gnu::unique_xmalloc_ptr<void *> q;
+  ASSERT_EQ (NULL, q);
+}
+
+/* Verify that deletion happens when a unique_ptr goes out of scope.  */
+
+static void
+test_implicit_deletion ()
+{
+  stats s;
+  ASSERT_EQ (0, s.ctor_count);
+  ASSERT_EQ (0, s.dtor_count);
+
+  {
+    gnu::unique_ptr<foo> f (new foo (s));
+    ASSERT_NE (NULL, f);
+    ASSERT_EQ (1, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+  }
+
+  /* Verify that the foo was implicitly deleted.  */
+  ASSERT_EQ (1, s.ctor_count);
+  ASSERT_EQ (1, s.dtor_count);
+}
+
+/* Verify that we can assign to a NULL unique_ptr.  */
+
+static void
+test_overwrite_of_null ()
+{
+  stats s;
+  ASSERT_EQ (0, s.ctor_count);
+  ASSERT_EQ (0, s.dtor_count);
+
+  {
+    gnu::unique_ptr<foo> f;
+    ASSERT_EQ (NULL, f);
+    ASSERT_EQ (0, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+
+    /* Overwrite with a non-NULL value.  */
+    f = gnu::unique_ptr<foo> (new foo (s));
+    ASSERT_EQ (1, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+  }
+
+  /* Verify that the foo is implicitly deleted.  */
+  ASSERT_EQ (1, s.ctor_count);
+  ASSERT_EQ (1, s.dtor_count);
+}
+
+/* Verify that we can assign to a non-NULL unique_ptr.  */
+
+static void
+test_overwrite_of_non_null ()
+{
+  stats s;
+  ASSERT_EQ (0, s.ctor_count);
+  ASSERT_EQ (0, s.dtor_count);
+
+  {
+    gnu::unique_ptr<foo> f (new foo (s));
+    ASSERT_NE (NULL, f);
+    ASSERT_EQ (1, s.ctor_count);
+    ASSERT_EQ (0, s.dtor_count);
+
+    /* Overwrite with a different value.  */
+    f = gnu::unique_ptr<foo> (new foo (s));
+    ASSERT_EQ (2, s.ctor_count);
+    ASSERT_EQ (1, s.dtor_count);
+  }
+
+  /* Verify that the 2nd foo was implicitly deleted.  */
+  ASSERT_EQ (2, s.ctor_count);
+  ASSERT_EQ (2, s.dtor_count);
+}
+
+/* Verify that unique_ptr's overloaded ops work.  */
+
+static void
+test_overloaded_ops ()
+{
+  stats s;
+  gnu::unique_ptr<foo> f (new foo (s));
+  ASSERT_EQ (42, f->example_method ());
+  ASSERT_EQ (42, (*f).example_method ());
+  ASSERT_EQ (f, f);
+  ASSERT_NE (NULL, f.get ());
+
+  gnu::unique_ptr<foo> g (new foo (s));
+  ASSERT_NE (f, g);
+}
+
+/* Verify that the gnu::unique_ptr specialization for T[] works.  */
+
+static void
+test_array_new ()
+{
+  const int num = 10;
+  gnu::unique_ptr<has_default_ctor[]> p (new has_default_ctor[num]);
+  ASSERT_NE (NULL, p.get ());
+  /* Verify that operator[] works, and that the default ctor was called
+     on each element.  */
+  for (int i = 0; i < num; i++)
+    ASSERT_EQ (42, p[i].m_field);
+}
+
+/* Verify that gnu::unique_xmalloc_ptr works.  */
+
+static void
+test_xmalloc ()
+{
+  gnu::unique_xmalloc_ptr<dummy> p (XNEW (dummy));
+  ASSERT_NE (NULL, p.get ());
+}
+
+/* Verify the gnu::unique_xmalloc_ptr specialization for T[].  */
+
+static void
+test_xmalloc_array ()
+{
+  const int num = 10;
+  gnu::unique_xmalloc_ptr<dummy[]> p (XNEWVEC (dummy, num));
+  ASSERT_NE (NULL, p.get ());
+
+  /* Verify that operator[] works.  */
+  for (int i = 0; i < num; i++)
+    p[i].field = 42;
+  for (int i = 0; i < num; i++)
+    ASSERT_EQ (42, p[i].field);
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+unique_ptr_tests_cc_tests ()
+{
+  test_null_ptr ();
+  test_implicit_deletion ();
+  test_overwrite_of_null ();
+  test_overwrite_of_non_null ();
+  test_overloaded_ops ();
+  test_array_new ();
+  test_xmalloc ();
+  test_xmalloc_array ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/include/unique-ptr.h b/include/unique-ptr.h
new file mode 100644
index 0000000..c5ca031
--- /dev/null
+++ b/include/unique-ptr.h
@@ -0,0 +1,403 @@
+/* gnu::unique_ptr, a simple std::unique_ptr replacement for C++03.
+
+   Copyright (C) 2007-2016 Free Software Foundation, Inc.
+
+   This file is part of GCC.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* gnu::unique_ptr defines a C++ owning smart pointer that exposes a
+   subset of the std::unique_ptr API.
+
+   In fact, when compiled with a C++11 compiler, gnu::unique_ptr
+   actually _is_ std::unique_ptr.  When compiled with a C++03 compiler
+   OTOH, it's an hand coded std::unique_ptr emulation that assumes
+   code is correct and doesn't try to be too smart.
+
+   This supports custom deleters, but not _stateful_ deleters, so you
+   can't use those in C++11 mode either.  Only the managed pointer is
+   stored in the smart pointer.  That could be changed; it simply
+   wasn't found necessary.
+
+   At the end of the file you'll find a gnu::unique_ptr partial
+   specialization that uses a custom (stateless) deleter:
+   gnu::unique_xmalloc_ptr.  That is used to manage pointers to
+   objects allocated with xmalloc.
+
+   The C++03 version was originally based on GCC 7.0's std::auto_ptr
+   and then heavily customized to behave more like C++11's
+   std::unique_ptr, but at this point, it no longer shares much at all
+   with the original file.  But, that's the history and the reason for
+   the copyright's starting year.
+
+   The C++03 version lets you shoot yourself in the foot, since
+   similarly to std::auto_ptr, the copy constructor and assignment
+   operators actually move.  Also, in the name of simplicity, no
+   effort is spent on using SFINAE to prevent invalid conversions,
+   etc.  This is not really a problem, because the goal here is to
+   allow code that would be correct using std::unique_ptr to be
+   equally correct in C++03 mode, and, just as efficient.  If client
+   code compiles correctly with a C++11 (or newer) compiler, we know
+   we're not doing anything invalid by mistake.
+
+   Usage notes:
+
+   - Putting gnu::unique_ptr in standard containers is not supported,
+     since C++03 containers are not move-aware (and our emulation
+     relies on copy actually moving).
+
+   - Since there's no nullptr in C++03, gnu::unique_ptr allows
+     implicit initialization and assignment from NULL instead.
+
+   - To check whether there's an associated managed object, all these
+     work as expected:
+
+      if (ptr)
+      if (!ptr)
+      if (ptr != NULL)
+      if (ptr == NULL)
+      if (NULL != ptr)
+      if (NULL == ptr)
+*/
+
+#ifndef GNU_UNIQUE_PTR_H
+#define GNU_UNIQUE_PTR_H 1
+
+#include <memory>
+
+namespace gnu
+{
+
+#if __cplusplus >= 201103
+
+/* In C++11 mode, all we need is import the standard
+   std::unique_ptr.  */
+template<typename T> using unique_ptr = std::unique_ptr<T>;
+
+/* Pull in move as well.  */
+using std::move;
+
+#else /* C++11 */
+
+/* Default destruction policy used by gnu::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; }
+};
+
+namespace detail
+{
+/* Type used to support implicit construction from NULL:
+
+     gnu::unique_ptr<foo> func (....)
+     {
+     return NULL;
+     }
+
+   and assignment from NULL:
+
+     gnu::unique_ptr<foo> ptr (....);
+     ...
+     ptr = NULL;
+
+  It is intentionally not defined anywhere.  */
+struct nullptr_t;
+
+/* Base class of our unique_ptr emulation.  Contains code common to
+   both 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;
+
+  /* Takes ownership of a pointer.  P is a pointer to an object of
+     element_type type.  Defaults to NULL.  */
+  explicit unique_ptr_base (element_type *p = NULL) throw () : m_ptr (p) {}
+
+  /* The "move" constructor.  Really a copy constructor that actually
+     moves.  Even though std::unique_ptr is not copyable, our little
+     simpler emulation allows it, because:
+
+       - There are no rvalue references in C++03.  Our move emulation
+       instead relies on copy/assignment moving, like std::auto_ptr.
+       - RVO/NRVO requires an accessible copy constructor
+  */
+  unique_ptr_base (const unique_ptr_base &other) throw ()
+    : m_ptr (const_cast<unique_ptr_base &> (other).release ()) {}
+
+  /* Converting "move" constructor.  Really an lvalue ref converting
+     constructor that actually moves.  This allows constructs such as:
+
+      unique_ptr<Derived> func_returning_unique_ptr (.....);
+      ...
+      unique_ptr<Base> ptr = func_returning_unique_ptr (.....);
+  */
+  template<typename T1, typename D1>
+  unique_ptr_base (const unique_ptr_base<T1, D1> &other) throw ()
+    : m_ptr (const_cast<unique_ptr_base<T1, D1> &> (other).release ()) {}
+
+  /* The "move" assignment operator.  Really an lvalue ref copy
+     assignment operator that actually moves.  See comments above.  */
+  unique_ptr_base &operator= (const unique_ptr_base &other) throw ()
+  {
+    reset (const_cast<unique_ptr_base &> (other).release ());
+    return *this;
+  }
+
+  /* Converting "move" assignment.  Really an lvalue ref converting
+     copy assignment operator that moves.  See comments above.  */
+  template<typename T1, typename D1>
+  unique_ptr_base &operator= (const unique_ptr_base<T1, D1> &other) throw ()
+  {
+    reset (const_cast<unique_ptr_base<T1, D1> &> (other).release ());
+    return *this;
+  }
+
+  /* std::unique_ptr does not allow assignment, except from nullptr.
+     nullptr doesn't exist in C++03, so we allow assignment from NULL
+     instead [ptr = NULL;].
+  */
+  unique_ptr_base &operator= (detail::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::*explicit_operator_bool) () const;
+  void this_type_does_not_support_comparisons () const {}
+
+public:
+  operator explicit_operator_bool () 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;
+      }
+  }
+
+private:
+
+  /* Call the deleter.  Note we assume the deleter is "stateless".  */
+  void call_deleter ()
+  {
+    D d;
+
+    d (m_ptr);
+  }
+
+  element_type *m_ptr;
+};
+
+} /* namespace detail */
+
+/* Macro used to create a unique_ptr_base "partial specialization" --
+   a subclass that uses a specific deleter.  Basically this re-defines
+   the necessary constructors.  This is necessary because C++03
+   doesn't support inheriting constructors with "using".  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 TYPE is inheriting from.  */
+#define DEFINE_GNU_UNIQUE_PTR(TYPE)						\
+public:									\
+  explicit TYPE (T *p = NULL) throw ()					\
+    : base_type (p) {}							\
+									\
+  TYPE (const TYPE &other) throw () : base_type (other) {}		\
+									\
+  TYPE (detail::nullptr_t *) throw () : base_type (NULL) {}		\
+									\
+  template<typename T1, typename D1>					\
+  TYPE (const detail::unique_ptr_base<T1, D1> &other) throw ()		\
+    : base_type (other) {}						\
+									\
+  using base_type::operator=;
+
+/* Define single-object gnu::unique_ptr.  */
+
+template <typename T, typename D = default_delete<T> >
+class unique_ptr : public detail::unique_ptr_base<T, D>
+{
+  typedef detail::unique_ptr_base<T, D> base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_ptr)
+
+public:
+  /* Dereferencing.  */
+  T &operator* () const throw () { return *this->get (); }
+  T *operator-> () const throw () { return this->get (); }
+};
+
+/* Define gnu::unique_ptr specialization for T[].  */
+
+template <typename T, typename D>
+class unique_ptr<T[], D> : public detail::unique_ptr_base<T, D>
+{
+  typedef detail::unique_ptr_base<T, D> base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_ptr)
+
+public:
+  /* 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 detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return x.get() == y.get(); }
+
+template <typename T, typename D,
+	  typename U, typename E>
+inline bool
+operator!= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return x.get() != y.get(); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator< (const detail::unique_ptr_base<T, D> &x,
+	   const detail::unique_ptr_base<U, E> &y)
+{ return x.get() < y.get (); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator<= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return !(y < x); }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator> (const detail::unique_ptr_base<T, D> &x,
+	   const detail::unique_ptr_base<U, E> &y)
+{ return y < x; }
+
+template<typename T, typename D,
+	 typename U, typename E>
+inline bool
+operator>= (const detail::unique_ptr_base<T, D> &x,
+	    const detail::unique_ptr_base<U, E> &y)
+{ return !(x < y); }
+
+/* std::move "emulation".  This is as simple as it can be -- no
+   attempt is made to emulate rvalue references.  Instead relies on
+   the fact that gnu::unique_ptr has move semantics like
+   std::auto_ptr.  I.e., copy/assignment actually moves.  */
+
+template<typename T, typename D>
+unique_ptr<T, D>
+move (unique_ptr<T, D> v)
+{
+  return v;
+}
+
+#endif /* C++11 */
+
+/* Define gnu::unique_xmalloc_ptr, a gnu::unique_ptr that manages
+   xmalloc'ed memory.  */
+
+/* The deleter for gnu::unique_xmalloc_ptr.  Uses free.  */
+template <typename T>
+struct xmalloc_deleter
+{
+  void operator() (T *ptr) const { free (ptr); }
+};
+
+/* Same, for arrays.  */
+template <typename T>
+struct xmalloc_deleter<T[]>
+{
+  void operator() (T *ptr) const { free (ptr); }
+};
+
+#if __cplusplus >= 201103
+
+/* In C++11, we just import the standard unique_ptr to our namespace
+   with a custom deleter.  */
+
+template<typename T> using unique_xmalloc_ptr
+  = std::unique_ptr<T, xmalloc_deleter<T>>;
+
+#else /* C++11 */
+
+/* In C++03, we don't have template aliases, so we need to define a
+   subclass instead, and re-define the constructors, because C++03
+   doesn't support inheriting constructors either.  */
+
+template <typename T>
+class unique_xmalloc_ptr : public unique_ptr<T, xmalloc_deleter<T> >
+{
+  typedef unique_ptr<T, xmalloc_deleter<T> > base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
+};
+
+/* Define gnu::unique_xmalloc_ptr specialization for T[].  */
+
+template <typename T>
+class unique_xmalloc_ptr<T[]> : public unique_ptr<T[], xmalloc_deleter<T[]> >
+{
+  typedef unique_ptr<T[], xmalloc_deleter<T[]> > base_type;
+
+  DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
+};
+
+#endif /* C++11 */
+
+} /* namespace gnu */
+
+#endif /* GNU_UNIQUE_PTR_H */
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [committed] Add gnu::unique_ptr
  2017-10-16 21:04                         ` [committed] Add gnu::unique_ptr David Malcolm
@ 2017-10-19 17:27                           ` Gerald Pfeifer
  2017-10-19 17:41                             ` David Malcolm
  0 siblings, 1 reply; 34+ messages in thread
From: Gerald Pfeifer @ 2017-10-19 17:27 UTC (permalink / raw)
  To: David Malcolm
  Cc: Pedro Alves, Richard Biener, gcc-patches, Trevor Saunders,
	Jonathan Wakely

On Mon, 16 Oct 2017, David Malcolm wrote:
> For reference, here's what I committed:

I'm afraid this may have broken the bootstrap with clang?

In file included from /scratch/tmp/gerald/gcc-HEAD/gcc/unique-ptr-tests.cc:23:
In file included from /scratch/tmp/gerald/gcc-HEAD/gcc/../include/unique-ptr.h:77:
In file included from /usr/include/c++/v1/memory:629:
/usr/include/c++/v1/typeinfo:199:2: error: no member named 'fancy_abort' in namespace 'std::__1'; did you mean simply 'fancy_abort'?
        _VSTD::abort();
        ^~~~~~~
/usr/include/c++/v1/__config:390:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_NAMESPACE
              ^
/scratch/tmp/gerald/gcc-HEAD/gcc/system.h:725:13: note: 'fancy_abort' declared here
extern void fancy_abort (const char *, int, const char *)
            ^


This is FreeBSD clang version 4.0.0 (tags/RELEASE_400/final 297347) 
(based on LLVM 4.0.0), on x86_64-unknown-freebsd11.1.

Gerald

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [committed] Add gnu::unique_ptr
  2017-10-19 17:27                           ` Gerald Pfeifer
@ 2017-10-19 17:41                             ` David Malcolm
  2017-10-22 11:40                               ` Gerald Pfeifer
  0 siblings, 1 reply; 34+ messages in thread
From: David Malcolm @ 2017-10-19 17:41 UTC (permalink / raw)
  To: Gerald Pfeifer
  Cc: Pedro Alves, Richard Biener, gcc-patches, Trevor Saunders,
	Jonathan Wakely

On Thu, 2017-10-19 at 19:23 +0200, Gerald Pfeifer wrote:
> On Mon, 16 Oct 2017, David Malcolm wrote:
> > For reference, here's what I committed:
> 
> I'm afraid this may have broken the bootstrap with clang?
> 
> In file included from /scratch/tmp/gerald/gcc-HEAD/gcc/unique-ptr-
> tests.cc:23:
> In file included from /scratch/tmp/gerald/gcc-
> HEAD/gcc/../include/unique-ptr.h:77:
> In file included from /usr/include/c++/v1/memory:629:
> /usr/include/c++/v1/typeinfo:199:2: error: no member named
> 'fancy_abort' in namespace 'std::__1'; did you mean simply
> 'fancy_abort'?
>         _VSTD::abort();
>         ^~~~~~~
> /usr/include/c++/v1/__config:390:15: note: expanded from macro
> '_VSTD'
> #define _VSTD std::_LIBCPP_NAMESPACE
>               ^
> /scratch/tmp/gerald/gcc-HEAD/gcc/system.h:725:13: note: 'fancy_abort'
> declared here
> extern void fancy_abort (const char *, int, const char *)
>             ^
> 
> 
> This is FreeBSD clang version 4.0.0 (tags/RELEASE_400/final 297347) 
> (based on LLVM 4.0.0), on x86_64-unknown-freebsd11.1.
> 
> Gerald

Sorry about the breakage.

There seem to have been similar problems on OS X:
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82610

The proposed fix there is to include <memory> in system.h, which
presumably would fix this also.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [committed] Add gnu::unique_ptr
  2017-10-19 17:41                             ` David Malcolm
@ 2017-10-22 11:40                               ` Gerald Pfeifer
  0 siblings, 0 replies; 34+ messages in thread
From: Gerald Pfeifer @ 2017-10-22 11:40 UTC (permalink / raw)
  To: David Malcolm
  Cc: Pedro Alves, Richard Biener, gcc-patches, Trevor Saunders,
	Jonathan Wakely

On Thu, 19 Oct 2017, David Malcolm wrote:
>> In file included from /scratch/tmp/gerald/gcc-HEAD/gcc/unique-ptr-tests.cc:23:
>> In file included from /scratch/tmp/gerald/gcc-HEAD/gcc/../include/unique-ptr.h:77:
>> In file included from /usr/include/c++/v1/memory:629:
>> /usr/include/c++/v1/typeinfo:199:2: error: no member named
>> 'fancy_abort' in namespace 'std::__1'; did you mean simply 'fancy_abort'?
>>         _VSTD::abort();
>>         ^~~~~~~
>> /usr/include/c++/v1/__config:390:15: note: expanded from macro '_VSTD'
>> #define _VSTD std::_LIBCPP_NAMESPACE
> There seem to have been similar problems on OS X:
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82610

Yes, I believe it's the same actually (unearthed by clang as system
compiler).

> The proposed fix there is to include <memory> in system.h, which
> presumably would fix this also.

That appears to work around the bootstrap failure on my tester as
well.

How can we go about fixing this in the tree?

Gerald

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] add unique_ptr class
@ 2017-08-14  8:26 Ville Voutilainen
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Voutilainen @ 2017-08-14  8:26 UTC (permalink / raw)
  To: gcc-patches, Jonathan Wakely, palves, tbsaunde+gcc

>Good point. "gnutools" might be more accurate, but people might object
>to adding 10 extra characters for "gnutools::".

>Naming is important, especially for a whole namespace (not just a
>single type) so I do think it's worth spending time getting it right.

>But I could live with gtl as long as nobody ever says "GTL is the GNU
>STL" or mentions "gtl" and STL in the same breath :-)

I wonder why such things wouldn't live in a namespace called 'gcc'.

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2017-10-22  7:28 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-31 23:46 [PATCH 0/2] add unique_ptr class tbsaunde+gcc
2017-07-31 23:46 ` [PATCH 1/2] add unique_ptr header tbsaunde+gcc
2017-08-01 14:40   ` David Malcolm
2017-08-02  3:09     ` Trevor Saunders
2017-08-04 19:55       ` Jonathan Wakely
2017-08-05  5:39         ` Trevor Saunders
2017-10-11 18:52           ` David Malcolm
2017-10-12 14:08             ` Trevor Saunders
2017-10-13  3:05               ` [PATCH] Add gnu::unique_ptr David Malcolm
2017-10-13  9:36                 ` Richard Biener
2017-10-13 12:08                   ` Pedro Alves
2017-10-14  0:35                     ` [PATCH] Implement unique_xmalloc_ptr<T[]> and add more selftests David Malcolm
2017-10-16 10:53                       ` Pedro Alves
2017-10-16 21:04                         ` [committed] Add gnu::unique_ptr David Malcolm
2017-10-19 17:27                           ` Gerald Pfeifer
2017-10-19 17:41                             ` David Malcolm
2017-10-22 11:40                               ` Gerald Pfeifer
2017-08-02  8:14     ` [PATCH 1/2] add unique_ptr header Richard Biener
2017-07-31 23:46 ` [PATCH 2/2] use unique_ptr some tbsaunde+gcc
2017-08-01  0:51 ` [PATCH 0/2] add unique_ptr class David Malcolm
2017-08-04 18:52 ` Jonathan Wakely
2017-08-05  5:36   ` Trevor Saunders
2017-08-07 13:01     ` Jonathan Wakely
2017-08-05 19:05   ` Pedro Alves
2017-08-11 21:30     ` Jonathan Wakely
2017-09-03  1:24       ` Trevor Saunders
2017-09-04  9:31       ` Richard Biener
2017-09-04 10:05         ` Pedro Alves
2017-09-05 16:52     ` Manuel López-Ibáñez
2017-09-05 17:40       ` Pedro Alves
2017-09-06 12:35         ` replace libiberty with gnulib (was: Re: [PATCH 0/2] add unique_ptr class) Manuel López-Ibáñez
2017-08-04 22:43 ` [PATCH 0/2] add unique_ptr class Martin Sebor
2017-08-05  5:32   ` Trevor Saunders
2017-08-14  8:26 Ville Voutilainen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).