Re: [PATCH 4/7] Use try_emplace in index-write.c

[Pedro Alves <pedro@palves.net>]

On 2023-10-17 00:02, Tom Tromey wrote:
> index-write.c has a comment indicating that C++17's try_emplace could
> be used.  This patch makes the change.
> ---
>  gdb/dwarf2/index-write.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
> index a6d770c9ee5..e402d407ee7 100644
> --- a/gdb/dwarf2/index-write.c
> +++ b/gdb/dwarf2/index-write.c
> @@ -384,24 +384,16 @@ write_hash_table (mapped_symtab *symtab, data_buf &output, data_buf &cpool)
>  	  continue;
>  	gdb_assert (entry.index_offset == 0);
>  
> -	/* Finding before inserting is faster than always trying to
> -	   insert, because inserting always allocates a node, does the
> -	   lookup, and then destroys the new node if another node
> -	   already had the same key.  C++17 try_emplace will avoid
> -	   this.  */
> -	const auto found
> -	  = symbol_hash_table.find (entry.cu_indices);
> -	if (found != symbol_hash_table.end ())
> +	const auto &pair = symbol_hash_table.try_emplace (entry.cu_indices,
> +							  cpool.size ());

A little odd to take a const ref to the returned pair instead of taking
it by value.  Better yet, C++17 gives us structured bindings, which lets us
hide the ugly pair completely and use helpful names instead of first/second:

  auto [it, ins] = symbol_hash_table.try_emplace (entry.cu_indices, 
                                                  cpool.size ());

How about we use it here?

Pedro Alves

> +	entry.index_offset = pair.first->second;
> +	if (pair.second)
>  	  {
> -	    entry.index_offset = found->second;
> -	    continue;
> +	    /* Newly inserted.  */
> +	    cpool.append_offset (entry.cu_indices.size ());
> +	    for (const auto index : entry.cu_indices)
> +	      cpool.append_offset (index);
>  	  }
> -
> -	symbol_hash_table.emplace (entry.cu_indices, cpool.size ());
> -	entry.index_offset = cpool.size ();
> -	cpool.append_offset (entry.cu_indices.size ());
> -	for (const auto index : entry.cu_indices)
> -	  cpool.append_offset (index);
>        }
>    }
>