[v3,14/19] Implement completion limiting in add_struct_fields.

Message ID 20150806191936.32159.73330.stgit@valrhona.uglyboxes.com
State New, archived
Headers

Commit Message

Keith Seitz Aug. 6, 2015, 7:19 p.m. UTC
  There are no revisions in this version.

---

This patch converts add_struct_fields (used by expression_completer) to use
add_completion and adds (coverage) tests for this new behavior.

break.exp contains two overly greedy regexp patterns which required
tweaking, too.

gdb/ChangeLog

	* completer.c (add_struct_fields): Use add_completion.

gdb/testsuite/ChangeLog

	* gdb.base/break.c (struct field_test): New structure.
	(field_test_global): New global variable.
	* gdb.base/break.exp: Add srcfile to otherwise too greedy regexp
	for tests "check disable with history value" and "check disable
	with convenience values".
	* gdb.base/completion.exp: Add completion limiting tests for
	add_struct_fields using the print command.
	* gdb.cp/cpcompletion.exp (test_completion_limit): New procedure.
	Add completion limiting tests for C++-specific parts of
	add_struct_fields using the print command.
	* gdb.cp/pr9694.cc (class Foo) <repeated1, repeated2, repeated3,
	repeated4, repeated5, repeated6, repeated7, repeated8>: New members.
	(Foo::Foo): Add initializer for new members.
	(main): Add new methods method1, method2, method3, method4, method5,
	method6, method7, and method8 to anonymous structure and make
	sure they are not optimized away.
---
 gdb/completer.c                       |   14 +++++++-
 gdb/testsuite/gdb.base/break.c        |   14 ++++++++
 gdb/testsuite/gdb.base/break.exp      |    4 +-
 gdb/testsuite/gdb.base/completion.exp |    4 ++
 gdb/testsuite/gdb.cp/cpcompletion.exp |   55 +++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/pr9594.cc        |   28 ++++++++++++++++-
 6 files changed, 113 insertions(+), 6 deletions(-)
  

Comments

Doug Evans Aug. 23, 2015, 4:22 a.m. UTC | #1
Keith Seitz <keiths@redhat.com> writes:
> There are no revisions in this version.
>
> ---
>
> This patch converts add_struct_fields (used by expression_completer) to use
> add_completion and adds (coverage) tests for this new behavior.
>
> break.exp contains two overly greedy regexp patterns which required
> tweaking, too.
>
> gdb/ChangeLog
>
> 	* completer.c (add_struct_fields): Use add_completion.
>
> gdb/testsuite/ChangeLog
>
> 	* gdb.base/break.c (struct field_test): New structure.
> 	(field_test_global): New global variable.
> 	* gdb.base/break.exp: Add srcfile to otherwise too greedy regexp
> 	for tests "check disable with history value" and "check disable
> 	with convenience values".
> 	* gdb.base/completion.exp: Add completion limiting tests for
> 	add_struct_fields using the print command.
> 	* gdb.cp/cpcompletion.exp (test_completion_limit): New procedure.
> 	Add completion limiting tests for C++-specific parts of
> 	add_struct_fields using the print command.
> 	* gdb.cp/pr9694.cc (class Foo) <repeated1, repeated2, repeated3,
> 	repeated4, repeated5, repeated6, repeated7, repeated8>: New members.
> 	(Foo::Foo): Add initializer for new members.
> 	(main): Add new methods method1, method2, method3, method4, method5,
> 	method6, method7, and method8 to anonymous structure and make
> 	sure they are not optimized away.

LGTM, but see comment below.

