public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [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 3/3] Tighten regexp of lib/completion-support.exp:test_gdb_complete_tab_multiple Pedro Alves
@ 2017-12-13 13:26 ` Pedro Alves
  2017-12-13 13:26 ` [PATCH 2/3] Fix regression: expression completer and scope operator (PR gdb/22584) Pedro Alves
  2017-12-13 15:58 ` [PATCH 0/3] " 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

* [PATCH 0/3] Fix regression: expression completer and scope operator (PR gdb/22584)
@ 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
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Pedro Alves @ 2017-12-13 13:26 UTC (permalink / raw)
  To: gdb-patches

This patch series fixes PR gdb/22584, a regression in the expression
completer:

 "(gdb) p std::[TAB]" => "(gdb) p std::std::"

GDB got confused above, there's no such thing as "std::std::".

Patch #1 factors out some code duplicated throughout the codebase.

The new function patched by patch #1 is used by patch #2 (the actual fix) uses.

Patch #3 fixes something in the new completion testing support
routines that I noticed while writing the tests for patch #2.

Pedro Alves (3):
  Factor out final completion match string building
  Fix regression: expression completer and scope operator (PR gdb/22584)
  Tighten regexp of
    lib/completion-support.exp:test_gdb_complete_tab_multiple

 gdb/cli/cli-decode.c                     |  41 +----------
 gdb/completer.c                          | 115 +++++++++++++++++++++----------
 gdb/completer.h                          |  24 ++++++-
 gdb/interps.c                            |  20 +-----
 gdb/symtab.c                             |  52 ++------------
 gdb/testsuite/gdb.cp/cpcompletion.exp    |  42 +++++++++++
 gdb/testsuite/gdb.cp/pr9594.cc           |  13 ++++
 gdb/testsuite/lib/completion-support.exp |   7 +-
 8 files changed, 169 insertions(+), 145 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [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 ` [PATCH 3/3] Tighten regexp of lib/completion-support.exp:test_gdb_complete_tab_multiple Pedro Alves
  2017-12-13 13:26 ` [PATCH 1/3] Factor out final completion match string building Pedro Alves
@ 2017-12-13 13:26 ` Pedro Alves
  2017-12-13 15:58 ` [PATCH 0/3] " Simon Marchi
  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 3/3] Tighten regexp of lib/completion-support.exp:test_gdb_complete_tab_multiple
  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

While writing the tests included in the previous commit, I noticed
that test_gdb_complete_tab_multiple would not FAIL if GDB happens to
show more completions than expected before the expected list.

E.g., with something like this, expecting "p foo" to complete to
"foo2" and "foo3":

 test_gdb_complete_tab_multiple "p foo" "" {
	"foo2"
	"foo3"
 }

and then if foo actually completes to:

 (gdb) p foo[TAB]
 foo1   foo2  foo3
 ^^^^

we'd still PASS.  (Note the spurious "foo1" above.)

This tightens the regexp with a beginning anchor thus making the
completions above cause a FAIL.  Other similar functions in
completion-support.exp already do something like this; I had just
missed this one originally.  Thankfully, this did not expose any
problems in the gdb.linespec/ tests.  Phew.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* lib/completion-support.exp (test_gdb_complete_tab_multiple):
	Tighten regexp by matching with an anchor.
---
 gdb/testsuite/lib/completion-support.exp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp
index fe5b16a..bebf90e 100644
--- a/gdb/testsuite/lib/completion-support.exp
+++ b/gdb/testsuite/lib/completion-support.exp
@@ -139,9 +139,12 @@ proc test_gdb_complete_tab_multiple { input_line add_completed_line \
 	    # extra tab to show the matches list.
 	    if {$add_completed_line != ""} {
 		send_gdb "\t"
+		set maybe_bell ${completion::bell_re}
+	    } else {
+		set maybe_bell ""
 	    }
 	    gdb_test_multiple "" "$test (second tab)" {
-		-re "$expected_re\r\n$gdb_prompt $input_line_re$add_completed_line_re$" {
+		-re "^${maybe_bell}\r\n$expected_re\r\n$gdb_prompt $input_line_re$add_completed_line_re$" {
 		    pass "$test"
 		}
 	    }
-- 
2.5.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/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
                   ` (2 preceding siblings ...)
  2017-12-13 13:26 ` [PATCH 2/3] Fix regression: expression completer and scope operator (PR gdb/22584) Pedro Alves
@ 2017-12-13 15:58 ` Simon Marchi
  2017-12-13 16:41   ` Pedro Alves
  3 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2017-12-13 15:58 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 2017-12-13 08:26 AM, Pedro Alves wrote:
> This patch series fixes PR gdb/22584, a regression in the expression
> completer:
> 
>  "(gdb) p std::[TAB]" => "(gdb) p std::std::"
> 
> GDB got confused above, there's no such thing as "std::std::".
> 
> Patch #1 factors out some code duplicated throughout the codebase.
> 
> The new function patched by patch #1 is used by patch #2 (the actual fix) uses.
> 
> Patch #3 fixes something in the new completion testing support
> routines that I noticed while writing the tests for patch #2.
> 
> Pedro Alves (3):
>   Factor out final completion match string building
>   Fix regression: expression completer and scope operator (PR gdb/22584)
>   Tighten regexp of
>     lib/completion-support.exp:test_gdb_complete_tab_multiple
> 
>  gdb/cli/cli-decode.c                     |  41 +----------
>  gdb/completer.c                          | 115 +++++++++++++++++++++----------
>  gdb/completer.h                          |  24 ++++++-
>  gdb/interps.c                            |  20 +-----
>  gdb/symtab.c                             |  52 ++------------
>  gdb/testsuite/gdb.cp/cpcompletion.exp    |  42 +++++++++++
>  gdb/testsuite/gdb.cp/pr9594.cc           |  13 ++++
>  gdb/testsuite/lib/completion-support.exp |   7 +-
>  8 files changed, 169 insertions(+), 145 deletions(-)

That series looks good to me.

Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] Fix regression: expression completer and scope operator (PR gdb/22584)
  2017-12-13 15:58 ` [PATCH 0/3] " Simon Marchi
