On 06/02/2017 05:22 AM, Pedro Alves wrote:
> diff --git a/gdb/completer.h b/gdb/completer.h
> index df90822..db781ab 100644
> --- a/gdb/completer.h
> +++ b/gdb/completer.h
> @@ -116,12 +116,44 @@ private:
> std::string m_storage;
> };
>
> +/* The result of a successful completion match, but for least common
> + denominator (LCD) computation. Some completers provide matches
> + that don't start with the completion "word". E.g., completing on
> + "b push_ba" on a C++ program usually completes to
> + std::vector<...>::push_back, std::string::push_back etc. In such
> + case, the symbol comparison routine will set the LCD match to point
> + into the "push_back" substring within the symbol's name string. */
[missing newline?]
> +class completion_match_for_lcd
> +{
> +public:
> + /* Set the match for LCD. See m_match's description. */
> + void set_match (const char *match)
> + { m_match = match; }
> +
> + /* Get the resulting LCD, after a successful match. */
> + const char *finish ()
> + { return m_match; }
> +
> + /* Prepare for another completion matching sequence. */
> + void clear ()
> + { m_match = NULL; }
> +
> +private:
> + /* The completion match result for LCD. This is usually either a
> + pointer into to a substring within a symbol's name, or to the
> + storage of the pairing completion_match object. */
> + const char *m_match;
> +};
> +
> /* Convenience aggregate holding info returned by the symbol name
> matching routines (see symbol_name_matcher_ftype). */
> struct completion_match_result
> {
> /* The completion match candidate. */
> completion_match match;
> +
> + /* The completion match, for LCD computation purposes. */
> + completion_match_for_lcd match_for_lcd;
> };
>
> /* The final result of a completion that is handed over to either
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index 064a45b..397c738 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -1660,8 +1660,11 @@ cp_fq_symbol_name_matches (const char *symbol_search_name,
> name.c_str (), name.size (),
> mode) == 0)
> {
> - if (match != NULL)
> - match->set_match (symbol_search_name);
> + if (comp_match_res != NULL)
> + {
> + comp_match_res->match.set_match (symbol_search_name);
> + comp_match_res->match_for_lcd.set_match (symbol_search_name);
> + }
> return true;
> }
>
> diff --git a/gdb/language.c b/gdb/language.c
> index 6705a3b..511a86f 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -725,8 +725,11 @@ default_symbol_name_matcher (const char *symbol_search_name,
> if (strncmp_iw_with_mode (symbol_search_name, name.c_str (), name.size (),
> mode) == 0)
> {
> - if (match != NULL)
> - match->set_match (symbol_search_name);
> + if (comp_match_res != NULL)
> + {
> + comp_match_res->match.set_match (symbol_search_name);
> + comp_match_res->match_for_lcd.set_match (symbol_search_name);
> + }
> return true;
> }
> else
In the above two hunks, the code calls result->match.set_match (A)
and result->match_for_lcd.set_match (A), i.e., both these calls take the
same argument and are called at the same time.
Furthermore, on the final series of the patch,
$ grep -n set_match\ \( *.c
ada-lang.c:6416: comp_match_res->match.set_match (match_str.c_str ());
ada-lang.c:6417: comp_match_res->match_for_lcd.set_match (match_str.c_str ());
ada-lang.c:6426: comp_match_res->match.set_match (match_str.c_str ());
ada-lang.c:6427: comp_match_res->match_for_lcd.set_match (match_str.c_str ());
cp-support.c:1734: comp_match_res->match.set_match (symbol_search_name);
cp-support.c:1735: comp_match_res->match_for_lcd.set_match (sname);
cp-support.c:1773: comp_match_res->match.set_match (symbol_search_name);
cp-support.c:1774: comp_match_res->match_for_lcd.set_match (symbol_search_name);
language.c:725: comp_match_res->match.set_match (symbol_search_name);
language.c:726: comp_match_res->match_for_lcd.set_match (symbol_search_name);
It appears that these two are /always/ called together, suggesting that
completion_match_result might "benefit" from a convenience method to set both
of these parameters at the same time. [Unless there is a specific reason
not to do this, of course.] WDYT?
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index d68eed8..736fea0 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -292,15 +292,21 @@ private:
>
> SYMBOL_SEARCH_NAME should be a symbol's "search" name.
>
> - On success and if non-NULL, MATCH is set to point to the symbol
> - name as should be presented to the user as a completion match list
> - element. In most languages, this is the same as the symbol's
> - search name, but in some, like Ada, the display name is dynamically
> - computed within the comparison routine. */
> + On success and if non-NULL, COMP_MATCH_RES->match is set to point
> + to the symbol name as should be presented to the user as a
> + completion match list element. In most languages, this is the same
> + as the symbol's search name, but in some, like Ada, the display
> + name is dynamically computed within the comparison routine.
> +
> + Also, on success and if non-NULL, COMP_MATCH_RES->match_for_lcd
> + points the part of SYMBOL_SEARCH_NAME that was considered to match
^
missing "to"
> + LOOKUP_NAME. E.g., in C++, in linespec/wild mode, if the symbol is
> + "foo::function()" and LOOKUP_NAME is "function(", MATCH_FOR_LCD
> + points to "function()" inside SYMBOL_SEARCH_NAME. */
> typedef bool (symbol_name_matcher_ftype)
> (const char *symbol_search_name,
> const lookup_name_info &lookup_name,
> - completion_match *match);
> + completion_match_result *comp_match_res);
>
> /* Some of the structures in this file are space critical.
> The space-critical structures are:
>
Keith
@@ -6341,7 +6341,7 @@ bool
ada_lookup_name_info::matches
(const char *sym_name,
symbol_name_match_type match_type,
- completion_match *comp_match) const
+ completion_match_result *comp_match_res) const
{
bool match = false;
const char *text = m_encoded_name.c_str ();
@@ -6399,14 +6399,15 @@ ada_lookup_name_info::matches
if (!match)
return false;
- if (comp_match != NULL)
+ if (comp_match_res != NULL)
{
- std::string &match_str = comp_match->storage ();
+ std::string &match_str = comp_match_res->match.storage ();
if (!m_encoded_p)
{
match_str = ada_decode (sym_name);
- comp_match->set_match (match_str.c_str ());
+ comp_match_res->match.set_match (match_str.c_str ());
+ comp_match_res->match_for_lcd.set_match (match_str.c_str ());
}
else
{
@@ -6415,7 +6416,8 @@ ada_lookup_name_info::matches
else
match_str = sym_name;
- comp_match->set_match (match_str.c_str ());
+ comp_match_res->match.set_match (match_str.c_str ());
+ comp_match_res->match_for_lcd.set_match (match_str.c_str ());
}
}
@@ -13863,7 +13865,7 @@ static const struct exp_descriptor ada_exp_descriptor = {
static bool
do_wild_match (const char *symbol_search_name,
const lookup_name_info &lookup_name,
- completion_match *match)
+ completion_match_result *comp_match_res)
{
return wild_match (symbol_search_name, ada_lookup_name (lookup_name));
}
@@ -13873,7 +13875,7 @@ do_wild_match (const char *symbol_search_name,
static bool
do_full_match (const char *symbol_search_name,
const lookup_name_info &lookup_name,
- completion_match *match)
+ completion_match_result *comp_match_res)
{
return full_match (symbol_search_name, ada_lookup_name (lookup_name));
}
@@ -13943,11 +13945,11 @@ ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)
static bool
ada_symbol_name_matches (const char *symbol_search_name,
const lookup_name_info &lookup_name,
- completion_match *match)
+ completion_match_result *comp_match_res)
{
return lookup_name.ada ().matches (symbol_search_name,
lookup_name.match_type (),
- match);
+ comp_match_res);
}
/* Implement the "la_get_symbol_name_matcher" language_defn method for
@@ -1503,7 +1503,9 @@ completion_tracker::~completion_tracker ()
/* See completer.h. */
bool
-completion_tracker::maybe_add_completion (gdb::unique_xmalloc_ptr<char> name)
+completion_tracker::maybe_add_completion
+ (gdb::unique_xmalloc_ptr<char> name,
+ completion_match_for_lcd *match_for_lcd)
{
void **slot;
@@ -1516,7 +1518,13 @@ completion_tracker::maybe_add_completion (gdb::unique_xmalloc_ptr<char> name)
slot = htab_find_slot (m_entries_hash, name.get (), INSERT);
if (*slot == HTAB_EMPTY_ENTRY)
{
- const char *match_for_lcd_str = name.get ();
+ const char *match_for_lcd_str = NULL;
+
+ if (match_for_lcd != NULL)
+ match_for_lcd_str = match_for_lcd->finish ();
+
+ if (match_for_lcd_str == NULL)
+ match_for_lcd_str = name.get ();
recompute_lowest_common_denominator (match_for_lcd_str);
@@ -1528,9 +1536,10 @@ completion_tracker::maybe_add_completion (gdb::unique_xmalloc_ptr<char> name)
}
void
-completion_tracker::add_completion (gdb::unique_xmalloc_ptr<char> name)
+completion_tracker::add_completion (gdb::unique_xmalloc_ptr<char> name,
+ completion_match_for_lcd *match_for_lcd)
{
- if (!maybe_add_completion (std::move (name)))
+ if (!maybe_add_completion (std::move (name), match_for_lcd))
throw_max_completions_reached_error ();
}
@@ -116,12 +116,44 @@ private:
std::string m_storage;
};
+/* The result of a successful completion match, but for least common
+ denominator (LCD) computation. Some completers provide matches
+ that don't start with the completion "word". E.g., completing on
+ "b push_ba" on a C++ program usually completes to
+ std::vector<...>::push_back, std::string::push_back etc. In such
+ case, the symbol comparison routine will set the LCD match to point
+ into the "push_back" substring within the symbol's name string. */
+class completion_match_for_lcd
+{
+public:
+ /* Set the match for LCD. See m_match's description. */
+ void set_match (const char *match)
+ { m_match = match; }
+
+ /* Get the resulting LCD, after a successful match. */
+ const char *finish ()
+ { return m_match; }
+
+ /* Prepare for another completion matching sequence. */
+ void clear ()
+ { m_match = NULL; }
+
+private:
+ /* The completion match result for LCD. This is usually either a
+ pointer into to a substring within a symbol's name, or to the
+ storage of the pairing completion_match object. */
+ const char *m_match;
+};
+
/* Convenience aggregate holding info returned by the symbol name
matching routines (see symbol_name_matcher_ftype). */
struct completion_match_result
{
/* The completion match candidate. */
completion_match match;
+
+ /* The completion match, for LCD computation purposes. */
+ completion_match_for_lcd match_for_lcd;
};
/* The final result of a completion that is handed over to either
@@ -190,6 +222,21 @@ public:
that necessitates the time consuming expansion of many symbol
tables.
+ - The completer's idea of least common denominator (aka the common
+ prefix) between all completion matches to hand over to readline.
+ Some completers provide matches that don't start with the
+ completion "word". E.g., completing on "b push_ba" on a C++
+ program usually completes to std::vector<...>::push_back,
+ std::string::push_back etc. If all matches happen to start with
+ "std::", then readline would figure out that the lowest common
+ denominator is "std::", and thus would do a partial completion
+ with that. I.e., it would replace "push_ba" in the input buffer
+ with "std::", losing the original "push_ba", which is obviously
+ undesirable. To avoid that, such completers pass the substring
+ of the match that matters for common denominator computation as
+ MATCH_FOR_LCD argument to add_completion. The end result is
+ passed to readline in gdb_rl_attempted_completion_function.
+
- The custom word point to hand over to readline, for completers
that parse the input string in order to dynamically adjust
themselves depending on exactly what they're completing. E.g.,
@@ -209,7 +256,8 @@ public:
/* Add the completion NAME to the list of generated completions if
it is not there already. If too many completions were already
found, this throws an error. */
- void add_completion (gdb::unique_xmalloc_ptr<char> name);
+ void add_completion (gdb::unique_xmalloc_ptr<char> name,
+ completion_match_for_lcd *match_for_lcd = NULL);
/* Add all completions matches in LIST. Elements are moved out of
LIST. */
@@ -272,6 +320,7 @@ public:
/* Clear any previous match. */
res.match.clear ();
+ res.match_for_lcd.clear ();
return m_completion_match_result;
}
@@ -294,10 +343,19 @@ private:
/* Add the completion NAME to the list of generated completions if
it is not there already. If false is returned, too many
completions were found. */
- bool maybe_add_completion (gdb::unique_xmalloc_ptr<char> name);
+ bool maybe_add_completion (gdb::unique_xmalloc_ptr<char> name,
+ completion_match_for_lcd *match_for_lcd);
/* Given a new match, recompute the lowest common denominator (LCD)
- to hand over to readline. */
+ to hand over to readline. Normally readline computes this itself
+ based on the whole set of completion matches. However, some
+ completers want to override readline, in order to be able to
+ provide a LCD that is not really a prefix of the matches, but the
+ lowest common denominator of some relevant substring of each
+ match. E.g., "b push_ba" completes to
+ "std::vector<..>::push_back", "std::string::push_back", etc., and
+ in this case we want the lowest common denominator to be
+ "push_back" instead of "std::". */
void recompute_lowest_common_denominator (const char *new_match);
/* Completion match outputs returned by the symbol name matching
@@ -352,8 +410,13 @@ private:
See intro. */
char *m_lowest_common_denominator = NULL;
- /* If true, the LCD is unique. I.e., all completion candidates had
- the same string. */
+ /* If true, the LCD is unique. I.e., all completions had the same
+ MATCH_FOR_LCD substring, even if the completions were different.
+ For example, if "break function<tab>" found "a::function()" and
+ "b::function()", the LCD will be "function()" in both cases and
+ so we want to tell readline to complete the line with
+ "function()", instead of showing all the possible
+ completions. */
bool m_lowest_common_denominator_unique = false;
};
@@ -1647,7 +1647,7 @@ gdb_sniff_from_mangled_name (const char *mangled, char **demangled)
static bool
cp_fq_symbol_name_matches (const char *symbol_search_name,
const lookup_name_info &lookup_name,
- completion_match *match)
+ completion_match_result *comp_match_res)
{
/* Get the demangled name. */
const std::string &name = lookup_name.cplus ().lookup_name ();
@@ -1660,8 +1660,11 @@ cp_fq_symbol_name_matches (const char *symbol_search_name,
name.c_str (), name.size (),
mode) == 0)
{
- if (match != NULL)
- match->set_match (symbol_search_name);
+ if (comp_match_res != NULL)
+ {
+ comp_match_res->match.set_match (symbol_search_name);
+ comp_match_res->match_for_lcd.set_match (symbol_search_name);
+ }
return true;
}
@@ -714,7 +714,7 @@ default_get_string (struct value *value, gdb_byte **buffer, int *length,
bool
default_symbol_name_matcher (const char *symbol_search_name,
const lookup_name_info &lookup_name,
- completion_match *match)
+ completion_match_result *comp_match_res)
{
const std::string &name = lookup_name.name ();
@@ -725,8 +725,11 @@ default_symbol_name_matcher (const char *symbol_search_name,
if (strncmp_iw_with_mode (symbol_search_name, name.c_str (), name.size (),
mode) == 0)
{
- if (match != NULL)
- match->set_match (symbol_search_name);
+ if (comp_match_res != NULL)
+ {
+ comp_match_res->match.set_match (symbol_search_name);
+ comp_match_res->match_for_lcd.set_match (symbol_search_name);
+ }
return true;
}
else
@@ -37,6 +37,7 @@ struct type_print_options;
struct lang_varobj_ops;
struct parser_state;
struct compile_instance;
+struct completion_match_for_lcd;
#define MAX_FORTRAN_DIMS 7 /* Maximum number of F77 array dims. */
@@ -628,7 +629,7 @@ void c_get_string (struct value *value, gdb_byte **buffer, int *length,
extern bool default_symbol_name_matcher
(const char *symbol_search_name,
const lookup_name_info &lookup_name,
- completion_match *match);
+ completion_match_result *comp_match_res);
/* Get language LANG's symbol_name_matcher method for LOOKUP_NAME.
Returns default_symbol_name_matcher if not set. */
@@ -4807,7 +4807,7 @@ compare_symbol_name (const char *symbol_name, language symbol_language,
symbol_name_matcher_ftype *name_match
= language_get_symbol_name_matcher (lang, lookup_name);
- return name_match (symbol_name, lookup_name, &match_res.match);
+ return name_match (symbol_name, lookup_name, &match_res);
}
/* Test to see if the symbol specified by SYMNAME (which is already
@@ -4862,7 +4862,14 @@ completion_list_add_name (completion_tracker &tracker,
gdb::unique_xmalloc_ptr<char> completion (newobj);
- tracker.add_completion (std::move (completion));
+ /* Here we pass the match-for-lcd object to add_completion. Some
+ languages match the user text against substrings of symbol
+ names in some cases. E.g., in C++, "b push_ba" completes to
+ "std::vector::push_back", "std::string::push_back", etc., and
+ in this case we want the completion lowest common denominator
+ to be "push_back" instead of "std::". */
+ tracker.add_completion (std::move (completion),
+ &match_res.match_for_lcd);
}
}
@@ -81,7 +81,7 @@ class ada_lookup_name_info final
otherwise. If non-NULL, store the matching results in MATCH. */
bool matches (const char *symbol_search_name,
symbol_name_match_type match_type,
- completion_match *match) const;
+ completion_match_result *comp_match_res) const;
/* The Ada-encoded lookup name. */
const std::string &lookup_name () const
@@ -292,15 +292,21 @@ private:
SYMBOL_SEARCH_NAME should be a symbol's "search" name.
- On success and if non-NULL, MATCH is set to point to the symbol
- name as should be presented to the user as a completion match list
- element. In most languages, this is the same as the symbol's
- search name, but in some, like Ada, the display name is dynamically
- computed within the comparison routine. */
+ On success and if non-NULL, COMP_MATCH_RES->match is set to point
+ to the symbol name as should be presented to the user as a
+ completion match list element. In most languages, this is the same
+ as the symbol's search name, but in some, like Ada, the display
+ name is dynamically computed within the comparison routine.
+
+ Also, on success and if non-NULL, COMP_MATCH_RES->match_for_lcd
+ points the part of SYMBOL_SEARCH_NAME that was considered to match
+ LOOKUP_NAME. E.g., in C++, in linespec/wild mode, if the symbol is
+ "foo::function()" and LOOKUP_NAME is "function(", MATCH_FOR_LCD
+ points to "function()" inside SYMBOL_SEARCH_NAME. */
typedef bool (symbol_name_matcher_ftype)
(const char *symbol_search_name,
const lookup_name_info &lookup_name,
- completion_match *match);
+ completion_match_result *comp_match_res);
/* Some of the structures in this file are space critical.
The space-critical structures are: