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

Doug Evans
2015-08-23T04:23:43Z

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