> diff --git a/gdb/completer.c b/gdb/completer.c
> index 7ede0f9..134a3c2 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -373,8 +373,12 @@ add_struct_fields (struct completer_data *cdata,
>  	    {
>  	      if (! strncmp (TYPE_FIELD_NAME (type, i), 
>  			     fieldname, namelen))
> -		VEC_safe_push (char_ptr, *output,
> -			       xstrdup (TYPE_FIELD_NAME (type, i)));
> +		{
> +		  if (add_completion (cdata, output, TYPE_FIELD_NAME (type, i),
> +				      NULL, NULL)
> +		      == ADD_COMPLETION_MAX_REACHED)
> +		    return;
> +		}
>  	    }
>  	  else if (TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_UNION)
>  	    {
> @@ -398,7 +402,11 @@ add_struct_fields (struct completer_data *cdata,
>  	    }
>  	  /* Omit constructors from the completion list.  */
>  	  if (!type_name || strcmp (type_name, name))
> -	    VEC_safe_push (char_ptr, *output, xstrdup (name));
> +	    {
> +	      if (add_completion (cdata, output, name, NULL, NULL)
> +		  == ADD_COMPLETION_MAX_REACHED)
> +		return;
> +	    }
>  	}
>      }
>  }
> diff --git a/gdb/testsuite/gdb.base/break.c b/gdb/testsuite/gdb.base/break.c
> index d017a91..8ee11b1 100644
> --- a/gdb/testsuite/gdb.base/break.c
> +++ b/gdb/testsuite/gdb.base/break.c
> @@ -30,6 +30,20 @@ void *need_malloc ()
>    return malloc (1);
>  }
>  
> +/* A structure for testing add_struct_fields.  */
> +struct field_test
> +{
> +  int a1_field;
> +  int a2_field;
> +  int a3_field;
> +  int a4_field;
> +  int a5_field;
> +  int a6_field;
> +  int a7_field;
> +};
> +
> +struct field_test field_test_global = {1, 2, 3, 4, 5, 6, 7};
> +
>  /*
>   *	This simple classical example of recursion is useful for
>   *	testing stack backtraces and such.
> diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
> index f879bc8..5344533 100644
> --- a/gdb/testsuite/gdb.base/break.exp
> +++ b/gdb/testsuite/gdb.base/break.exp
> @@ -249,7 +249,7 @@ set see5 0
>  set see6 0
>  
>  gdb_test_multiple "info break" "check disable with history values" {
> -    -re "1\[\t \]+breakpoint *keep y.* in main at .*:$main_line\[^\r\n\]*" {
> +    -re "1\[\t \]+breakpoint *keep y.* at .*$srcfile:$main_line\[\r\n\]*" {
>  	set see1 1
>  	exp_continue
>      }
> @@ -295,7 +295,7 @@ set see5 0
>  set see6 0
>  
>  gdb_test_multiple "info break" "check disable with convenience values" {
> -    -re "1\[\t \]+breakpoint *keep y.* in main at .*:$main_line\[^\r\n\]*" {
> +    -re "1\[\t \]+breakpoint *keep y.* in main at .*$srcfile:$main_line\[^\r\n\]*" {
>  	set see1 1
>  	exp_continue
>      }
> diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
> index 6c81505..1036af5 100644
> --- a/gdb/testsuite/gdb.base/completion.exp
> +++ b/gdb/testsuite/gdb.base/completion.exp
> @@ -1054,6 +1054,10 @@ if {$signal_to_use != ""} {
>  	$max_completions
>  }
>  
> +# Test add_struct_fields.
> +test_completion_limit "print field_test_global.a" \
> +    "print field_test_global\\\.a\[1-9\]_field" $max_completions
> +
>  #
>  # Test TUI completions
>  #
> diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp b/gdb/testsuite/gdb.cp/cpcompletion.exp
> index 023c261..a1bc6d8 100644
> --- a/gdb/testsuite/gdb.cp/cpcompletion.exp
> +++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
> @@ -85,3 +85,58 @@ gdb_test "complete p foo1.Fo" "p foo1\\.Foofoo"
>  
>  # Test completion with an anonymous struct.
>  gdb_test "complete p a.g" "p a\\.get"
> +
> +# Test completion limiting (add_struct_fields)
> +set max_completions 3
> +gdb_test_no_output "set max-completions $max_completions"
> +
> +# A convenience function for testing completion limiting.
> +# CMD is a GDB command to to run with "complete".
> +# PATTERN is a regexp pattern matching the expected output
> +#   of completion items "seen" in the output.
> +# NUM is the number of maximum completions expected.
> +#
> +# The test will use the test name "limit complete CMD"
> +# and will only count the number of completion items matching
> +# PATTERN.  No assumptions are made on the order of the items
> +# seen in GDB's output.
> +#
> +# If NUM items are seen before the truncation message, the test
> +# passes, otherwise it fails.  The test can also fail if no
> +# truncation message is seen at all, in which case the test
> +# failure message will say "(unlimited)".
> +
> +proc test_completion_limit {cmd pattern num} {

====
Note: Another version of test_completion_limit.

> +    global gdb_prompt
> +
> +    # The terminal at the end of the complete command
> +    set end "\\\*\\\*\\\* List may be truncated, "
> +    append end "max-completions reached\\\. \\\*\\\*\\\*"
> +
> +    set cmdr [string_to_regexp $cmd]
> +    set seen 0
> +    gdb_test_multiple "complete $cmd" "limit complete $cmd" {
> +	"complete $cmdr" { exp_continue }
> +	-re "$pattern\r\n" {
> +	    incr seen
> +	    exp_continue
> +	}
> +	-re ".*$end\r\n$gdb_prompt $" {
> +	    if {$seen == $num} {
> +		pass "limit complete $cmd"
> +	    } else {
> +		fail "limit complete $cmd ($seen/$num)"
> +	    }
> +	}
> +	-re ".*$gdb_prompt $" {
> +	    fail "limit complete $cmd (unlimited)"
> +	}
> +    }
> +}
> +
> +# Completion limiting tests for add_struct_fields.
> +test_completion_limit "print a.meth" "print a\\\.method\[1-8\]" \
> +    $max_completions
> +
> +test_completion_limit "print foo1.repeat" \
> +    "print foo1\\\.repeated\[1-8\]" $max_completions
> diff --git a/gdb/testsuite/gdb.cp/pr9594.cc b/gdb/testsuite/gdb.cp/pr9594.cc
> index 8fdee84..e4b49ce 100644
> --- a/gdb/testsuite/gdb.cp/pr9594.cc
> +++ b/gdb/testsuite/gdb.cp/pr9594.cc
> @@ -11,9 +11,19 @@ class Foo : public Base
>  
>  private:
>    int foo_value;
> +  int repeated1;
> +  int repeated2;
> +  int repeated3;
> +  int repeated4;
> +  int repeated5;
> +  int repeated6;
> +  int repeated7;
> +  int repeated8;
>  
>  public:
> -  Foo () { foo_value = 0;}
> +  Foo () : foo_value (0), repeated1 (1), repeated2 (2), repeated3 (3),
> +	   repeated4 (4), repeated5 (5), repeated6 (6), repeated7 (7),
> +	   repeated8 (8) {}
>    Foo (int i) { foo_value = i;}
>    ~Foo () { }
>    void set_foo (int value);
> @@ -44,9 +54,25 @@ int main ()
>    // Anonymous struct with method.
>    struct {
>      int get() { return 5; }
> +    int method1 (void) { return 1; }
> +    int method2 (void) { return 2; }
> +    int method3 (void) { return 3; }
> +    int method4 (void) { return 4; }
> +    int method5 (void) { return 5; }
> +    int method6 (void) { return 6; }
> +    int method7 (void) { return 7; }
> +    int method8 (void) { return 8; }
>    } a;
>    Foo foo1;
>    foo1.set_foo (42);		// Set breakpoint here.
>    a.get();			// Prevent compiler from throwing 'a' away.
> +  a.method1 ();
> +  a.method2 ();
> +  a.method3 ();
> +  a.method4 ();
> +  a.method5 ();
> +  a.method6 ();
> +  a.method7 ();
> +  a.method8 ();
>    return 0;
>  }
  

Patch

diff --git a/gdb/completer.c b/gdb/completer.c
index 7ede0f9..134a3c2 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -373,8 +373,12 @@  add_struct_fields (struct completer_data *cdata,
 	    {
 	      if (! strncmp (TYPE_FIELD_NAME (type, i), 
 			     fieldname, namelen))
-		VEC_safe_push (char_ptr, *output,
-			       xstrdup (TYPE_FIELD_NAME (type, i)));
+		{
+		  if (add_completion (cdata, output, TYPE_FIELD_NAME (type, i),
+				      NULL, NULL)
+		      == ADD_COMPLETION_MAX_REACHED)
+		    return;
+		}
 	    }
 	  else if (TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_UNION)
 	    {
@@ -398,7 +402,11 @@  add_struct_fields (struct completer_data *cdata,
 	    }
 	  /* Omit constructors from the completion list.  */
 	  if (!type_name || strcmp (type_name, name))
