* [PATCH 2/3] Fix regression: expression completer and scope operator (PR gdb/22584)
2017-12-13 13:26 [PATCH 0/3] Fix regression: expression completer and scope operator (PR gdb/22584) Pedro Alves
@ 2017-12-13 13:26 ` Pedro Alves
2017-12-13 13:26 ` [PATCH 1/3] Factor out final completion match string building Pedro Alves
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2017-12-13 13:26 UTC (permalink / raw)
To: gdb-patches
I noticed this regression in the expression completer:
"(gdb) p std::[TAB]" => "(gdb) p std::std::"
obviously we should have not completed to "std::std::".
The problem is that in the earlier big completer rework, I missed
taking into account the fact that with expressions, the completion
word point is not always at the start of the symbol name (it is with
linespecs).
The fix is to run the common prefix / LCD string (what readline uses
to expand the input line) through make_completion_match_str too.
New testcase included, exercising both TAB completion and the complete
command.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* completer.c (completion_tracker::maybe_add_completion): New
'text' and 'word' parameters. Use make_completion_match_str.
(completion_tracker::add_completion): New 'text' and 'word'
parameters. Pass down.
(completion_tracker::recompute_lowest_common_denominator): Change
parameter type to gdb::unique_xmalloc_ptr rval ref. Adjust.
* completer.h (completion_tracker::add_completion): New 'text' and
'word' parameters.
(completion_tracker::recompute_lowest_common_denominator): Change
parameter type to gdb::unique_xmalloc_ptr rval ref.
(completion_tracker::recompute_lowest_common_denominator): Change
parameter type to gdb::unique_xmalloc_ptr rval ref.
* symtab.c (completion_list_add_name): Pass down 'text' and 'word'
as well.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdb.cp/cpcompletion.exp: Load completion-support.exp.
("expression with namespace"): New set of tests.
* gdb.cp/pr9594.cc (Test_NS::foo, Test_NS::bar)
(Nested::Test_NS::qux): New.
* lib/completion-support.exp (test_gdb_complete_cmd_multiple): Add
defaults to 'start_quote_char' and 'end_quote_char' parameters.
---
gdb/completer.c | 23 +++++++++++------
gdb/completer.h | 9 ++++---
gdb/symtab.c | 2 +-
gdb/testsuite/gdb.cp/cpcompletion.exp | 42 ++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.cp/pr9594.cc | 13 ++++++++++
gdb/testsuite/lib/completion-support.exp | 2 +-
6 files changed, 78 insertions(+), 13 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index 0195114..844696f 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1510,7 +1510,8 @@ completion_tracker::~completion_tracker ()
bool
completion_tracker::maybe_add_completion
(gdb::unique_xmalloc_ptr<char> name,
- completion_match_for_lcd *match_for_lcd)
+ completion_match_for_lcd *match_for_lcd,
+ const char *text, const char *word)
{
void **slot;
@@ -1531,7 +1532,10 @@ completion_tracker::maybe_add_completion
if (match_for_lcd_str == NULL)
match_for_lcd_str = name.get ();
- recompute_lowest_common_denominator (match_for_lcd_str);
+ gdb::unique_xmalloc_ptr<char> lcd
+ = make_completion_match_str (match_for_lcd_str, text, word);
+
+ recompute_lowest_common_denominator (std::move (lcd));
*slot = name.get ();
m_entries_vec.push_back (std::move (name));
@@ -1544,9 +1548,10 @@ completion_tracker::maybe_add_completion
void
completion_tracker::add_completion (gdb::unique_xmalloc_ptr<char> name,
- completion_match_for_lcd *match_for_lcd)
+ completion_match_for_lcd *match_for_lcd,
+ const char *text, const char *word)
{
- if (!maybe_add_completion (std::move (name), match_for_lcd))
+ if (!maybe_add_completion (std::move (name), match_for_lcd, text, word))
throw_error (MAX_COMPLETIONS_REACHED_ERROR, _("Max completions reached."));
}
@@ -1904,21 +1909,23 @@ completion_find_completion_word (completion_tracker &tracker, const char *text,
/* See completer.h. */
void
-completion_tracker::recompute_lowest_common_denominator (const char *new_match)
+completion_tracker::recompute_lowest_common_denominator
+ (gdb::unique_xmalloc_ptr<char> &&new_match_up)
{
if (m_lowest_common_denominator == NULL)
{
/* We don't have a lowest common denominator yet, so simply take
- the whole NEW_MATCH as being it. */
- m_lowest_common_denominator = xstrdup (new_match);
+ the whole NEW_MATCH_UP as being it. */
+ m_lowest_common_denominator = new_match_up.release ();
m_lowest_common_denominator_unique = true;
}
else
{
/* Find the common denominator between the currently-known
- lowest common denominator and NEW_MATCH. That becomes the
+ lowest common denominator and NEW_MATCH_UP. That becomes the
new lowest common denominator. */
size_t i;
+ const char *new_match = new_match_up.get ();
for (i = 0;
(new_match[i] != '\0'
diff --git a/gdb/completer.h b/gdb/completer.h
index 73c0f4c..df3c8e8 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -319,7 +319,8 @@ public:
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,
- completion_match_for_lcd *match_for_lcd = NULL);
+ completion_match_for_lcd *match_for_lcd = NULL,
+ const char *text = NULL, const char *word = NULL);
/* Add all completions matches in LIST. Elements are moved out of
LIST. */
@@ -406,7 +407,8 @@ private:
it is not there already. If false is returned, too many
completions were found. */
bool maybe_add_completion (gdb::unique_xmalloc_ptr<char> name,
- completion_match_for_lcd *match_for_lcd);
+ completion_match_for_lcd *match_for_lcd,
+ const char *text, const char *word);
/* Given a new match, recompute the lowest common denominator (LCD)
to hand over to readline. Normally readline computes this itself
@@ -418,7 +420,8 @@ private:
"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);
+ void recompute_lowest_common_denominator
+ (gdb::unique_xmalloc_ptr<char> &&new_match);
/* Completion match outputs returned by the symbol name matching
routines (see symbol_name_matcher_ftype). These results are only
diff --git a/gdb/symtab.c b/gdb/symtab.c
index bb98619..220ae09 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4735,7 +4735,7 @@ completion_list_add_name (completion_tracker &tracker,
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);
+ &match_res.match_for_lcd, text, word);
}
}
diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp b/gdb/testsuite/gdb.cp/cpcompletion.exp
index c7883ee..62669dc 100644
--- a/gdb/testsuite/gdb.cp/cpcompletion.exp
+++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
@@ -15,6 +15,8 @@
# This file is part of the gdb testsuite.
+load_lib completion-support.exp
+
# A helper procedure to test location completions restricted by
# class.
proc test_class_complete {class expr name matches} {
@@ -85,3 +87,43 @@ gdb_test "complete p foo1.Fo" "p foo1\\.Foofoo"
# Test completion with an anonymous struct.
gdb_test "complete p a.g" "p a\\.get"
+
+with_test_prefix "expression with namespace" {
+ # Before the scope operator, GDB shows all the symbols whose
+ # fully-qualified name matches the completion word.
+ test_gdb_complete_multiple "p " "Test_NS" "" {
+ "Test_NS"
+ "Test_NS::Nested"
+ "Test_NS::Nested::qux"
+ "Test_NS::bar"
+ "Test_NS::foo"
+ }
+
+ # Unlike in linespecs, tab- and complete-command completion work a
+ # bit differently when completing around the scope operator. The
+ # matches in the tab-completion case only show the part of the
+ # symbol after the scope, since ':' is a word break character.
+
+ set tab_completion_list {
+ "Nested"
+ "Nested::qux"
+ "bar"
+ "foo"
+ }
+ test_gdb_complete_tab_multiple "p Test_NS:" ":" $tab_completion_list
+ test_gdb_complete_tab_multiple "p Test_NS::" "" $tab_completion_list
+
+ # OTOH, the complete command must show the whole command, with
+ # qualified symbol displayed as entered by the user.
+ set cmd_completion_list {
+ "Test_NS::Nested"
+ "Test_NS::Nested::qux"
+ "Test_NS::bar"
+ "Test_NS::foo"
+ }
+ test_gdb_complete_cmd_multiple "p " "Test_NS:" $cmd_completion_list
+ test_gdb_complete_cmd_multiple "p " "Test_NS::" $cmd_completion_list
+
+ # Add a disambiguating character and we get a unique completion.
+ test_gdb_complete_unique "p Test_NS::f" "p Test_NS::foo"
+}
diff --git a/gdb/testsuite/gdb.cp/pr9594.cc b/gdb/testsuite/gdb.cp/pr9594.cc
index 8fdee84..54ddaaf 100644
--- a/gdb/testsuite/gdb.cp/pr9594.cc
+++ b/gdb/testsuite/gdb.cp/pr9594.cc
@@ -39,6 +39,19 @@ void Foo::Foofoo ()
{
}
+namespace Test_NS {
+
+int foo;
+int bar;
+
+namespace Nested {
+
+int qux;
+
+} /* namespace Nested */
+
+} /* namespace Test_NS */
+
int main ()
{
// Anonymous struct with method.
diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp
index c7cc1c9..fe5b16a 100644
--- a/gdb/testsuite/lib/completion-support.exp
+++ b/gdb/testsuite/lib/completion-support.exp
@@ -178,7 +178,7 @@ proc test_gdb_complete_cmd_unique { input_line complete_line_re } {
# complete command displays the COMPLETION_LIST completion list. Each
# entry in the list should be prefixed by CMD_PREFIX.
-proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list start_quote_char end_quote_char } {
+proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list {start_quote_char ""} {end_quote_char ""} } {
global gdb_prompt
set expected_re [make_cmd_completion_list_re $cmd_prefix $completion_list $start_quote_char $end_quote_char]
--
2.5.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] Factor out final completion match string building
2017-12-13 13:26 [PATCH 0/3] Fix regression: expression completer and scope operator (PR gdb/22584) Pedro Alves
2017-12-13 13:26 ` [PATCH 2/3] " Pedro Alves
@ 2017-12-13 13:26 ` Pedro Alves
2017-12-13 13:26 ` [PATCH 3/3] Tighten regexp of lib/completion-support.exp:test_gdb_complete_tab_multiple Pedro Alves
2017-12-13 15:58 ` [PATCH 0/3] Fix regression: expression completer and scope operator (PR gdb/22584) Simon Marchi
3 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2017-12-13 13:26 UTC (permalink / raw)
To: gdb-patches
We have several places doing essentially the same thing; factor them
out to a central place. Some of the places overallocate for no good
reason, or use strcat unnecessarily. The centralized version is more
precise and to the point.
(I considered making the gdb::unique_xmalloc_ptr overload version of
make_completer_match_str try to realloc (not xrealloc) probably
avoiding an allocation in most cases, but that'd be probably overdoing
it, and also, now that I'm writing this I thought I'd try to see how
could we ever get to filename_completer with "text != word", but I
couldn't figure it out. Running the testsuite with 'gdb_assert (text
== word);' never tripped on the assertion either. So post gdb 8.1,
I'll probably propose a patch to simplify filename_completer a bit,
and the gdb::unique_xmalloc_str overload can be removed then.)
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* cli/cli-decode.c (complete_on_cmdlist, complete_on_enum): Use
make_completion_match_str.
* completer.c: Use gdb::unique_xmalloc_ptr and
make_completion_match_str.
(make_completion_match_str_1): New.
(make_completion_match_str(const char *, const char *,
const char *)): New.
(make_completion_match_str(gdb::unique_xmalloc_ptr<char> &&,
const char *, const char *)): New.
* completer.h (make_completion_match_str(const char *,
const char *, const char *)): New.
(make_completion_match_str(gdb::unique_xmalloc_ptr<char> &&,
const char *, const char *)): New.
* interps.c (interpreter_completer): Use make_completion_match_str.
* symtab.c (completion_list_add_name, add_filename_to_list): Use
make_completion_match_str.
---
gdb/cli/cli-decode.c | 41 ++---------------------
gdb/completer.c | 92 ++++++++++++++++++++++++++++++++++++----------------
gdb/completer.h | 15 +++++++++
gdb/interps.c | 20 ++----------
gdb/symtab.c | 50 ++--------------------------
5 files changed, 87 insertions(+), 131 deletions(-)
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 81df308..d657de2 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1816,8 +1816,6 @@ complete_on_cmdlist (struct cmd_list_element *list,
&& (!ignore_help_classes || ptr->func
|| ptr->prefixlist))
{
- char *match;
-
if (pass == 0)
{
if (ptr->cmd_deprecated)
@@ -1827,22 +1825,8 @@ complete_on_cmdlist (struct cmd_list_element *list,
}
}
- match = (char *) xmalloc (strlen (word) + strlen (ptr->name) + 1);
- if (word == text)
- strcpy (match, ptr->name);
- else if (word > text)
- {
- /* Return some portion of ptr->name. */
- strcpy (match, ptr->name + (word - text));
- }
- else
- {
- /* Return some of text plus ptr->name. */
- strncpy (match, word, text - word);
- match[text - word] = '\0';
- strcat (match, ptr->name);
- }
- tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
+ tracker.add_completion
+ (make_completion_match_str (ptr->name, text, word));
got_matches = true;
}
@@ -1876,26 +1860,7 @@ complete_on_enum (completion_tracker &tracker,
for (i = 0; (name = enumlist[i]) != NULL; i++)
if (strncmp (name, text, textlen) == 0)
- {
- char *match;
-
- match = (char *) xmalloc (strlen (word) + strlen (name) + 1);
- if (word == text)
- strcpy (match, name);
- else if (word > text)
- {
- /* Return some portion of name. */
- strcpy (match, name + (word - text));
- }
- else
- {
- /* Return some of text plus name. */
- strncpy (match, word, text - word);
- match[text - word] = '\0';
- strcat (match, name);
- }
- tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
- }
+ tracker.add_completion (make_completion_match_str (name, text, word));
}
diff --git a/gdb/completer.c b/gdb/completer.c
index 1cfabcd..0195114 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -158,10 +158,9 @@ filename_completer (struct cmd_list_element *ignore,
subsequent_name = 0;
while (1)
{
- char *p, *q;
-
- p = rl_filename_completion_function (text, subsequent_name);
- if (p == NULL)
+ gdb::unique_xmalloc_ptr<char> p_rl
+ (rl_filename_completion_function (text, subsequent_name));
+ if (p_rl == NULL)
break;
/* We need to set subsequent_name to a non-zero value before the
continue line below, because otherwise, if the first file
@@ -170,32 +169,12 @@ filename_completer (struct cmd_list_element *ignore,
subsequent_name = 1;
/* Like emacs, don't complete on old versions. Especially
useful in the "source" command. */
+ const char *p = p_rl.get ();
if (p[strlen (p) - 1] == '~')
- {
- xfree (p);
- continue;
- }
+ continue;
- if (word == text)
- /* Return exactly p. */
- q = p;
- else if (word > text)
- {
- /* Return some portion of p. */
- q = (char *) xmalloc (strlen (p) + 5);
- strcpy (q, p + (word - text));
- xfree (p);
- }
- else
- {
- /* Return some of TEXT plus p. */
- q = (char *) xmalloc (strlen (p) + (text - word) + 5);
- strncpy (q, word, text - word);
- q[text - word] = '\0';
- strcat (q, p);
- xfree (p);
- }
- tracker.add_completion (gdb::unique_xmalloc_ptr<char> (q));
+ tracker.add_completion
+ (make_completion_match_str (std::move (p_rl), text, word));
}
#if 0
/* There is no way to do this just long enough to affect quote
@@ -1580,6 +1559,63 @@ completion_tracker::add_completions (completion_list &&list)
add_completion (std::move (candidate));
}
+/* Helper for the make_completion_match_str overloads. Returns NULL
+ as an indication that we want MATCH_NAME exactly. It is up to the
+ caller to xstrdup that string if desired. */
+
+static char *
+make_completion_match_str_1 (const char *match_name,
+ const char *text, const char *word)
+{
+ char *newobj;
+
+ if (word == text)
+ {
+ /* Return NULL as an indication that we want MATCH_NAME
+ exactly. */
+ return NULL;
+ }
+ else if (word > text)
+ {
+ /* Return some portion of MATCH_NAME. */
+ newobj = xstrdup (match_name + (word - text));
+ }
+ else
+ {
+ /* Return some of WORD plus MATCH_NAME. */
+ size_t len = strlen (match_name);
+ newobj = (char *) xmalloc (text - word + len + 1);
+ memcpy (newobj, word, text - word);
+ memcpy (newobj + (text - word), match_name, len + 1);
+ }
+
+ return newobj;
+}
+
+/* See completer.h. */
+
+gdb::unique_xmalloc_ptr<char>
+make_completion_match_str (const char *match_name,
+ const char *text, const char *word)
+{
+ char *newobj = make_completion_match_str_1 (match_name, text, word);
+ if (newobj == NULL)
+ newobj = xstrdup (match_name);
+ return gdb::unique_xmalloc_ptr<char> (newobj);
+}
+
+/* See completer.h. */
+
+gdb::unique_xmalloc_ptr<char>
+make_completion_match_str (gdb::unique_xmalloc_ptr<char> &&match_name,
+ const char *text, const char *word)
+{
+ char *newobj = make_completion_match_str_1 (match_name.get (), text, word);
+ if (newobj == NULL)
+ return std::move (match_name);
+ return gdb::unique_xmalloc_ptr<char> (newobj);
+}
+
/* Generate completions all at once. Does nothing if max_completions
is 0. If max_completions is non-negative, this will collect at
most max_completions strings.
diff --git a/gdb/completer.h b/gdb/completer.h
index 9ce70bf..73c0f4c 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -482,6 +482,21 @@ private:
bool m_lowest_common_denominator_unique = false;
};
+/* Return a string to hand off to readline as a completion match
+ candidate, potentially composed of parts of MATCH_NAME and of
+ TEXT/WORD. For a description of TEXT/WORD see completer_ftype. */
+
+extern gdb::unique_xmalloc_ptr<char>
+ make_completion_match_str (const char *match_name,
+ const char *text, const char *word);
+
+/* Like above, but takes ownership of MATCH_NAME (i.e., can
+ reuse/return it). */
+
+extern gdb::unique_xmalloc_ptr<char>
+ make_completion_match_str (gdb::unique_xmalloc_ptr<char> &&match_name,
+ const char *text, const char *word);
+
extern void gdb_display_match_list (char **matches, int len, int max,
const struct match_list_displayer *);
diff --git a/gdb/interps.c b/gdb/interps.c
index b177a89..fa71bb4 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -439,24 +439,8 @@ interpreter_completer (struct cmd_list_element *ignore,
{
if (strncmp (interp.name, text, textlen) == 0)
{
- char *match;
-
- match = (char *) xmalloc (strlen (word) + strlen (interp.name) + 1);
- if (word == text)
- strcpy (match, interp.name);
- else if (word > text)
- {
- /* Return some portion of interp->name. */
- strcpy (match, interp.name + (word - text));
- }
- else
- {
- /* Return some of text plus interp->name. */
- strncpy (match, word, text - word);
- match[text - word] = '\0';
- strcat (match, interp.name);
- }
- tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
+ tracker.add_completion
+ (make_completion_match_str (interp.name, text, word));
}
}
}
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 996d521..bb98619 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4725,29 +4725,8 @@ completion_list_add_name (completion_tracker &tracker,
of matches. Note that the name is moved to freshly malloc'd space. */
{
- char *newobj;
-
- if (word == text)
- {
- newobj = (char *) xmalloc (strlen (symname) + 5);
- strcpy (newobj, symname);
- }
- else if (word > text)
- {
- /* Return some portion of symname. */
- newobj = (char *) xmalloc (strlen (symname) + 5);
- strcpy (newobj, symname + (word - text));
- }
- else
- {
- /* Return some of SYM_TEXT plus symname. */
- newobj = (char *) xmalloc (strlen (symname) + (text - word) + 5);
- strncpy (newobj, word, text - word);
- newobj[text - word] = '\0';
- strcat (newobj, symname);
- }
-
- gdb::unique_xmalloc_ptr<char> completion (newobj);
+ gdb::unique_xmalloc_ptr<char> completion
+ = make_completion_match_str (symname, text, word);
/* Here we pass the match-for-lcd object to add_completion. Some
languages match the user text against substrings of symbol
@@ -5322,30 +5301,7 @@ static void
add_filename_to_list (const char *fname, const char *text, const char *word,
completion_list *list)
{
- char *newobj;
- size_t fnlen = strlen (fname);
-
- if (word == text)
- {
- /* Return exactly fname. */
- newobj = (char *) xmalloc (fnlen + 5);
- strcpy (newobj, fname);
- }
- else if (word > text)
- {
- /* Return some portion of fname. */
- newobj = (char *) xmalloc (fnlen + 5);
- strcpy (newobj, fname + (word - text));
- }
- else
- {
- /* Return some of TEXT plus fname. */
- newobj = (char *) xmalloc (fnlen + (text - word) + 5);
- strncpy (newobj, word, text - word);
- newobj[text - word] = '\0';
- strcat (newobj, fname);
- }
- list->emplace_back (newobj);
+ list->emplace_back (make_completion_match_str (fname, text, word));
}
static int
--
2.5.5
^ permalink raw reply [flat|nested] 6+ messages in thread