@ 2017-12-13 16:41   ` Pedro Alves
  0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2017-12-13 16:41 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 12/13/2017 03:57 PM, Simon Marchi wrote:
> On 2017-12-13 08:26 AM, Pedro Alves wrote:
>> This patch series fixes PR gdb/22584, a regression in the expression
>> completer:
>>
>>  "(gdb) p std::[TAB]" => "(gdb) p std::std::"
>>
>> GDB got confused above, there's no such thing as "std::std::".
>>
>> Patch #1 factors out some code duplicated throughout the codebase.
>>
>> The new function patched by patch #1 is used by patch #2 (the actual fix) uses.
>>
>> Patch #3 fixes something in the new completion testing support
>> routines that I noticed while writing the tests for patch #2.
>>
>> Pedro Alves (3):
>>   Factor out final completion match string building
>>   Fix regression: expression completer and scope operator (PR gdb/22584)
>>   Tighten regexp of
>>     lib/completion-support.exp:test_gdb_complete_tab_multiple
>>
>>  gdb/cli/cli-decode.c                     |  41 +----------
>>  gdb/completer.c                          | 115 +++++++++++++++++++++----------
>>  gdb/completer.h                          |  24 ++++++-
>>  gdb/interps.c                            |  20 +-----
>>  gdb/symtab.c                             |  52 ++------------
>>  gdb/testsuite/gdb.cp/cpcompletion.exp    |  42 +++++++++++
>>  gdb/testsuite/gdb.cp/pr9594.cc           |  13 ++++
>>  gdb/testsuite/lib/completion-support.exp |   7 +-
>>  8 files changed, 169 insertions(+), 145 deletions(-)
> 
> That series looks good to me.

Thanks, pushed.

Pedro Alves

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-13 16:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 3/3] Tighten regexp of lib/completion-support.exp:test_gdb_complete_tab_multiple Pedro Alves
2017-12-13 13:26 ` [PATCH 1/3] Factor out final completion match string building Pedro Alves
2017-12-13 13:26 ` [PATCH 2/3] Fix regression: expression completer and scope operator (PR gdb/22584) Pedro Alves
2017-12-13 15:58 ` [PATCH 0/3] " Simon Marchi
2017-12-13 16:41   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).