public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use
@ 2023-08-14 13:42 Andrew Burgess
  2023-08-14 13:42 ` [PATCH 1/3] gdb: add gdb::make_unique function Andrew Burgess
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Andrew Burgess @ 2023-08-14 13:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess

This started while working on another series, in order to avoid
calling 'new' directly, I really wanted a std::make_unique function --
but that's C++14, so not available for GDB right now.

So I added a gdb::make_unique, which should be equivalent to the C++14
function.

The last two commits are additional make_unique/unique_ptr changes,
but target specific areas that needed slightly more refactoring.

---

Andrew Burgess (3):
  gdb: add gdb::make_unique function
  gdb: have mi_out_new return std::unique_ptr
  gdb: remove mi_parse::make functions

 gdb/addrmap.c                          |  2 +-
 gdb/break-catch-load.c                 |  5 +-
 gdb/compile/compile-c-support.c        |  2 +-
 gdb/cp-name-parser.y                   |  2 +-
 gdb/cp-support.c                       |  2 +-
 gdb/dwarf2/frame.c                     |  2 +-
 gdb/dwarf2/read-debug-names.c          |  2 +-
 gdb/dwarf2/read-gdb-index.c            |  2 +-
 gdb/dwarf2/read.c                      |  2 +-
 gdb/gdbtypes.c                         |  2 +-
 gdb/mi/mi-interp.c                     |  2 +-
 gdb/mi/mi-main.c                       |  4 +-
 gdb/mi/mi-out.c                        |  8 +--
 gdb/mi/mi-out.h                        |  2 +-
 gdb/mi/mi-parse.c                      | 76 ++++++++++++--------------
 gdb/mi/mi-parse.h                      | 18 ++----
 gdb/python/py-mi.c                     |  4 +-
 gdb/python/py-varobj.c                 |  4 +-
 gdb/ui-out.c                           |  8 +--
 gdb/unittests/parallel-for-selftests.c |  4 +-
 gdb/varobj.c                           |  2 +-
 gdbsupport/gdb_unique_ptr.h            |  9 +++
 22 files changed, 78 insertions(+), 86 deletions(-)


base-commit: 86dfe011797b3e442622d427e9abd1e0f70f3a62
-- 
2.25.4


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

* [PATCH 1/3] gdb: add gdb::make_unique function
  2023-08-14 13:42 [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Andrew Burgess
@ 2023-08-14 13:42 ` Andrew Burgess
  2023-08-14 15:38   ` Tom Tromey
  2023-08-14 13:42 ` [PATCH 2/3] gdb: have mi_out_new return std::unique_ptr Andrew Burgess
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2023-08-14 13:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess

While GDB is still C++11, lets add a gdb::make_unique template
function that can be used to create std::unique_ptr objects, just like
the C++14 std::make_unique.

When we move to C++14 we can either alias gdb::make_unique to
std::make_unique, or just replace the 'gdb::' prefix throughout.

I've make use of this function in all the places I think this can
easily be used, though I'm sure I've probably missed some.

Should be no user visible changes after this commit.
---
 gdb/addrmap.c                          | 2 +-
 gdb/break-catch-load.c                 | 5 ++---
 gdb/compile/compile-c-support.c        | 2 +-
 gdb/cp-name-parser.y                   | 2 +-
 gdb/cp-support.c                       | 2 +-
 gdb/dwarf2/frame.c                     | 2 +-
 gdb/dwarf2/read-debug-names.c          | 2 +-
 gdb/dwarf2/read-gdb-index.c            | 2 +-
 gdb/dwarf2/read.c                      | 2 +-
 gdb/gdbtypes.c                         | 2 +-
 gdb/python/py-varobj.c                 | 4 +---
 gdb/ui-out.c                           | 8 ++++----
 gdb/unittests/parallel-for-selftests.c | 4 ++--
 gdb/varobj.c                           | 2 +-
 gdbsupport/gdb_unique_ptr.h            | 9 +++++++++
 15 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 33dc7762d91..d16775d49d4 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -428,7 +428,7 @@ test_addrmap ()
 
   /* Create mutable addrmap.  */
   auto_obstack temp_obstack;
-  std::unique_ptr<struct addrmap_mutable> map (new addrmap_mutable);
+  auto map = gdb::make_unique<struct addrmap_mutable> ();
   SELF_CHECK (map != nullptr);
 
   /* Check initial state.  */
diff --git a/gdb/break-catch-load.c b/gdb/break-catch-load.c
index 440b42852bb..94d8b420d32 100644
--- a/gdb/break-catch-load.c
+++ b/gdb/break-catch-load.c
@@ -230,9 +230,8 @@ add_solib_catchpoint (const char *arg, bool is_load, bool is_temp, bool enabled)
   if (*arg == '\0')
     arg = nullptr;
 
-  std::unique_ptr<solib_catchpoint> c (new solib_catchpoint (gdbarch, is_temp,
-							     nullptr,
-							     is_load, arg));
+  auto c = gdb::make_unique<solib_catchpoint> (gdbarch, is_temp, nullptr,
+					       is_load, arg);
 
   c->enable_state = enabled ? bp_enabled : bp_disabled;
 
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index f9b32205b7a..53b7285b366 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -118,7 +118,7 @@ get_compile_context (const char *fe_libcc, const char *fe_context,
     error (_("The loaded version of GCC does not support the required version "
 	     "of the API."));
 
-  return std::unique_ptr<compile_instance> (new INSTTYPE (context));
+  return gdb::make_unique<INSTTYPE> (context);
 }
 
 /* A C-language implementation of get_compile_context.  */
diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 80188074202..324166a03ff 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -2038,7 +2038,7 @@ cp_demangled_name_to_comp (const char *demangled_name,
 
   state.demangle_info = allocate_info ();
 
-  std::unique_ptr<demangle_parse_info> result (new demangle_parse_info);
+  auto result = gdb::make_unique<demangle_parse_info> ();
   result->info = state.demangle_info;
 
   if (yyparse (&state))
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 0300727434d..2af0218dba0 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -674,7 +674,7 @@ mangled_name_to_comp (const char *mangled_name, int options,
 					      options, memory);
       if (ret)
 	{
-	  std::unique_ptr<demangle_parse_info> info (new demangle_parse_info);
+	  auto info = gdb::make_unique<demangle_parse_info> ();
 	  info->tree = ret;
 	  *demangled_p = NULL;
 	  return info;
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 940a01e9612..abc8d613482 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -2126,7 +2126,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
   struct gdbarch *gdbarch = objfile->arch ();
 
   /* Build a minimal decoding of the DWARF2 compilation unit.  */
-  std::unique_ptr<comp_unit> unit (new comp_unit (objfile));
+  auto unit = gdb::make_unique<comp_unit> (objfile);
 
   if (objfile->separate_debug_objfile_backlink == NULL)
     {
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 3d96bf476f8..2e5067efb3d 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -462,7 +462,7 @@ create_cus_from_debug_names (dwarf2_per_bfd *per_bfd,
 bool
 dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
 {
-  std::unique_ptr<mapped_debug_names> map (new mapped_debug_names);
+  auto map = gdb::make_unique<mapped_debug_names> ();
   mapped_debug_names dwz_map;
   struct objfile *objfile = per_objfile->objfile;
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 1127643e53a..9bfc5302b0e 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -778,7 +778,7 @@ dwarf2_read_gdb_index
   if (main_index_contents.empty ())
     return 0;
 
-  std::unique_ptr<mapped_gdb_index> map (new mapped_gdb_index);
+  auto map = gdb::make_unique<mapped_gdb_index> ();
   if (!read_gdb_index_from_buffer (objfile_name (objfile),
 				   use_deprecated_index_sections,
 				   main_index_contents, map.get (), &cu_list,
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a64f82bd24a..7d27ff14128 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4535,7 +4535,7 @@ allocate_type_unit_groups_table ()
 static std::unique_ptr<type_unit_group>
 create_type_unit_group (struct dwarf2_cu *cu, sect_offset line_offset_struct)
 {
-  std::unique_ptr<type_unit_group> tu_group (new type_unit_group);
+  auto tu_group = gdb::make_unique<type_unit_group> ();
 
   tu_group->hash.dwo_unit = cu->dwo_unit;
   tu_group->hash.line_sect_off = line_offset_struct;
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index d7db7beb554..939fcc23b82 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5853,7 +5853,7 @@ static const struct registry<objfile>::key<fixed_point_type_storage>
 void
 allocate_fixed_point_type_info (struct type *type)
 {
-  std::unique_ptr<fixed_point_type_info> up (new fixed_point_type_info);
+  auto up = gdb::make_unique<fixed_point_type_info> ();
   fixed_point_type_info *info;
 
   if (type->is_objfile_owned ())
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 08e790b7e7e..98603cec009 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -170,7 +170,5 @@ py_varobj_get_iterator (struct varobj *var, PyObject *printer,
       error (_("Could not get children iterator"));
     }
 
-  return std::unique_ptr<varobj_iter> (new py_varobj_iter (var,
-							   std::move (iter),
-							   opts));
+  return gdb::make_unique<py_varobj_iter> (var, std::move (iter), opts);
 }
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 9380630c320..b4b8e486cb8 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -236,9 +236,9 @@ void ui_out_table::append_header (int width, ui_align alignment,
     internal_error (_("table header must be specified after table_begin and "
 		      "before table_body."));
 
-  std::unique_ptr<ui_out_hdr> header (new ui_out_hdr (m_headers.size () + 1,
-							width, alignment,
-							col_name, col_hdr));
+  auto header = gdb::make_unique<ui_out_hdr> (m_headers.size () + 1,
+					      width, alignment,
+					      col_name, col_hdr);
 
   m_headers.push_back (std::move (header));
 }
@@ -328,7 +328,7 @@ ui_out::current_level () const
 void
 ui_out::push_level (ui_out_type type)
 {
-  std::unique_ptr<ui_out_level> level (new ui_out_level (type));
+  auto level = gdb::make_unique<ui_out_level> (type);
 
   m_levels.push_back (std::move (level));
 }
diff --git a/gdb/unittests/parallel-for-selftests.c b/gdb/unittests/parallel-for-selftests.c
index 15a095ae62b..1ad7eaa701c 100644
--- a/gdb/unittests/parallel-for-selftests.c
+++ b/gdb/unittests/parallel-for-selftests.c
@@ -160,7 +160,7 @@ TEST (int n_threads)
 	      {
 		if (start == end)
 		  any_empty_tasks = true;
-		return std::unique_ptr<int> (new int (end - start));
+		return gdb::make_unique<int> (end - start);
 	      });
   SELF_CHECK (!any_empty_tasks);
   SELF_CHECK (std::all_of (intresults.begin (),
@@ -178,7 +178,7 @@ TEST (int n_threads)
 	      {
 		if (start == end)
 		  any_empty_tasks = true;
-		return std::unique_ptr<int> (new int (end - start));
+		return gdb::make_unique<int> (end - start);
 	      },
 	    task_size_one);
   SELF_CHECK (!any_empty_tasks);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 81b8e61f304..e7323bed127 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -261,7 +261,7 @@ varobj_create (const char *objname,
 	       const char *expression, CORE_ADDR frame, enum varobj_type type)
 {
   /* Fill out a varobj structure for the (root) variable being constructed.  */
-  std::unique_ptr<varobj> var (new varobj (new varobj_root));
+  auto var = gdb::make_unique<varobj> (new varobj_root);
 
   if (expression != NULL)
     {
diff --git a/gdbsupport/gdb_unique_ptr.h b/gdbsupport/gdb_unique_ptr.h
index a3ab62405d4..e25e5e37efe 100644
--- a/gdbsupport/gdb_unique_ptr.h
+++ b/gdbsupport/gdb_unique_ptr.h
@@ -56,6 +56,15 @@ struct noop_deleter
   void operator() (T *ptr) const { }
 };
 
+/* Create simple std::unique_ptr<T> objects.  */
+
+template<typename T, typename... Arg>
+std::unique_ptr<T>
+make_unique (Arg... args)
+{
+  return std::unique_ptr<T> (new T (std::forward<Arg> (args)...));
+}
+
 } /* namespace gdb */
 
 /* Dup STR and return a unique_xmalloc_ptr for the result.  */
-- 
2.25.4


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

* [PATCH 2/3] gdb: have mi_out_new return std::unique_ptr
  2023-08-14 13:42 [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Andrew Burgess
  2023-08-14 13:42 ` [PATCH 1/3] gdb: add gdb::make_unique function Andrew Burgess
@ 2023-08-14 13:42 ` Andrew Burgess
  2023-08-14 13:42 ` [PATCH 3/3] gdb: remove mi_parse::make functions Andrew Burgess
  2023-08-22 15:33 ` [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Tom Tromey
  3 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2023-08-14 13:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess

Have the mi_out_new function return a std::unique_ptr instead of a raw
pointer.  Update the two uses of mi_out_new.

There should be no user visible changes after this commit.
---
 gdb/mi/mi-interp.c | 2 +-
 gdb/mi/mi-main.c   | 2 +-
 gdb/mi/mi-out.c    | 8 ++++----
 gdb/mi/mi-out.h    | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 0e51c884a65..a7fcf17e51c 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -95,7 +95,7 @@ mi_interp::init (bool top_level)
   mi->log = mi->err;
   mi->targ = new mi_console_file (mi->raw_stdout, "@", '"');
   mi->event_channel = new mi_console_file (mi->raw_stdout, "=", 0);
-  mi->mi_uiout = mi_out_new (name ());
+  mi->mi_uiout = mi_out_new (name ()).release ();
   gdb_assert (mi->mi_uiout != nullptr);
   mi->cli_uiout = new cli_ui_out (mi->out);
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0ac2c74153d..f9bc1cb9cfb 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2207,7 +2207,7 @@ mi_load_progress (const char *section_name,
      which means uiout may not be correct.  Fix it for the duration
      of this function.  */
 
-  std::unique_ptr<ui_out> uiout (mi_out_new (current_interpreter ()->name ()));
+  auto uiout = mi_out_new (current_interpreter ()->name ());
   if (uiout == nullptr)
     return;
 
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 29a416a426d..bbd21287b28 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -336,17 +336,17 @@ mi_ui_out::~mi_ui_out ()
 
 /* See mi/mi-out.h.  */
 
-mi_ui_out *
+std::unique_ptr<mi_ui_out>
 mi_out_new (const char *mi_version)
 {
   if (streq (mi_version, INTERP_MI4) ||  streq (mi_version, INTERP_MI))
-    return new mi_ui_out (4);
+    return gdb::make_unique<mi_ui_out> (4);
 
   if (streq (mi_version, INTERP_MI3))
-    return new mi_ui_out (3);
+    return gdb::make_unique<mi_ui_out> (3);
 
   if (streq (mi_version, INTERP_MI2))
-    return new mi_ui_out (2);
+    return gdb::make_unique<mi_ui_out> (2);
 
   return nullptr;
 }
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index 10c9f8a4585..0dd7479a52f 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -143,7 +143,7 @@ class mi_ui_out : public ui_out
    to one of the INTERP_MI* constants (see interps.h).
 
    Return nullptr if an invalid version is provided.  */
-mi_ui_out *mi_out_new (const char *mi_version);
+std::unique_ptr<mi_ui_out> mi_out_new (const char *mi_version);
 
 void mi_out_put (ui_out *uiout, struct ui_file *stream);
 void mi_out_rewind (ui_out *uiout);
-- 
2.25.4


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

* [PATCH 3/3] gdb: remove mi_parse::make functions
  2023-08-14 13:42 [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Andrew Burgess
  2023-08-14 13:42 ` [PATCH 1/3] gdb: add gdb::make_unique function Andrew Burgess
  2023-08-14 13:42 ` [PATCH 2/3] gdb: have mi_out_new return std::unique_ptr Andrew Burgess
@ 2023-08-14 13:42 ` Andrew Burgess
  2023-08-22 15:33 ` [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Tom Tromey
  3 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2023-08-14 13:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess

Remove the static mi_parse::make functions, and instead use the
mi_parse constructor.

This is a partial revert of the commit:

  commit fde3f93adb50c9937cd2e1c93561aea2fd167156
  Date:   Mon Mar 20 10:56:55 2023 -0600

      Introduce "static constructor" for mi_parse

which introduced the mi_parse::make functions, though after discussion
on the list the reasons for seem to have been lost[1].  Given there
are no test regressions when moving back to using the constructors, I
propose we should do that for now.

There should be no user visible changes after this commit.

[1] https://inbox.sourceware.org/gdb-patches/20230404-dap-loaded-sources-v2-5-93f229095e03@adacore.com/
---
 gdb/mi/mi-main.c   |  2 +-
 gdb/mi/mi-parse.c  | 76 +++++++++++++++++++++-------------------------
 gdb/mi/mi-parse.h  | 18 +++--------
 gdb/python/py-mi.c |  4 +--
 4 files changed, 43 insertions(+), 57 deletions(-)

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f9bc1cb9cfb..573ff9270da 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1935,7 +1935,7 @@ mi_execute_command (const char *cmd, int from_tty)
     = gdb::checked_static_cast<mi_interp *> (command_interp ());
   try
     {
-      command = mi_parse::make (cmd, &token);
+      command = gdb::make_unique<mi_parse> (cmd, &token);
     }
   catch (const gdb_exception &exception)
     {
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index a9b9cdaf88f..433edbf4140 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -287,13 +287,12 @@ mi_parse::set_language (const char *arg, const char **endp)
     *endp = arg;
 }
 
-std::unique_ptr<struct mi_parse>
-mi_parse::make (const char *cmd, std::string *token)
+/* See mi-parse.h.  */
+
+mi_parse::mi_parse (const char *cmd, std::string *token)
 {
   const char *chp;
 
-  std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
-
   /* Before starting, skip leading white space.  */
   cmd = skip_spaces (cmd);
 
@@ -306,10 +305,10 @@ mi_parse::make (const char *cmd, std::string *token)
   if (*chp != '-')
     {
       chp = skip_spaces (chp);
-      parse->command = make_unique_xstrdup (chp);
-      parse->op = CLI_COMMAND;
+      this->command = make_unique_xstrdup (chp);
+      this->op = CLI_COMMAND;
 
-      return parse;
+      return;
     }
 
   /* Extract the command.  */
@@ -318,14 +317,14 @@ mi_parse::make (const char *cmd, std::string *token)
 
     for (; *chp && !isspace (*chp); chp++)
       ;
-    parse->command = make_unique_xstrndup (tmp, chp - tmp);
+    this->command = make_unique_xstrndup (tmp, chp - tmp);
   }
 
   /* Find the command in the MI table.  */
-  parse->cmd = mi_cmd_lookup (parse->command.get ());
-  if (parse->cmd == NULL)
+  this->cmd = mi_cmd_lookup (this->command.get ());
+  if (this->cmd == NULL)
     throw_error (UNDEFINED_COMMAND_ERROR,
-		 _("Undefined MI command: %s"), parse->command.get ());
+		 _("Undefined MI command: %s"), this->command.get ());
 
   /* Skip white space following the command.  */
   chp = skip_spaces (chp);
@@ -349,13 +348,13 @@ mi_parse::make (const char *cmd, std::string *token)
 
       if (strncmp (chp, "--all ", as) == 0)
 	{
-	  parse->all = 1;
+	  this->all = 1;
 	  chp += as;
 	}
       /* See if --all is the last token in the input.  */
       if (strcmp (chp, "--all") == 0)
 	{
-	  parse->all = 1;
+	  this->all = 1;
 	  chp += strlen (chp);
 	}
       if (strncmp (chp, "--thread-group ", tgs) == 0)
@@ -364,7 +363,7 @@ mi_parse::make (const char *cmd, std::string *token)
 
 	  option = "--thread-group";
 	  chp += tgs;
-	  parse->set_thread_group (chp, &endp);
+	  this->set_thread_group (chp, &endp);
 	  chp = endp;
 	}
       else if (strncmp (chp, "--thread ", ts) == 0)
@@ -373,7 +372,7 @@ mi_parse::make (const char *cmd, std::string *token)
 
 	  option = "--thread";
 	  chp += ts;
-	  parse->set_thread (chp, &endp);
+	  this->set_thread (chp, &endp);
 	  chp = endp;
 	}
       else if (strncmp (chp, "--frame ", fs) == 0)
@@ -382,14 +381,14 @@ mi_parse::make (const char *cmd, std::string *token)
 
 	  option = "--frame";
 	  chp += fs;
-	  parse->set_frame (chp, &endp);
+	  this->set_frame (chp, &endp);
 	  chp = endp;
 	}
       else if (strncmp (chp, "--language ", ls) == 0)
 	{
 	  option = "--language";
 	  chp += ls;
-	  parse->set_language (chp, &chp);
+	  this->set_language (chp, &chp);
 	}
       else
 	break;
@@ -400,37 +399,33 @@ mi_parse::make (const char *cmd, std::string *token)
     }
 
   /* Save the rest of the arguments for the command.  */
-  parse->m_args = chp;
+  this->m_args = chp;
 
   /* Fully parsed, flag as an MI command.  */
-  parse->op = MI_COMMAND;
-  return parse;
+  this->op = MI_COMMAND;
 }
 
 /* See mi-parse.h.  */
 
-std::unique_ptr<struct mi_parse>
-mi_parse::make (gdb::unique_xmalloc_ptr<char> command,
-		std::vector<gdb::unique_xmalloc_ptr<char>> args)
+mi_parse::mi_parse (gdb::unique_xmalloc_ptr<char> command,
+		    std::vector<gdb::unique_xmalloc_ptr<char>> args)
 {
-  std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
-
-  parse->command = std::move (command);
-  parse->token = "";
+  this->command = std::move (command);
+  this->token = "";
 
-  if (parse->command.get ()[0] != '-')
+  if (this->command.get ()[0] != '-')
     throw_error (UNDEFINED_COMMAND_ERROR,
 		 _("MI command '%s' does not start with '-'"),
-		 parse->command.get ());
+		 this->command.get ());
 
   /* Find the command in the MI table.  */
-  parse->cmd = mi_cmd_lookup (parse->command.get () + 1);
-  if (parse->cmd == NULL)
+  this->cmd = mi_cmd_lookup (this->command.get () + 1);
+  if (this->cmd == NULL)
     throw_error (UNDEFINED_COMMAND_ERROR,
-		 _("Undefined MI command: %s"), parse->command.get ());
+		 _("Undefined MI command: %s"), this->command.get ());
 
   /* This over-allocates slightly, but it seems unimportant.  */
-  parse->argv = XCNEWVEC (char *, args.size () + 1);
+  this->argv = XCNEWVEC (char *, args.size () + 1);
 
   for (size_t i = 0; i < args.size (); ++i)
     {
@@ -439,43 +434,42 @@ mi_parse::make (gdb::unique_xmalloc_ptr<char> command,
       /* See if --all is the last token in the input.  */
       if (strcmp (chp, "--all") == 0)
 	{
-	  parse->all = 1;
+	  this->all = 1;
 	}
       else if (strcmp (chp, "--thread-group") == 0)
 	{
 	  ++i;
 	  if (i == args.size ())
 	    error ("No argument to '--thread-group'");
-	  parse->set_thread_group (args[i].get (), nullptr);
+	  this->set_thread_group (args[i].get (), nullptr);
 	}
       else if (strcmp (chp, "--thread") == 0)
 	{
 	  ++i;
 	  if (i == args.size ())
 	    error ("No argument to '--thread'");
-	  parse->set_thread (args[i].get (), nullptr);
+	  this->set_thread (args[i].get (), nullptr);
 	}
       else if (strcmp (chp, "--frame") == 0)
 	{
 	  ++i;
 	  if (i == args.size ())
 	    error ("No argument to '--frame'");
-	  parse->set_frame (args[i].get (), nullptr);
+	  this->set_frame (args[i].get (), nullptr);
 	}
       else if (strcmp (chp, "--language") == 0)
 	{
 	  ++i;
 	  if (i == args.size ())
 	    error ("No argument to '--language'");
-	  parse->set_language (args[i].get (), nullptr);
+	  this->set_language (args[i].get (), nullptr);
 	}
       else
-	parse->argv[parse->argc++] = args[i].release ();
+	this->argv[this->argc++] = args[i].release ();
     }
 
   /* Fully parsed, flag as an MI command.  */
-  parse->op = MI_COMMAND;
-  return parse;
+  this->op = MI_COMMAND;
 }
 
 enum print_values
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index c729e94c1f0..6bf516cfc57 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -41,24 +41,18 @@ enum mi_command_type
 
 struct mi_parse
   {
-    /* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
+    /* Attempt to parse CMD creating a ``struct mi_parse''.  If CMD is
        invalid, an exception is thrown.  For an MI_COMMAND COMMAND, ARGS
        and OP are initialized.  Un-initialized fields are zero.  *TOKEN is
-       set to the token, even if an exception is thrown.  It can be
-       assigned to the TOKEN field of the resultant mi_parse object,
-       to be freed by mi_parse_free.  */
-
-    static std::unique_ptr<struct mi_parse> make (const char *cmd,
-						  std::string *token);
+       set to the token, even if an exception is thrown.  */
+    mi_parse (const char *cmd, std::string *token);
 
     /* Create an mi_parse object given the command name and a vector
        of arguments.  Unlike with the other constructor, here the
        arguments are treated "as is" -- no escape processing is
        done.  */
-
-    static std::unique_ptr<struct mi_parse> make
-	 (gdb::unique_xmalloc_ptr<char> command,
-	  std::vector<gdb::unique_xmalloc_ptr<char>> args);
+    mi_parse (gdb::unique_xmalloc_ptr<char> command,
+	      std::vector<gdb::unique_xmalloc_ptr<char>> args);
 
     ~mi_parse ();
 
@@ -91,8 +85,6 @@ struct mi_parse
 
   private:
 
-    mi_parse () = default;
-
     /* Helper methods for parsing arguments.  Each takes the argument
        to be parsed.  It will either set a member of this object, or
        throw an exception on error.  In each case, *ENDP, if non-NULL,
diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
index 0fcd57844e7..66dc6fb8a32 100644
--- a/gdb/python/py-mi.c
+++ b/gdb/python/py-mi.c
@@ -284,8 +284,8 @@ gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
   try
     {
       scoped_restore save_uiout = make_scoped_restore (&current_uiout, &uiout);
-      std::unique_ptr<struct mi_parse> parser
-	= mi_parse::make (std::move (mi_command), std::move (arg_strings));
+      auto parser = gdb::make_unique<mi_parse> (std::move (mi_command),
+						std::move (arg_strings));
       mi_execute_command (parser.get ());
     }
   catch (const gdb_exception &except)
-- 
2.25.4


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

* Re: [PATCH 1/3] gdb: add gdb::make_unique function
  2023-08-14 13:42 ` [PATCH 1/3] gdb: add gdb::make_unique function Andrew Burgess
@ 2023-08-14 15:38   ` Tom Tromey
  2023-08-17 10:26     ` Andrew Burgess
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2023-08-14 15:38 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Tom Tromey

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> While GDB is still C++11, lets add a gdb::make_unique template
Andrew> function that can be used to create std::unique_ptr objects, just like
Andrew> the C++14 std::make_unique.

Andrew> When we move to C++14 we can either alias gdb::make_unique to
Andrew> std::make_unique, or just replace the 'gdb::' prefix throughout.

This looks totally fine to me, but I wonder if this is a situation where
we'd want to use the standard make_unique when it is available, like we
do for some other things.

Tom

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

* Re: [PATCH 1/3] gdb: add gdb::make_unique function
  2023-08-14 15:38   ` Tom Tromey
@ 2023-08-17 10:26     ` Andrew Burgess
  2023-08-17 18:07       ` Tom Tromey
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2023-08-17 10:26 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Tom Tromey

Tom Tromey <tromey@adacore.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> While GDB is still C++11, lets add a gdb::make_unique template
> Andrew> function that can be used to create std::unique_ptr objects, just like
> Andrew> the C++14 std::make_unique.
>
> Andrew> When we move to C++14 we can either alias gdb::make_unique to
> Andrew> std::make_unique, or just replace the 'gdb::' prefix throughout.
>
> This looks totally fine to me, but I wonder if this is a situation where
> we'd want to use the standard make_unique when it is available, like we
> do for some other things.

Hi Tom,

I couldn't figure out a way to truly 'alias' gdb::make_unique to
std::make_unique.  For non template functions it is possible to do
something like:

  const auto alias_func = original_func;

But I couldn't get anything working for template functions, so I just
made gdb::make_unique wrap around a call to std::make_unique (when using
C++14), with -O1 this optimises out, so hopefully this should be enough.

I figure that at some point GDB will move off C++11, at which time it's
easy enough to globally: s/gdb::make_unique/std::make_unique/.

Let me know what you think of this.

Thanks,
Andrew

---

commit 7a7a2f37fa6152afd5f7d0b3fb668e081a7f3ed0
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Aug 10 17:57:46 2023 +0100

    gdb: add gdb::make_unique function
    
    While GDB is still C++11, lets add a gdb::make_unique template
    function that can be used to create std::unique_ptr objects, just like
    the C++14 std::make_unique.
    
    If GDB is being compiled with a C++14 compiler then the new
    gdb::make_unique function will delegate to the std::make_unique.  I
    checked with gcc, and at -O1 and above gdb::make_unique will be
    optimised away completely in this case.
    
    If C++14 (or later) becomes our minimum, then it will be easy enough
    to go through the code and replace gdb::make_unique with
    std::make_unique later on.
    
    I've make use of this function in all the places I think this can
    easily be used, though I'm sure I've probably missed some.
    
    Should be no user visible changes after this commit.

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 33dc7762d91..d16775d49d4 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -428,7 +428,7 @@ test_addrmap ()
 
   /* Create mutable addrmap.  */
   auto_obstack temp_obstack;
-  std::unique_ptr<struct addrmap_mutable> map (new addrmap_mutable);
+  auto map = gdb::make_unique<struct addrmap_mutable> ();
   SELF_CHECK (map != nullptr);
 
   /* Check initial state.  */
diff --git a/gdb/break-catch-load.c b/gdb/break-catch-load.c
index 440b42852bb..94d8b420d32 100644
--- a/gdb/break-catch-load.c
+++ b/gdb/break-catch-load.c
@@ -230,9 +230,8 @@ add_solib_catchpoint (const char *arg, bool is_load, bool is_temp, bool enabled)
   if (*arg == '\0')
     arg = nullptr;
 
-  std::unique_ptr<solib_catchpoint> c (new solib_catchpoint (gdbarch, is_temp,
-							     nullptr,
-							     is_load, arg));
+  auto c = gdb::make_unique<solib_catchpoint> (gdbarch, is_temp, nullptr,
+					       is_load, arg);
 
   c->enable_state = enabled ? bp_enabled : bp_disabled;
 
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index f9b32205b7a..53b7285b366 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -118,7 +118,7 @@ get_compile_context (const char *fe_libcc, const char *fe_context,
     error (_("The loaded version of GCC does not support the required version "
 	     "of the API."));
 
-  return std::unique_ptr<compile_instance> (new INSTTYPE (context));
+  return gdb::make_unique<INSTTYPE> (context);
 }
 
 /* A C-language implementation of get_compile_context.  */
diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 80188074202..324166a03ff 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -2038,7 +2038,7 @@ cp_demangled_name_to_comp (const char *demangled_name,
 
   state.demangle_info = allocate_info ();
 
-  std::unique_ptr<demangle_parse_info> result (new demangle_parse_info);
+  auto result = gdb::make_unique<demangle_parse_info> ();
   result->info = state.demangle_info;
 
   if (yyparse (&state))
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 0300727434d..2af0218dba0 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -674,7 +674,7 @@ mangled_name_to_comp (const char *mangled_name, int options,
 					      options, memory);
       if (ret)
 	{
-	  std::unique_ptr<demangle_parse_info> info (new demangle_parse_info);
+	  auto info = gdb::make_unique<demangle_parse_info> ();
 	  info->tree = ret;
 	  *demangled_p = NULL;
 	  return info;
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 940a01e9612..abc8d613482 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -2126,7 +2126,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
   struct gdbarch *gdbarch = objfile->arch ();
 
   /* Build a minimal decoding of the DWARF2 compilation unit.  */
-  std::unique_ptr<comp_unit> unit (new comp_unit (objfile));
+  auto unit = gdb::make_unique<comp_unit> (objfile);
 
   if (objfile->separate_debug_objfile_backlink == NULL)
     {
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 3d96bf476f8..2e5067efb3d 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -462,7 +462,7 @@ create_cus_from_debug_names (dwarf2_per_bfd *per_bfd,
 bool
 dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
 {
-  std::unique_ptr<mapped_debug_names> map (new mapped_debug_names);
+  auto map = gdb::make_unique<mapped_debug_names> ();
   mapped_debug_names dwz_map;
   struct objfile *objfile = per_objfile->objfile;
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 1127643e53a..9bfc5302b0e 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -778,7 +778,7 @@ dwarf2_read_gdb_index
   if (main_index_contents.empty ())
     return 0;
 
-  std::unique_ptr<mapped_gdb_index> map (new mapped_gdb_index);
+  auto map = gdb::make_unique<mapped_gdb_index> ();
   if (!read_gdb_index_from_buffer (objfile_name (objfile),
 				   use_deprecated_index_sections,
 				   main_index_contents, map.get (), &cu_list,
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a64f82bd24a..7d27ff14128 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4535,7 +4535,7 @@ allocate_type_unit_groups_table ()
 static std::unique_ptr<type_unit_group>
 create_type_unit_group (struct dwarf2_cu *cu, sect_offset line_offset_struct)
 {
-  std::unique_ptr<type_unit_group> tu_group (new type_unit_group);
+  auto tu_group = gdb::make_unique<type_unit_group> ();
 
   tu_group->hash.dwo_unit = cu->dwo_unit;
   tu_group->hash.line_sect_off = line_offset_struct;
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index d7db7beb554..939fcc23b82 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5853,7 +5853,7 @@ static const struct registry<objfile>::key<fixed_point_type_storage>
 void
 allocate_fixed_point_type_info (struct type *type)
 {
-  std::unique_ptr<fixed_point_type_info> up (new fixed_point_type_info);
+  auto up = gdb::make_unique<fixed_point_type_info> ();
   fixed_point_type_info *info;
 
   if (type->is_objfile_owned ())
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 08e790b7e7e..98603cec009 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -170,7 +170,5 @@ py_varobj_get_iterator (struct varobj *var, PyObject *printer,
       error (_("Could not get children iterator"));
     }
 
-  return std::unique_ptr<varobj_iter> (new py_varobj_iter (var,
-							   std::move (iter),
-							   opts));
+  return gdb::make_unique<py_varobj_iter> (var, std::move (iter), opts);
 }
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 9380630c320..b4b8e486cb8 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -236,9 +236,9 @@ void ui_out_table::append_header (int width, ui_align alignment,
     internal_error (_("table header must be specified after table_begin and "
 		      "before table_body."));
 
-  std::unique_ptr<ui_out_hdr> header (new ui_out_hdr (m_headers.size () + 1,
-							width, alignment,
-							col_name, col_hdr));
+  auto header = gdb::make_unique<ui_out_hdr> (m_headers.size () + 1,
+					      width, alignment,
+					      col_name, col_hdr);
 
   m_headers.push_back (std::move (header));
 }
@@ -328,7 +328,7 @@ ui_out::current_level () const
 void
 ui_out::push_level (ui_out_type type)
 {
-  std::unique_ptr<ui_out_level> level (new ui_out_level (type));
+  auto level = gdb::make_unique<ui_out_level> (type);
 
   m_levels.push_back (std::move (level));
 }
diff --git a/gdb/unittests/parallel-for-selftests.c b/gdb/unittests/parallel-for-selftests.c
index 15a095ae62b..1ad7eaa701c 100644
--- a/gdb/unittests/parallel-for-selftests.c
+++ b/gdb/unittests/parallel-for-selftests.c
@@ -160,7 +160,7 @@ TEST (int n_threads)
 	      {
 		if (start == end)
 		  any_empty_tasks = true;
-		return std::unique_ptr<int> (new int (end - start));
+		return gdb::make_unique<int> (end - start);
 	      });
   SELF_CHECK (!any_empty_tasks);
   SELF_CHECK (std::all_of (intresults.begin (),
@@ -178,7 +178,7 @@ TEST (int n_threads)
 	      {
 		if (start == end)
 		  any_empty_tasks = true;
-		return std::unique_ptr<int> (new int (end - start));
+		return gdb::make_unique<int> (end - start);
 	      },
 	    task_size_one);
   SELF_CHECK (!any_empty_tasks);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 81b8e61f304..e7323bed127 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -261,7 +261,7 @@ varobj_create (const char *objname,
 	       const char *expression, CORE_ADDR frame, enum varobj_type type)
 {
   /* Fill out a varobj structure for the (root) variable being constructed.  */
-  std::unique_ptr<varobj> var (new varobj (new varobj_root));
+  auto var = gdb::make_unique<varobj> (new varobj_root);
 
   if (expression != NULL)
     {
diff --git a/gdbsupport/gdb_unique_ptr.h b/gdbsupport/gdb_unique_ptr.h
index a3ab62405d4..9a0ff0b48e0 100644
--- a/gdbsupport/gdb_unique_ptr.h
+++ b/gdbsupport/gdb_unique_ptr.h
@@ -56,6 +56,19 @@ struct noop_deleter
   void operator() (T *ptr) const { }
 };
 
+/* Create simple std::unique_ptr<T> objects.  */
+
+template<typename T, typename... Arg>
+std::unique_ptr<T>
+make_unique (Arg... args)
+{
+#if __cplusplus >= 201402L
+  return std::make_unique<T> (std::forward<Arg> (args)...);
+#else
+  return std::unique_ptr<T> (new T (std::forward<Arg> (args)...));
+#endif /* __cplusplus < 201402L */
+}
+
 } /* namespace gdb */
 
 /* Dup STR and return a unique_xmalloc_ptr for the result.  */


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

* Re: [PATCH 1/3] gdb: add gdb::make_unique function
  2023-08-17 10:26     ` Andrew Burgess
@ 2023-08-17 18:07       ` Tom Tromey
  2023-08-21 10:02         ` Andrew Burgess
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2023-08-17 18:07 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> I couldn't figure out a way to truly 'alias' gdb::make_unique to
Andrew> std::make_unique.

Yeah, sorry about that.  I confused this with type aliases.

Andrew> I figure that at some point GDB will move off C++11, at which time it's
Andrew> easy enough to globally: s/gdb::make_unique/std::make_unique/.

Yep.

Andrew> +template<typename T, typename... Arg>
Andrew> +std::unique_ptr<T>
Andrew> +make_unique (Arg... args)

I think it's more idiomatic to use 'Arg &&...args' here.

Tom

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

* Re: [PATCH 1/3] gdb: add gdb::make_unique function
  2023-08-17 18:07       ` Tom Tromey
@ 2023-08-21 10:02         ` Andrew Burgess
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2023-08-21 10:02 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Tom Tromey, gdb-patches

Tom Tromey <tromey@adacore.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> I couldn't figure out a way to truly 'alias' gdb::make_unique to
> Andrew> std::make_unique.
>
> Yeah, sorry about that.  I confused this with type aliases.
>
> Andrew> I figure that at some point GDB will move off C++11, at which time it's
> Andrew> easy enough to globally: s/gdb::make_unique/std::make_unique/.
>
> Yep.
>
> Andrew> +template<typename T, typename... Arg>
> Andrew> +std::unique_ptr<T>
> Andrew> +make_unique (Arg... args)
>
> I think it's more idiomatic to use 'Arg &&...args' here.

Done in the version below.

Thanks,
Andrew

---

commit be5be0a3e802192ae9353f8d804bc37034fa0a6c
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Aug 10 17:57:46 2023 +0100

    gdb: add gdb::make_unique function
    
    While GDB is still C++11, lets add a gdb::make_unique template
    function that can be used to create std::unique_ptr objects, just like
    the C++14 std::make_unique.
    
    If GDB is being compiled with a C++14 compiler then the new
    gdb::make_unique function will delegate to the std::make_unique.  I
    checked with gcc, and at -O1 and above gdb::make_unique will be
    optimised away completely in this case.
    
    If C++14 (or later) becomes our minimum, then it will be easy enough
    to go through the code and replace gdb::make_unique with
    std::make_unique later on.
    
    I've make use of this function in all the places I think this can
    easily be used, though I'm sure I've probably missed some.
    
    Should be no user visible changes after this commit.

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 33dc7762d91..d16775d49d4 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -428,7 +428,7 @@ test_addrmap ()
 
   /* Create mutable addrmap.  */
   auto_obstack temp_obstack;
-  std::unique_ptr<struct addrmap_mutable> map (new addrmap_mutable);
+  auto map = gdb::make_unique<struct addrmap_mutable> ();
   SELF_CHECK (map != nullptr);
 
   /* Check initial state.  */
diff --git a/gdb/break-catch-load.c b/gdb/break-catch-load.c
index 440b42852bb..94d8b420d32 100644
--- a/gdb/break-catch-load.c
+++ b/gdb/break-catch-load.c
@@ -230,9 +230,8 @@ add_solib_catchpoint (const char *arg, bool is_load, bool is_temp, bool enabled)
   if (*arg == '\0')
     arg = nullptr;
 
-  std::unique_ptr<solib_catchpoint> c (new solib_catchpoint (gdbarch, is_temp,
-							     nullptr,
-							     is_load, arg));
+  auto c = gdb::make_unique<solib_catchpoint> (gdbarch, is_temp, nullptr,
+					       is_load, arg);
 
   c->enable_state = enabled ? bp_enabled : bp_disabled;
 
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index f9b32205b7a..53b7285b366 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -118,7 +118,7 @@ get_compile_context (const char *fe_libcc, const char *fe_context,
     error (_("The loaded version of GCC does not support the required version "
 	     "of the API."));
 
-  return std::unique_ptr<compile_instance> (new INSTTYPE (context));
+  return gdb::make_unique<INSTTYPE> (context);
 }
 
 /* A C-language implementation of get_compile_context.  */
diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 80188074202..324166a03ff 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -2038,7 +2038,7 @@ cp_demangled_name_to_comp (const char *demangled_name,
 
   state.demangle_info = allocate_info ();
 
-  std::unique_ptr<demangle_parse_info> result (new demangle_parse_info);
+  auto result = gdb::make_unique<demangle_parse_info> ();
   result->info = state.demangle_info;
 
   if (yyparse (&state))
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 0300727434d..2af0218dba0 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -674,7 +674,7 @@ mangled_name_to_comp (const char *mangled_name, int options,
 					      options, memory);
       if (ret)
 	{
-	  std::unique_ptr<demangle_parse_info> info (new demangle_parse_info);
+	  auto info = gdb::make_unique<demangle_parse_info> ();
 	  info->tree = ret;
 	  *demangled_p = NULL;
 	  return info;
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 940a01e9612..abc8d613482 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -2126,7 +2126,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
   struct gdbarch *gdbarch = objfile->arch ();
 
   /* Build a minimal decoding of the DWARF2 compilation unit.  */
-  std::unique_ptr<comp_unit> unit (new comp_unit (objfile));
+  auto unit = gdb::make_unique<comp_unit> (objfile);
 
   if (objfile->separate_debug_objfile_backlink == NULL)
     {
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 3d96bf476f8..2e5067efb3d 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -462,7 +462,7 @@ create_cus_from_debug_names (dwarf2_per_bfd *per_bfd,
 bool
 dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
 {
-  std::unique_ptr<mapped_debug_names> map (new mapped_debug_names);
+  auto map = gdb::make_unique<mapped_debug_names> ();
   mapped_debug_names dwz_map;
   struct objfile *objfile = per_objfile->objfile;
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 1127643e53a..9bfc5302b0e 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -778,7 +778,7 @@ dwarf2_read_gdb_index
   if (main_index_contents.empty ())
     return 0;
 
-  std::unique_ptr<mapped_gdb_index> map (new mapped_gdb_index);
+  auto map = gdb::make_unique<mapped_gdb_index> ();
   if (!read_gdb_index_from_buffer (objfile_name (objfile),
 				   use_deprecated_index_sections,
 				   main_index_contents, map.get (), &cu_list,
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f1d7bfdfdec..eb4cb9ba72e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4535,7 +4535,7 @@ allocate_type_unit_groups_table ()
 static std::unique_ptr<type_unit_group>
 create_type_unit_group (struct dwarf2_cu *cu, sect_offset line_offset_struct)
 {
-  std::unique_ptr<type_unit_group> tu_group (new type_unit_group);
+  auto tu_group = gdb::make_unique<type_unit_group> ();
 
   tu_group->hash.dwo_unit = cu->dwo_unit;
   tu_group->hash.line_sect_off = line_offset_struct;
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index d7db7beb554..939fcc23b82 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5853,7 +5853,7 @@ static const struct registry<objfile>::key<fixed_point_type_storage>
 void
 allocate_fixed_point_type_info (struct type *type)
 {
-  std::unique_ptr<fixed_point_type_info> up (new fixed_point_type_info);
+  auto up = gdb::make_unique<fixed_point_type_info> ();
   fixed_point_type_info *info;
 
   if (type->is_objfile_owned ())
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 08e790b7e7e..98603cec009 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -170,7 +170,5 @@ py_varobj_get_iterator (struct varobj *var, PyObject *printer,
       error (_("Could not get children iterator"));
     }
 
-  return std::unique_ptr<varobj_iter> (new py_varobj_iter (var,
-							   std::move (iter),
-							   opts));
+  return gdb::make_unique<py_varobj_iter> (var, std::move (iter), opts);
 }
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 9380630c320..b4b8e486cb8 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -236,9 +236,9 @@ void ui_out_table::append_header (int width, ui_align alignment,
     internal_error (_("table header must be specified after table_begin and "
 		      "before table_body."));
 
-  std::unique_ptr<ui_out_hdr> header (new ui_out_hdr (m_headers.size () + 1,
-							width, alignment,
-							col_name, col_hdr));
+  auto header = gdb::make_unique<ui_out_hdr> (m_headers.size () + 1,
+					      width, alignment,
+					      col_name, col_hdr);
 
   m_headers.push_back (std::move (header));
 }
@@ -328,7 +328,7 @@ ui_out::current_level () const
 void
 ui_out::push_level (ui_out_type type)
 {
-  std::unique_ptr<ui_out_level> level (new ui_out_level (type));
+  auto level = gdb::make_unique<ui_out_level> (type);
 
   m_levels.push_back (std::move (level));
 }
diff --git a/gdb/unittests/parallel-for-selftests.c b/gdb/unittests/parallel-for-selftests.c
index 15a095ae62b..1ad7eaa701c 100644
--- a/gdb/unittests/parallel-for-selftests.c
+++ b/gdb/unittests/parallel-for-selftests.c
@@ -160,7 +160,7 @@ TEST (int n_threads)
 	      {
 		if (start == end)
 		  any_empty_tasks = true;
-		return std::unique_ptr<int> (new int (end - start));
+		return gdb::make_unique<int> (end - start);
 	      });
   SELF_CHECK (!any_empty_tasks);
   SELF_CHECK (std::all_of (intresults.begin (),
@@ -178,7 +178,7 @@ TEST (int n_threads)
 	      {
 		if (start == end)
 		  any_empty_tasks = true;
-		return std::unique_ptr<int> (new int (end - start));
+		return gdb::make_unique<int> (end - start);
 	      },
 	    task_size_one);
   SELF_CHECK (!any_empty_tasks);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 81b8e61f304..e7323bed127 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -261,7 +261,7 @@ varobj_create (const char *objname,
 	       const char *expression, CORE_ADDR frame, enum varobj_type type)
 {
   /* Fill out a varobj structure for the (root) variable being constructed.  */
-  std::unique_ptr<varobj> var (new varobj (new varobj_root));
+  auto var = gdb::make_unique<varobj> (new varobj_root);
 
   if (expression != NULL)
     {
diff --git a/gdbsupport/gdb_unique_ptr.h b/gdbsupport/gdb_unique_ptr.h
index a3ab62405d4..8ff7cec1da9 100644
--- a/gdbsupport/gdb_unique_ptr.h
+++ b/gdbsupport/gdb_unique_ptr.h
@@ -56,6 +56,19 @@ struct noop_deleter
   void operator() (T *ptr) const { }
 };
 
+/* Create simple std::unique_ptr<T> objects.  */
+
+template<typename T, typename... Arg>
+std::unique_ptr<T>
+make_unique (Arg &&...args)
+{
+#if __cplusplus >= 201402L
+  return std::make_unique<T> (std::forward<Arg> (args)...);
+#else
+  return std::unique_ptr<T> (new T (std::forward<Arg> (args)...));
+#endif /* __cplusplus < 201402L */
+}
+
 } /* namespace gdb */
 
 /* Dup STR and return a unique_xmalloc_ptr for the result.  */


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

* Re: [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use
  2023-08-14 13:42 [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Andrew Burgess
                   ` (2 preceding siblings ...)
  2023-08-14 13:42 ` [PATCH 3/3] gdb: remove mi_parse::make functions Andrew Burgess
@ 2023-08-22 15:33 ` Tom Tromey
  2023-08-23  8:52   ` Andrew Burgess
  3 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2023-08-22 15:33 UTC (permalink / raw)
  To: Andrew Burgess via Gdb-patches; +Cc: Andrew Burgess, Tom Tromey

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> This started while working on another series, in order to avoid
Andrew> calling 'new' directly, I really wanted a std::make_unique function --
Andrew> but that's C++14, so not available for GDB right now.

Andrew> So I added a gdb::make_unique, which should be equivalent to the C++14
Andrew> function.

Andrew> The last two commits are additional make_unique/unique_ptr changes,
Andrew> but target specific areas that needed slightly more refactoring.

Thanks for doing this.  I re-read the series & with your latest
make_unique patch, it looks good to me.

FWIW I think the ::make approach was just a sort of minimal refactoring
at the time.  Previously the constructor didn't do anything.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

* Re: [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use
  2023-08-22 15:33 ` [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Tom Tromey
@ 2023-08-23  8:52   ` Andrew Burgess
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2023-08-23  8:52 UTC (permalink / raw)
  To: gdb-patches

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> This started while working on another series, in order to avoid
> Andrew> calling 'new' directly, I really wanted a std::make_unique function --
> Andrew> but that's C++14, so not available for GDB right now.
>
> Andrew> So I added a gdb::make_unique, which should be equivalent to the C++14
> Andrew> function.
>
> Andrew> The last two commits are additional make_unique/unique_ptr changes,
> Andrew> but target specific areas that needed slightly more refactoring.
>
> Thanks for doing this.  I re-read the series & with your latest
> make_unique patch, it looks good to me.
>
> FWIW I think the ::make approach was just a sort of minimal refactoring
> at the time.  Previously the constructor didn't do anything.
>
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew


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

end of thread, other threads:[~2023-08-23  8:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-14 13:42 [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Andrew Burgess
2023-08-14 13:42 ` [PATCH 1/3] gdb: add gdb::make_unique function Andrew Burgess
2023-08-14 15:38   ` Tom Tromey
2023-08-17 10:26     ` Andrew Burgess
2023-08-17 18:07       ` Tom Tromey
2023-08-21 10:02         ` Andrew Burgess
2023-08-14 13:42 ` [PATCH 2/3] gdb: have mi_out_new return std::unique_ptr Andrew Burgess
2023-08-14 13:42 ` [PATCH 3/3] gdb: remove mi_parse::make functions Andrew Burgess
2023-08-22 15:33 ` [PATCH 0/3] New gdb::make_unique and more std::unique_ptr use Tom Tromey
2023-08-23  8:52   ` Andrew Burgess

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).