Re: [PATCH v3 14/19] Implement completion limiting in add_struct_fields.

[Doug Evans <xdje42 at gmail dot com>]

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;
>  }