-	    VEC_safe_push (char_ptr, *output, xstrdup (name));
+	    {
+	      if (add_completion (cdata, output, name, NULL, NULL)
+		  == ADD_COMPLETION_MAX_REACHED)
+		return;
+	    }
 	}
     }
 }
diff --git a/gdb/testsuite/gdb.base/break.c b/gdb/testsuite/gdb.base/break.c
index d017a91..8ee11b1 100644
--- a/gdb/testsuite/gdb.base/break.c
+++ b/gdb/testsuite/gdb.base/break.c
@@ -30,6 +30,20 @@  void *need_malloc ()
   return malloc (1);
 }
 
+/* A structure for testing add_struct_fields.  */
+struct field_test
+{
+  int a1_field;
+  int a2_field;
+  int a3_field;
+  int a4_field;
+  int a5_field;
+  int a6_field;
+  int a7_field;
+};
+
+struct field_test field_test_global = {1, 2, 3, 4, 5, 6, 7};
+
 /*
  *	This simple classical example of recursion is useful for
  *	testing stack backtraces and such.
diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
index f879bc8..5344533 100644
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -249,7 +249,7 @@  set see5 0
 set see6 0
 
 gdb_test_multiple "info break" "check disable with history values" {
-    -re "1\[\t \]+breakpoint *keep y.* in main at .*:$main_line\[^\r\n\]*" {
+    -re "1\[\t \]+breakpoint *keep y.* at .*$srcfile:$main_line\[\r\n\]*" {
 	set see1 1
 	exp_continue
     }
@@ -295,7 +295,7 @@  set see5 0
 set see6 0
 
 gdb_test_multiple "info break" "check disable with convenience values" {
-    -re "1\[\t \]+breakpoint *keep y.* in main at .*:$main_line\[^\r\n\]*" {
+    -re "1\[\t \]+breakpoint *keep y.* in main at .*$srcfile:$main_line\[^\r\n\]*" {
 	set see1 1
 	exp_continue
     }
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 6c81505..1036af5 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -1054,6 +1054,10 @@  if {$signal_to_use != ""} {
 	$max_completions
 }
 
+# Test add_struct_fields.
+test_completion_limit "print field_test_global.a" \
+    "print field_test_global\\\.a\[1-9\]_field" $max_completions
+
 #
 # Test TUI completions
 #
diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp b/gdb/testsuite/gdb.cp/cpcompletion.exp
index 023c261..a1bc6d8 100644
--- a/gdb/testsuite/gdb.cp/cpcompletion.exp
+++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
@@ -85,3 +85,58 @@  gdb_test "complete p foo1.Fo" "p foo1\\.Foofoo"
 
 # Test completion with an anonymous struct.
 gdb_test "complete p a.g" "p a\\.get"
+
+# Test completion limiting (add_struct_fields)
+set max_completions 3
+gdb_test_no_output "set max-completions $max_completions"
+
+# A convenience function for testing completion limiting.
+# CMD is a GDB command to to run with "complete".
+# PATTERN is a regexp pattern matching the expected output
+#   of completion items "seen" in the output.
+# NUM is the number of maximum completions expected.
+#
+# The test will use the test name "limit complete CMD"
+# and will only count the number of completion items matching
+# PATTERN.  No assumptions are made on the order of the items
+# seen in GDB's output.
+#
+# If NUM items are seen before the truncation message, the test
+# passes, otherwise it fails.  The test can also fail if no
+# truncation message is seen at all, in which case the test
+# failure message will say "(unlimited)".
+
+proc test_completion_limit {cmd pattern num} {
+    global gdb_prompt
+
+    # The terminal at the end of the complete command
+    set end "\\\*\\\*\\\* List may be truncated, "
+    append end "max-completions reached\\\. \\\*\\\*\\\*"
+
+    set cmdr [string_to_regexp $cmd]
+    set seen 0
+    gdb_test_multiple "complete $cmd" "limit complete $cmd" {
+	"complete $cmdr" { exp_continue }
+	-re "$pattern\r\n" {
+	    incr seen
+	    exp_continue
+	}
+	-re ".*$end\r\n$gdb_prompt $" {
+	    if {$seen == $num} {
+		pass "limit complete $cmd"
+	    } else {
+		fail "limit complete $cmd ($seen/$num)"
+	    }
+	}
+	-re ".*$gdb_prompt $" {
+	    fail "limit complete $cmd (unlimited)"
+	}
+    }
+}
+
+# Completion limiting tests for add_struct_fields.
+test_completion_limit "print a.meth" "print a\\\.method\[1-8\]" \
+    $max_completions
+
+test_completion_limit "print foo1.repeat" \
+    "print foo1\\\.repeated\[1-8\]" $max_completions
diff --git a/gdb/testsuite/gdb.cp/pr9594.cc b/gdb/testsuite/gdb.cp/pr9594.cc
index 8fdee84..e4b49ce 100644
--- a/gdb/testsuite/gdb.cp/pr9594.cc
+++ b/gdb/testsuite/gdb.cp/pr9594.cc
@@ -11,9 +11,19 @@  class Foo : public Base
 
 private:
   int foo_value;
+  int repeated1;
+  int repeated2;
+  int repeated3;
+  int repeated4;
+  int repeated5;
+  int repeated6;
+  int repeated7;
+  int repeated8;
 
 public:
-  Foo () { foo_value = 0;}
+  Foo () : foo_value (0), repeated1 (1), repeated2 (2), repeated3 (3),
+	   repeated4 (4), repeated5 (5), repeated6 (6), repeated7 (7),
+	   repeated8 (8) {}
   Foo (int i) { foo_value = i;}
   ~Foo () { }
   void set_foo (int value);
@@ -44,9 +54,25 @@  int main ()
   // Anonymous struct with method.
   struct {
     int get() { return 5; }
+    int method1 (void) { return 1; }
+    int method2 (void) { return 2; }
+    int method3 (void) { return 3; }
+    int method4 (void) { return 4; }
+    int method5 (void) { return 5; }
+    int method6 (void) { return 6; }
+    int method7 (void) { return 7; }
+    int method8 (void) { return 8; }
   } a;
   Foo foo1;
   foo1.set_foo (42);		// Set breakpoint here.
   a.get();			// Prevent compiler from throwing 'a' away.
+  a.method1 ();
+  a.method2 ();
+  a.method3 ();
+  a.method4 ();
+  a.method5 ();
+  a.method6 ();
+  a.method7 ();
+  a.method8 ();
   return 0;
 }