* [PATCH v3][PR symtab/30520 2/4] gdb/symtab: reuse last segment lookup name info by creating it outside the loop
2024-05-06 15:09 [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU Dmitry Neverov
@ 2024-05-06 15:09 ` Dmitry Neverov
2024-05-07 17:09 ` Tom Tromey
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 3/4] gdb/symtab: compute match_type " Dmitry Neverov
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Dmitry Neverov @ 2024-05-06 15:09 UTC (permalink / raw)
To: gdb-patches; +Cc: dmitry.neverov
---
gdb/dwarf2/read.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e2f010b7849..a49ade9dc31 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16643,6 +16643,9 @@ cooked_index_functions::expand_symtabs_matching
= lookup_name_without_params.split_name (lang);
std::string last_name (name_vec.back ());
+ lookup_name_info last_segment_lookup_name (
+ last_name, symbol_name_match_type::FULL, completing, true);
+
for (const cooked_index_entry *entry : table->find (last_name,
completing))
{
@@ -16702,9 +16705,6 @@ cooked_index_functions::expand_symtabs_matching
if (entry->lang != language_unknown)
{
const language_defn *lang_def = language_def (entry->lang);
- lookup_name_info last_segment_lookup_name (
- last_name.data (), symbol_name_match_type::FULL,
- false, true);
symbol_name_matcher_ftype *name_matcher
= lang_def->get_symbol_name_matcher
(last_segment_lookup_name);
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3][PR symtab/30520 3/4] gdb/symtab: compute match_type outside the loop
2024-05-06 15:09 [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU Dmitry Neverov
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 2/4] gdb/symtab: reuse last segment lookup name info by creating it outside the loop Dmitry Neverov
@ 2024-05-06 15:09 ` Dmitry Neverov
2024-05-07 17:10 ` Tom Tromey
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 4/4] gdb/symtab: use symbol name matcher for all segments in a qualified name Dmitry Neverov
2024-05-07 17:03 ` [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU Tom Tromey
3 siblings, 1 reply; 12+ messages in thread
From: Dmitry Neverov @ 2024-05-06 15:09 UTC (permalink / raw)
To: gdb-patches; +Cc: dmitry.neverov
It will be used for all segments in a qualified name,
not only the last one.
---
gdb/dwarf2/read.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a49ade9dc31..f2842f0f581 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16637,6 +16637,9 @@ cooked_index_functions::expand_symtabs_matching
language_ada
};
+ symbol_name_match_type match_type
+ = lookup_name_without_params.match_type ();
+
for (enum language lang : unique_styles)
{
std::vector<std::string_view> name_vec
@@ -16693,8 +16696,6 @@ cooked_index_functions::expand_symtabs_matching
"x::a::b". */
if (symbol_matcher == nullptr)
{
- symbol_name_match_type match_type
- = lookup_name_without_params.match_type ();
if ((match_type == symbol_name_match_type::FULL
|| (lang != language_ada
&& match_type == symbol_name_match_type::EXPRESSION)))
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3][PR symtab/30520 4/4] gdb/symtab: use symbol name matcher for all segments in a qualified name
2024-05-06 15:09 [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU Dmitry Neverov
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 2/4] gdb/symtab: reuse last segment lookup name info by creating it outside the loop Dmitry Neverov
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 3/4] gdb/symtab: compute match_type " Dmitry Neverov
@ 2024-05-06 15:09 ` Dmitry Neverov
2024-05-06 15:23 ` Dmitry Neverov
2024-05-07 17:26 ` Tom Tromey
2024-05-07 17:03 ` [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU Tom Tromey
3 siblings, 2 replies; 12+ messages in thread
From: Dmitry Neverov @ 2024-05-06 15:09 UTC (permalink / raw)
To: gdb-patches; +Cc: dmitry.neverov
---
gdb/dwarf2/read.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f2842f0f581..e8416905163 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16644,12 +16644,16 @@ cooked_index_functions::expand_symtabs_matching
{
std::vector<std::string_view> name_vec
= lookup_name_without_params.split_name (lang);
- std::string last_name (name_vec.back ());
-
- lookup_name_info last_segment_lookup_name (
- last_name, symbol_name_match_type::FULL, completing, true);
+ std::vector<std::string> name_str_vec (name_vec.begin (), name_vec.end ());
+ std::vector<lookup_name_info> segment_lookup_names;
+ segment_lookup_names.reserve (name_vec.size ());
+ for (auto &segment_name : name_str_vec)
+ {
+ segment_lookup_names.emplace_back (segment_name,
+ symbol_name_match_type::FULL, completing, true);
+ }
- for (const cooked_index_entry *entry : table->find (last_name,
+ for (const cooked_index_entry *entry : table->find (name_str_vec.back (),
completing))
{
QUIT;
@@ -16678,13 +16682,24 @@ cooked_index_functions::expand_symtabs_matching
{
/* If we ran out of entries, or if this segment doesn't
match, this did not match. */
- if (parent == nullptr
- || strncmp (parent->name, name_vec[i - 1].data (),
- name_vec[i - 1].length ()) != 0)
+ if (parent == nullptr)
{
found = false;
break;
}
+ if (parent->lang != language_unknown)
+ {
+ const language_defn *lang_def = language_def (parent->lang);
+ symbol_name_matcher_ftype *name_matcher
+ = lang_def->get_symbol_name_matcher
+ (segment_lookup_names[i-1]);
+ if (!name_matcher (parent->canonical,
+ segment_lookup_names[i-1], nullptr))
+ {
+ found = false;
+ break;
+ }
+ }
parent = parent->get_parent ();
}
@@ -16708,9 +16723,9 @@ cooked_index_functions::expand_symtabs_matching
const language_defn *lang_def = language_def (entry->lang);
symbol_name_matcher_ftype *name_matcher
= lang_def->get_symbol_name_matcher
- (last_segment_lookup_name);
+ (segment_lookup_names.back ());
if (!name_matcher (entry->canonical,
- last_segment_lookup_name, nullptr))
+ segment_lookup_names.back (), nullptr))
continue;
}
}
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3][PR symtab/30520 4/4] gdb/symtab: use symbol name matcher for all segments in a qualified name
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 4/4] gdb/symtab: use symbol name matcher for all segments in a qualified name Dmitry Neverov
@ 2024-05-06 15:23 ` Dmitry Neverov
2024-05-07 17:26 ` Tom Tromey
1 sibling, 0 replies; 12+ messages in thread
From: Dmitry Neverov @ 2024-05-06 15:23 UTC (permalink / raw)
To: gdb-patches
I've run tests on x86-64 linux. The only unexpected failure was
FAIL: gdb.base/empty-host-env-vars.exp: env_var_name=HOME: show
index-cache directory
But it doesn't seem to be related to this patch.
--
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3][PR symtab/30520 4/4] gdb/symtab: use symbol name matcher for all segments in a qualified name
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 4/4] gdb/symtab: use symbol name matcher for all segments in a qualified name Dmitry Neverov
2024-05-06 15:23 ` Dmitry Neverov
@ 2024-05-07 17:26 ` Tom Tromey
1 sibling, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2024-05-07 17:26 UTC (permalink / raw)
To: Dmitry Neverov; +Cc: gdb-patches
>>>>> "Dmitry" == Dmitry Neverov <dmitry.neverov@jetbrains.com> writes:
Thanks for the patch.
Dmitry> std::vector<std::string_view> name_vec
Dmitry> = lookup_name_without_params.split_name (lang);
Dmitry> - std::string last_name (name_vec.back ());
Dmitry> -
Dmitry> - lookup_name_info last_segment_lookup_name (
Dmitry> - last_name, symbol_name_match_type::FULL, completing, true);
Dmitry> + std::vector<std::string> name_str_vec (name_vec.begin (), name_vec.end ());
I was going to suggest changing split_name to simply return this vector,
but I'd forgotten that cooked-index.c also uses it.
Dmitry> + std::vector<lookup_name_info> segment_lookup_names;
Dmitry> + segment_lookup_names.reserve (name_vec.size ());
Dmitry> + for (auto &segment_name : name_str_vec)
Dmitry> + {
Dmitry> + segment_lookup_names.emplace_back (segment_name,
Dmitry> + symbol_name_match_type::FULL, completing, true);
This should be indented more to line up with "segment_name" from the
previous line.
Otherwise this looks good to me.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU
2024-05-06 15:09 [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU Dmitry Neverov
` (2 preceding siblings ...)
2024-05-06 15:09 ` [PATCH v3][PR symtab/30520 4/4] gdb/symtab: use symbol name matcher for all segments in a qualified name Dmitry Neverov
@ 2024-05-07 17:03 ` Tom Tromey
2024-05-08 11:32 ` Dmitry.Neverov
3 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2024-05-07 17:03 UTC (permalink / raw)
To: Dmitry Neverov; +Cc: gdb-patches
>>>>> "Dmitry" == Dmitry Neverov <dmitry.neverov@jetbrains.com> writes:
Thank you for your continued work on this.
Dmitry> + const language_defn *lang_def = language_def (entry->lang);
Dmitry> + lookup_name_info last_segment_lookup_name (
Dmitry> + last_name.data (), symbol_name_match_type::FULL,
Dmitry> + false, true);
Normally in gdb the open paren would go on the next line, like
> + lookup_name_info last_segment_lookup_name
> + (last_name.data (), symbol_name_match_type::FULL,
> + false, true);
... depending on how long the lines are -- it would be split after some
',' if each line would then fit in 80 columns.
Dmitry> + symbol_name_matcher_ftype *name_matcher
Dmitry> + = lang_def->get_symbol_name_matcher
Dmitry> + (last_segment_lookup_name);
This also looks weird by gdb rules.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU
2024-05-07 17:03 ` [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU Tom Tromey
@ 2024-05-08 11:32 ` Dmitry.Neverov
2024-05-08 15:48 ` Tom Tromey
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry.Neverov @ 2024-05-08 11:32 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Tom Tromey <tom@tromey.com> writes:
> Dmitry> + symbol_name_matcher_ftype *name_matcher
> Dmitry> + = lang_def->get_symbol_name_matcher
> Dmitry> + (last_segment_lookup_name);
>
> This also looks weird by gdb rules.
Do I understand correctly that it should look like this:
symbol_name_matcher_ftype *name_matcher = lang_def
->get_symbol_name_matcher (last_segment_lookup_name);
?
How do I send an updated patch? I've read the Contribution Checklist,
but didn't find the answer. Do I amend the commit and 'git send-mail' it
again with proper '--in-reply-to'?
--
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3][PR symtab/30520 1/4] gdb/symtab: check name matches before expanding a CU
2024-05-08 11:32 ` Dmitry.Neverov
@ 2024-05-08 15:48 ` Tom Tromey
2024-05-09 5:55 ` Dmitry.Neverov
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2024-05-08 15:48 UTC (permalink / raw)
To: Dmitry.Neverov; +Cc: Tom Tromey, gdb-patches
>>>>> "Dmitry" == Dmitry Neverov <dmitry.neverov@jetbrains.com> writes:
Dmitry> Tom Tromey <tom@tromey.com> writes:
Dmitry> + symbol_name_matcher_ftype *name_matcher
Dmitry> + = lang_def->get_symbol_name_matcher
Dmitry> + (last_segment_lookup_name);
>>
>> This also looks weird by gdb rules.
Dmitry> Do I understand correctly that it should look like this:
I'd write:
symbol_name_matcher_ftype *name_matcher
= lang_def->get_symbol_name_matcher (last_segment_lookup_name);
Sometimes wrapping is unavoidable but there's also
symbol_name_matcher_ftype *name_matcher
= (lang_def->get_symbol_name_matcher
(last_segment_lookup_name));
Dmitry> How do I send an updated patch? I've read the Contribution Checklist,
Dmitry> but didn't find the answer. Do I amend the commit and 'git send-mail' it
Dmitry> again with proper '--in-reply-to'?
I just re-send it with --reroll-count=...
I don't normally worry about in-reply-to. Not sure, maybe I should?
For series I often use b4 these days, though I've also been bitten by
its bizarre behavior, so I'm not sure I can really recommend it.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread