[v2,03/11] gdb: factor out regexp compilation from global_symbol_searcher::search
Commit Message
From: Simon Marchi <simon.marchi@efficios.com>
Factor out the scopes that compile the regexps, in order to make
global_symbol_searcher::search itself simpler to read. Factor out a helper
to compile a symbol search regex as well.
Change-Id: Ie4485b2eb593967069ac635c933df817bc5304e6
---
gdb/symtab.c | 118 ++++++++++++++++++++++++++++++---------------------
gdb/symtab.h | 9 ++++
2 files changed, 78 insertions(+), 49 deletions(-)
Comments
simon.marchi@polymtl.ca writes:
> From: Simon Marchi <simon.marchi@efficios.com>
>
> Factor out the scopes that compile the regexps, in order to make
> global_symbol_searcher::search itself simpler to read. Factor out a helper
> to compile a symbol search regex as well.
Three super minor nits, two of which are pre-existing, see inline...
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index aae82218b5cf..49e8719f4ec0 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4938,66 +4938,86 @@ global_symbol_searcher::add_matching_msymbols
> return true;
> }
>
> -/* See symtab.h. */
> +/* Compile REGEXP, a symbol name or symbol type regexp.
>
> -std::vector<symbol_search>
> -global_symbol_searcher::search () const
> + If REGEXP is nullptr, return an empty optional. */
> +
> +static std::optional<compiled_regex>
> +compile_symbol_search_regex (const char *regexp)
> {
> - std::optional<compiled_regex> name_regex;
> - std::optional<compiled_regex> type_regex;
> + if (regexp == nullptr)
> + return {};
>
> - if (m_symbol_name_regexp != NULL)
> - {
> - const char *symbol_name_regexp = m_symbol_name_regexp;
> - std::string symbol_name_regexp_holder;
> + int cflags
> + = REG_NOSUB | (case_sensitivity == case_sensitive_off ? REG_ICASE : 0);
>
> - /* Make sure spacing is right for C++ operators.
> - This is just a courtesy to make the matching less sensitive
> - to how many spaces the user leaves between 'operator'
> - and <TYPENAME> or <OPERATOR>. */
> - const char *op_end;
> - const char *opname = operator_chars (symbol_name_regexp, &op_end);
> + return std::make_optional<compiled_regex> (regexp, cflags,
> + _("Invalid regexp"));
I think the indentation of the "Invalid regexp" line is off.
> +}
>
> - if (*opname)
> - {
> - int fix = -1; /* -1 means ok; otherwise number of
> - spaces needed. */
> +/* See symtab.h. */
>
> - if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
> - {
> - /* There should 1 space between 'operator' and 'TYPENAME'. */
> - if (opname[-1] != ' ' || opname[-2] == ' ')
> - fix = 1;
> - }
> - else
> - {
> - /* There should 0 spaces between 'operator' and 'OPERATOR'. */
> - if (opname[-1] == ' ')
> - fix = 0;
> - }
> - /* If wrong number of spaces, fix it. */
> - if (fix >= 0)
> - {
> - symbol_name_regexp_holder
> - = string_printf ("operator%.*s%s", fix, " ", opname);
> - symbol_name_regexp = symbol_name_regexp_holder.c_str ();
> - }
> - }
> +std::optional<compiled_regex>
> +global_symbol_searcher::compile_name_regex () const
> +{
> + if (m_symbol_name_regexp == nullptr)
> + return {};
>
> - int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
> - ? REG_ICASE : 0);
> - name_regex.emplace (symbol_name_regexp, cflags,
> - _("Invalid regexp"));
> - }
> + const char *symbol_name_regexp = m_symbol_name_regexp;
> + std::string symbol_name_regexp_holder;
> +
> + /* Make sure spacing is right for C++ operators.
> + This is just a courtesy to make the matching less sensitive
> + to how many spaces the user leaves between 'operator'
> + and <TYPENAME> or <OPERATOR>. */
> + const char *op_end;
> + const char *opname = operator_chars (symbol_name_regexp, &op_end);
>
> - if (m_symbol_type_regexp != NULL)
> + if (*opname)
> {
> - int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
> - ? REG_ICASE : 0);
> - type_regex.emplace (m_symbol_type_regexp, cflags,
> - _("Invalid regexp"));
> + /* -1 means ok; otherwise number of spaces needed. */
> + int fix = -1;
> +
> + if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
> + {
> + /* There should 1 space between 'operator' and 'TYPENAME'. */
This comment has a (pre-existing) missing 'be':
/* There should BE 1 space between .... */
> + if (opname[-1] != ' ' || opname[-2] == ' ')
> + fix = 1;
> + }
> + else
> + {
> + /* There should 0 spaces between 'operator' and 'OPERATOR'. */
Same missing 'BE' here.
With those all fixed:
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>> +static std::optional<compiled_regex>
>> +compile_symbol_search_regex (const char *regexp)
>> {
>> - std::optional<compiled_regex> name_regex;
>> - std::optional<compiled_regex> type_regex;
>> + if (regexp == nullptr)
>> + return {};
>>
>> - if (m_symbol_name_regexp != NULL)
>> - {
>> - const char *symbol_name_regexp = m_symbol_name_regexp;
>> - std::string symbol_name_regexp_holder;
>> + int cflags
>> + = REG_NOSUB | (case_sensitivity == case_sensitive_off ? REG_ICASE : 0);
>>
>> - /* Make sure spacing is right for C++ operators.
>> - This is just a courtesy to make the matching less sensitive
>> - to how many spaces the user leaves between 'operator'
>> - and <TYPENAME> or <OPERATOR>. */
>> - const char *op_end;
>> - const char *opname = operator_chars (symbol_name_regexp, &op_end);
>> + return std::make_optional<compiled_regex> (regexp, cflags,
>> + _("Invalid regexp"));
>
> I think the indentation of the "Invalid regexp" line is off.
Ah yeah it's because I switched to make_optional at the last minute (it
avoids using std::in_place.
>> - if (m_symbol_type_regexp != NULL)
>> + if (*opname)
>> {
>> - int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
>> - ? REG_ICASE : 0);
>> - type_regex.emplace (m_symbol_type_regexp, cflags,
>> - _("Invalid regexp"));
>> + /* -1 means ok; otherwise number of spaces needed. */
>> + int fix = -1;
>> +
>> + if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
>> + {
>> + /* There should 1 space between 'operator' and 'TYPENAME'. */
>
> This comment has a (pre-existing) missing 'be':
>
> /* There should BE 1 space between .... */
>
>> + if (opname[-1] != ' ' || opname[-2] == ' ')
>> + fix = 1;
>> + }
>> + else
>> + {
>> + /* There should 0 spaces between 'operator' and 'OPERATOR'. */
>
> Same missing 'BE' here.
Fixed those 2.
>
> With those all fixed:
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Simon
@@ -4938,66 +4938,86 @@ global_symbol_searcher::add_matching_msymbols
return true;
}
-/* See symtab.h. */
+/* Compile REGEXP, a symbol name or symbol type regexp.
-std::vector<symbol_search>
-global_symbol_searcher::search () const
+ If REGEXP is nullptr, return an empty optional. */
+
+static std::optional<compiled_regex>
+compile_symbol_search_regex (const char *regexp)
{
- std::optional<compiled_regex> name_regex;
- std::optional<compiled_regex> type_regex;
+ if (regexp == nullptr)
+ return {};
- if (m_symbol_name_regexp != NULL)
- {
- const char *symbol_name_regexp = m_symbol_name_regexp;
- std::string symbol_name_regexp_holder;
+ int cflags
+ = REG_NOSUB | (case_sensitivity == case_sensitive_off ? REG_ICASE : 0);
- /* Make sure spacing is right for C++ operators.
- This is just a courtesy to make the matching less sensitive
- to how many spaces the user leaves between 'operator'
- and <TYPENAME> or <OPERATOR>. */
- const char *op_end;
- const char *opname = operator_chars (symbol_name_regexp, &op_end);
+ return std::make_optional<compiled_regex> (regexp, cflags,
+ _("Invalid regexp"));
+}
- if (*opname)
- {
- int fix = -1; /* -1 means ok; otherwise number of
- spaces needed. */
+/* See symtab.h. */
- if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
- {
- /* There should 1 space between 'operator' and 'TYPENAME'. */
- if (opname[-1] != ' ' || opname[-2] == ' ')
- fix = 1;
- }
- else
- {
- /* There should 0 spaces between 'operator' and 'OPERATOR'. */
- if (opname[-1] == ' ')
- fix = 0;
- }
- /* If wrong number of spaces, fix it. */
- if (fix >= 0)
- {
- symbol_name_regexp_holder
- = string_printf ("operator%.*s%s", fix, " ", opname);
- symbol_name_regexp = symbol_name_regexp_holder.c_str ();
- }
- }
+std::optional<compiled_regex>
+global_symbol_searcher::compile_name_regex () const
+{
+ if (m_symbol_name_regexp == nullptr)
+ return {};
- int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
- ? REG_ICASE : 0);
- name_regex.emplace (symbol_name_regexp, cflags,
- _("Invalid regexp"));
- }
+ const char *symbol_name_regexp = m_symbol_name_regexp;
+ std::string symbol_name_regexp_holder;
+
+ /* Make sure spacing is right for C++ operators.
+ This is just a courtesy to make the matching less sensitive
+ to how many spaces the user leaves between 'operator'
+ and <TYPENAME> or <OPERATOR>. */
+ const char *op_end;
+ const char *opname = operator_chars (symbol_name_regexp, &op_end);
- if (m_symbol_type_regexp != NULL)
+ if (*opname)
{
- int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
- ? REG_ICASE : 0);
- type_regex.emplace (m_symbol_type_regexp, cflags,
- _("Invalid regexp"));
+ /* -1 means ok; otherwise number of spaces needed. */
+ int fix = -1;
+
+ if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
+ {
+ /* There should 1 space between 'operator' and 'TYPENAME'. */
+ if (opname[-1] != ' ' || opname[-2] == ' ')
+ fix = 1;
+ }
+ else
+ {
+ /* There should 0 spaces between 'operator' and 'OPERATOR'. */
+ if (opname[-1] == ' ')
+ fix = 0;
+ }
+ /* If wrong number of spaces, fix it. */
+ if (fix >= 0)
+ {
+ symbol_name_regexp_holder
+ = string_printf ("operator%.*s%s", fix, " ", opname);
+ symbol_name_regexp = symbol_name_regexp_holder.c_str ();
+ }
}
+ return compile_symbol_search_regex (symbol_name_regexp);
+}
+
+/* See symtab.h. */
+
+std::optional<compiled_regex>
+global_symbol_searcher::compile_type_regex () const
+{
+ return compile_symbol_search_regex (m_symbol_type_regexp);
+}
+
+/* See symtab.h. */
+
+std::vector<symbol_search>
+global_symbol_searcher::search () const
+{
+ std::optional<compiled_regex> name_regex = compile_name_regex ();
+ std::optional<compiled_regex> type_regex = compile_type_regex ();
+
bool found_func_msymbol_without_debug_info = false;
std::set<symbol_search> result_set;
for (objfile &objfile : current_program_space->objfiles ())
@@ -2670,6 +2670,15 @@ class global_symbol_searcher
of SIZE_MAX, there is no "unlimited". */
size_t m_max_search_results = SIZE_MAX;
+ /* Compile M_SYMBOL_NAME_REGEXP, if set.
+
+ This function normalizes the spacing when it detects a C++ operator in the
+ regexp. */
+ std::optional<compiled_regex> compile_name_regex () const;
+
+ /* Compile M_SYMBOL_TYPE_REGEXP, if set. */
+ std::optional<compiled_regex> compile_type_regex () const;
+
/* Expand symtabs in OBJFILE that match NAME_REGEX, are of type M_KIND.
Return true if any msymbols were seen that we should later consider
adding to the results list. */