[4/7] Use try_emplace in index-write.c
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
fail
|
Testing failed
|
Commit Message
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(-)
Comments
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);
> }
> }
>
> > - 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
Hi,
I find such use of structured binding to be a big help in readability.
The pair is always clunky to use.
Best,
Lancelot.
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> A little odd to take a const ref to the returned pair instead of taking
Pedro> it by value. Better yet, C++17 gives us structured bindings, which lets us
Pedro> hide the ugly pair completely and use helpful names instead of first/second:
Pedro> auto [it, ins] = symbol_hash_table.try_emplace (entry.cu_indices,
Pedro> cpool.size ());
Pedro> How about we use it here?
I made this change.
Tom
@@ -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 ());
+ 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);
}
}