Use std::make_unique in more places

  I searched for spots using ".reset (new ...)" and replaced most of
these with std::make_unique.  I think this is a bit cleaner and more
idiomatic.

Regression tested on x86-64 Fedora 40.
---
 gdb/breakpoint.c     |  4 ++--
 gdb/cli/cli-interp.c |  2 +-
 gdb/dwarf2/cu.c      |  4 ++--
 gdb/dwarf2/read.c    | 16 ++++++++--------
 gdb/jit.c            |  7 ++++---
 gdb/osdata.c         |  2 +-
 gdb/parse.c          |  2 +-
 gdb/regcache-dump.c  | 15 ++++++++-------
 gdb/remote.c         |  4 ++--
 gdb/typeprint.c      |  4 ++--
 gdb/ui-out.c         |  2 +-
 11 files changed, 32 insertions(+), 30 deletions(-)
  

Hi,

I looked through this and it seems fine, just two slight suggestions.

diff --git a/gdb/dwarf2/cu.c b/gdb/dwarf2/cu.c
index 4cfef79b61f..76cdd6fb4b9 100644
--- a/gdb/dwarf2/cu.c
+++ b/gdb/dwarf2/cu.c
@@ -77,9 +77,12 @@ dwarf2_cu::start_compunit_symtab (const char *name, const char *comp_dir,
       name_for_id = name_for_id_holder.c_str ();
     }

-  m_builder = std::make_unique<buildsym_compunit>
-                  (this->per_objfile->objfile,
-                   name, comp_dir, name_for_id, lang (), low_pc);
+  m_builder = std::make_unique<buildsym_compunit> (this->per_objfile->objfile,
+                                                  name,
+                                                  comp_dir,
+                                                  name_for_id,
+                                                  lang (),
+                                                  low_pc);

   list_in_scope = get_builder ()->get_file_symbols ();

This would fit on two instead of three lines:

diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
index 78924afb103..b772f4a24b0 100644
--- a/gdb/regcache-dump.c
+++ b/gdb/regcache-dump.c
@@ -258,9 +258,8 @@ regcache_print (const char *args, enum regcache_dump_what what_to_dump)
        auto dump_pseudo = (what_to_dump == regcache_dump_cooked);

        if (target_has_registers ())
-         dump = (std::make_unique<register_dump_regcache>
-                 (get_thread_regcache (inferior_thread ()),
-                  dump_pseudo));
+         dump = std::make_unique<register_dump_regcache>
+                  (get_thread_regcache (inferior_thread ()), dump_pseudo);
        else
          {
            /* For the benefit of "maint print registers" & co when

Reviewed-By: Klaus Gerlicher<klaus.gerlicher@intel.com>

Thanks
Klaus


> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Wednesday, October 9, 2024 5:45 AM
> To: gdb-patches@sourceware.org
> Cc: Tom Tromey <tom@tromey.com>
> Subject: [PATCH] Use std::make_unique in more places
> 
> I searched for spots using ".reset (new ...)" and replaced most of
> these with std::make_unique.  I think this is a bit cleaner and more
> idiomatic.
> 
> Regression tested on x86-64 Fedora 40.
> ---
>  gdb/breakpoint.c     |  4 ++--
>  gdb/cli/cli-interp.c |  2 +-
>  gdb/dwarf2/cu.c      |  4 ++--
>  gdb/dwarf2/read.c    | 16 ++++++++--------
>  gdb/jit.c            |  7 ++++---
>  gdb/osdata.c         |  2 +-
>  gdb/parse.c          |  2 +-
>  gdb/regcache-dump.c  | 15 ++++++++-------
>  gdb/remote.c         |  4 ++--
>  gdb/typeprint.c      |  4 ++--
>  gdb/ui-out.c         |  2 +-
>  11 files changed, 32 insertions(+), 30 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 10e2ef38523..3614e4df106 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -10533,9 +10533,9 @@ watch_command_1 (const char *arg, int
> accessflag, int from_tty,
> 
>    std::unique_ptr<watchpoint> w;
>    if (use_mask)
> -    w.reset (new masked_watchpoint (nullptr, bp_type));
> +    w = std::make_unique<masked_watchpoint> (nullptr, bp_type);
>    else
> -    w.reset (new watchpoint (nullptr, bp_type));
> +    w = std::make_unique<watchpoint> (nullptr, bp_type);
> 
>    /* At most one of thread or task can be set on a watchpoint.  */
>    gdb_assert (thread == -1 || task == -1);
> diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
> index 18175735c4f..dbe2ed1b687 100644
> --- a/gdb/cli/cli-interp.c
> +++ b/gdb/cli/cli-interp.c
> @@ -269,7 +269,7 @@ cli_interp_base::set_logging (ui_file_up logfile, bool
> logging_redirect,
>    if (logfile != nullptr)
>      {
>        gdb_assert (m_saved_output == nullptr);
> -      m_saved_output.reset (new saved_output_files);
> +      m_saved_output = std::make_unique<saved_output_files> ();
>        m_saved_output->out = gdb_stdout;
>        m_saved_output->err = gdb_stderr;
>        m_saved_output->log = gdb_stdlog;
> diff --git a/gdb/dwarf2/cu.c b/gdb/dwarf2/cu.c
> index 5cb22919c32..4cfef79b61f 100644
> --- a/gdb/dwarf2/cu.c
> +++ b/gdb/dwarf2/cu.c
> @@ -77,9 +77,9 @@ dwarf2_cu::start_compunit_symtab (const char *name,
> const char *comp_dir,
>        name_for_id = name_for_id_holder.c_str ();
>      }
> 
> -  m_builder.reset (new struct buildsym_compunit
> +  m_builder = std::make_unique<buildsym_compunit>
>  		   (this->per_objfile->objfile,
> -		    name, comp_dir, name_for_id, lang (), low_pc));
> +		    name, comp_dir, name_for_id, lang (), low_pc);
> 
>    list_in_scope = get_builder ()->get_file_symbols ();
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index ea31d8dd851..5f554f94b12 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -3968,7 +3968,7 @@ cutu_reader::init_tu_and_read_dwo_dies
> (dwarf2_per_cu_data *this_cu,
>        /* If an existing_cu is provided, a dwarf2_cu must not exist for this_cu
>  	 in per_objfile yet.  */
>        gdb_assert (per_objfile->get_cu (this_cu) == nullptr);
> -      m_new_cu.reset (new dwarf2_cu (this_cu, per_objfile));
> +      m_new_cu = std::make_unique<dwarf2_cu> (this_cu, per_objfile);
>        cu = m_new_cu.get ();
>      }
> 
> @@ -4065,7 +4065,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data
> *this_cu,
>  	 thread-safe.  */
>        gdb_assert (cache != nullptr
>  		  || per_objfile->get_cu (this_cu) == nullptr);
> -      m_new_cu.reset (new dwarf2_cu (this_cu, per_objfile));
> +      m_new_cu = std::make_unique<dwarf2_cu> (this_cu, per_objfile);
>        cu = m_new_cu.get ();
>      }
> 
> @@ -4255,7 +4255,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data
> *this_cu,
>    /* This is cheap if the section is already read in.  */
>    section->read (objfile);
> 
> -  m_new_cu.reset (new dwarf2_cu (this_cu, per_objfile));
> +  m_new_cu = std::make_unique<dwarf2_cu> (this_cu, per_objfile);
> 
>    begin_info_ptr = info_ptr = section->buffer + to_underlying (this_cu-
> >sect_off);
>    info_ptr = read_and_check_comp_unit_head (per_objfile, &m_new_cu-
> >header,
> @@ -7364,7 +7364,7 @@ find_file_and_directory (struct die_info *die, struct
> dwarf2_cu *cu)
>        res.set_name (make_unique_xstrdup (lbasename (res.get_name ())));
>      }
> 
> -  cu->per_cu->fnd.reset (new file_and_directory (std::move (res)));
> +  cu->per_cu->fnd = std::make_unique<file_and_directory> (std::move (res));
>    return *cu->per_cu->fnd;
>  }
> 
> @@ -7610,11 +7610,11 @@ dwarf2_cu::setup_type_unit_groups (struct
> die_info *die)
>  	  gdb_assert (tug_unshare->symtabs == NULL);
>  	  gdb_assert (m_builder == nullptr);
>  	  struct compunit_symtab *cust = tug_unshare->compunit_symtab;
> -	  m_builder.reset (new struct buildsym_compunit
> +	  m_builder = std::make_unique<buildsym_compunit>
>  			   (cust->objfile (), "",
>  			    cust->dirname (),
>  			    cust->language (),
> -			    0, cust));
> +			    0, cust);
>  	  list_in_scope = get_builder ()->get_file_symbols ();
>  	}
>        return;
> @@ -7664,11 +7664,11 @@ dwarf2_cu::setup_type_unit_groups (struct
> die_info *die)
>      {
>        gdb_assert (m_builder == nullptr);
>        struct compunit_symtab *cust = tug_unshare->compunit_symtab;
> -      m_builder.reset (new struct buildsym_compunit
> +      m_builder = std::make_unique<buildsym_compunit>
>  		       (cust->objfile (), "",
>  			cust->dirname (),
>  			cust->language (),
> -			0, cust));
> +			0, cust);
>        list_in_scope = get_builder ()->get_file_symbols ();
> 
>        auto &file_names = line_header->file_names ();
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 78b3d984041..ed3b26cd4bd 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -222,7 +222,7 @@ static jiter_objfile_data *
>  get_jiter_objfile_data (objfile *objf)
>  {
>    if (objf->jiter_data == nullptr)
> -    objf->jiter_data.reset (new jiter_objfile_data ());
> +    objf->jiter_data = std::make_unique<jiter_objfile_data> ();
> 
>    return objf->jiter_data.get ();
>  }
> @@ -236,8 +236,9 @@ add_objfile_entry (struct objfile *objfile, CORE_ADDR
> entry,
>  {
>    gdb_assert (objfile->jited_data == nullptr);
> 
> -  objfile->jited_data.reset (new jited_objfile_data (entry, symfile_addr,
> -						     symfile_size));
> +  objfile->jited_data = std::make_unique<jited_objfile_data> (entry,
> +							      symfile_addr,
> +							      symfile_size);
>  }
> 
>  /* Helper function for reading the global JIT descriptor from remote
> diff --git a/gdb/osdata.c b/gdb/osdata.c
> index e4d9b0bef97..e22c249c493 100644
> --- a/gdb/osdata.c
> +++ b/gdb/osdata.c
> @@ -63,7 +63,7 @@ osdata_start_osdata (struct gdb_xml_parser *parser,
>      gdb_xml_error (parser, _("Seen more than on osdata element"));
> 
>    char *type = (char *) xml_find_attribute (attributes, "type")->value.get ();
> -  data->osdata.reset (new struct osdata (std::string (type)));
> +  data->osdata = std::make_unique<osdata> (std::string (type));
>  }
> 
>  /* Handle the start of a <item> element.  */
> diff --git a/gdb/parse.c b/gdb/parse.c
> index e0837de7b01..ffefe6fee5f 100644
> --- a/gdb/parse.c
> +++ b/gdb/parse.c
> @@ -100,7 +100,7 @@ void
>  parser_state::mark_struct_expression (expr::structop_base_operation *op)
>  {
>    gdb_assert (parse_completion && m_completion_state == nullptr);
> -  m_completion_state.reset (new expr_complete_structop (op));
> +  m_completion_state = std::make_unique<expr_complete_structop> (op);
>  }
> 
>  /* Indicate that the current parser invocation is completing a tag.
> diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
> index 6b711bf6c2a..78924afb103 100644
> --- a/gdb/regcache-dump.c
> +++ b/gdb/regcache-dump.c
> @@ -244,13 +244,13 @@ regcache_print (const char *args, enum
> regcache_dump_what what_to_dump)
>    switch (what_to_dump)
>      {
>      case regcache_dump_none:
> -      dump.reset (new register_dump_none (gdbarch));
> +      dump = std::make_unique<register_dump_none> (gdbarch);
>        break;
>      case regcache_dump_remote:
> -      dump.reset (new register_dump_remote (gdbarch));
> +      dump = std::make_unique<register_dump_remote> (gdbarch);
>        break;
>      case regcache_dump_groups:
> -      dump.reset (new register_dump_groups (gdbarch));
> +      dump = std::make_unique<register_dump_groups> (gdbarch);
>        break;
>      case regcache_dump_raw:
>      case regcache_dump_cooked:
> @@ -258,15 +258,16 @@ regcache_print (const char *args, enum
> regcache_dump_what what_to_dump)
>  	auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
> 
>  	if (target_has_registers ())
> -	  dump.reset (new register_dump_regcache (get_thread_regcache
> -						    (inferior_thread ()),
> -						  dump_pseudo));
> +	  dump = (std::make_unique<register_dump_regcache>
> +		  (get_thread_regcache (inferior_thread ()),
> +		   dump_pseudo));
>  	else
>  	  {
>  	    /* For the benefit of "maint print registers" & co when
>  	       debugging an executable, allow dumping a regcache even when
>  	       there is no thread selected / no registers.  */
> -	    dump.reset (new register_dump_reg_buffer (gdbarch,
> dump_pseudo));
> +	    dump = std::make_unique<register_dump_reg_buffer> (gdbarch,
> +							       dump_pseudo);
>  	  }
>        }
>        break;
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 53234915dea..fdd83ecbacc 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -3049,7 +3049,7 @@ get_remote_thread_info (thread_info *thread)
>    gdb_assert (thread != NULL);
> 
>    if (thread->priv == NULL)
> -    thread->priv.reset (new remote_thread_info);
> +    thread->priv = std::make_unique<remote_thread_info> ();
> 
>    return gdb::checked_static_cast<remote_thread_info *> (thread->priv.get
> ());
>  }
> @@ -7099,7 +7099,7 @@ static remote_inferior *
>  get_remote_inferior (inferior *inf)
>  {
>    if (inf->priv == NULL)
> -    inf->priv.reset (new remote_inferior);
> +    inf->priv = std::make_unique<remote_inferior> ();
> 
>    return gdb::checked_static_cast<remote_inferior *> (inf->priv.get ());
>  }
> diff --git a/gdb/typeprint.c b/gdb/typeprint.c
> index 274f6029a7e..456d8dcb00b 100644
> --- a/gdb/typeprint.c
> +++ b/gdb/typeprint.c
> @@ -562,10 +562,10 @@ whatis_exp (const char *exp, int show)
>    std::unique_ptr<ext_lang_type_printers> printer_holder;
>    if (!flags.raw)
>      {
> -      table_holder.reset (new typedef_hash_table);
> +      table_holder = std::make_unique<typedef_hash_table> ();
>        flags.global_typedefs = table_holder.get ();
> 
> -      printer_holder.reset (new ext_lang_type_printers);
> +      printer_holder = std::make_unique<ext_lang_type_printers> ();
>        flags.global_printers = printer_holder.get ();
>      }
> 
> diff --git a/gdb/ui-out.c b/gdb/ui-out.c
> index 3330cc87ebb..41ce6efd14f 100644
> --- a/gdb/ui-out.c
> +++ b/gdb/ui-out.c
> @@ -353,7 +353,7 @@ ui_out::table_begin (int nr_cols, int nr_rows, const
> std::string &tblid)
>      internal_error (_("tables cannot be nested; table_begin found before \
>  previous table_end."));
> 
> -  m_table_up.reset (new ui_out_table (level () + 1, nr_cols, tblid));
> +  m_table_up = std::make_unique<ui_out_table> (level () + 1, nr_cols, tblid);
> 
>    do_table_begin (nr_cols, nr_rows, tblid.c_str ());
>  }
> --
> 2.46.1
> 

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

>>>>> Gerlicher, Klaus <klaus.gerlicher@intel.com> writes:

> This would fit on two instead of three lines:

I'll make the change, but I think we normally review for over-long
lines, not perfect compactness.

> -         dump = (std::make_unique<register_dump_regcache>
> -                 (get_thread_regcache (inferior_thread ()),
> -                  dump_pseudo));
> +         dump = std::make_unique<register_dump_regcache>
> +                  (get_thread_regcache (inferior_thread ()), dump_pseudo);

This wouldn't be GNU style.  The extra parens are required here.

Tom
  

> I'll make the change, but I think we normally review for over-long
> lines, not perfect compactness.

I read your suggestion exactly backward.  I'm sorry about that.

>> -         dump = (std::make_unique<register_dump_regcache>
>> -                 (get_thread_regcache (inferior_thread ()),
>> -                  dump_pseudo));
>> +         dump = std::make_unique<register_dump_regcache>
>> +                  (get_thread_regcache (inferior_thread ()), dump_pseudo);

> This wouldn't be GNU style.  The extra parens are required here.

It fits on 2 lines even with the parens so I made this change as well.

Tom