Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Doug Evans <xdje42@gmail.com>
To: Keith Seitz <keiths@redhat.com>, brobecker@adacore.com
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v3 05/19] Implement completion limiting for ada_make_symbol_completion_list.
Date: Sun, 23 Aug 2015 03:47:00 -0000	[thread overview]
Message-ID: <m3bndyis5e.fsf@sspiff.org> (raw)
In-Reply-To: <20150806191601.32159.610.stgit@valrhona.uglyboxes.com> (Keith	Seitz's message of "Thu, 06 Aug 2015 12:16:21 -0700")

Keith Seitz <keiths@redhat.com> writes:
> Differences in this revision:
>
> 1. Remove partial copy code from symbol_completion_add.
>
> ---
>
> This patch converts the one Ada completion(-related) function,
> symbol_completion_add, to use maybe_add_completion, and tests have
> been added to exercise this newly implemented behavior.
>
> gdb/ChangeLog
>
> 	* ada-lang.c (symbol_completion_add): Return
> 	add_completion_status instead of void.
> 	Use add_completion and return the status of this function.
> 	(ada_make_symbol_completion_list): If symbol_completion_add
> 	returns that maximum completions have been reached, stop
> 	looking for completions and return the list.
>
> gdb/testsuite/ChangeLog
>
> 	* gdb.ada/complete.exp (limit_multi_line): New procedure.
> 	Update existing tests for source changes.
> 	Add additional tests for new types.
> 	Add tests for completion limiting.
> 	* gdb.ada/complete/foo.adb (Repeat_Variable_1, Repeat_Variable_2,
> 	Repeat_Variable_3, Repeat_Variable_4): Define.
> 	* gdb.ada/complete/pck.ads (Repeat_Variable_1, Repeat_Variable_2)
> 	(Repeat_Variable_3, Repeat_Variable_4): Declare.
> 	(Repeated_1, Repeated_2, Repeated_3, Repeated_4): Define.

LGTM.
I didn't study the ada tests too closely,
but they seemed ok.
Adding Joel to the To list as a "heads up".

> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 9782b69..5df08be 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -6198,8 +6198,9 @@ symbol_completion_match (const char *sym_name,
>     encoded formed (in which case the completion should also be
>     encoded).  */
>  
> -static void
> +static enum add_completion_status
>  symbol_completion_add (VEC(char_ptr) **sv,
> +		       struct completer_data *cdata,
>                         const char *sym_name,
>                         const char *text, int text_len,
>                         const char *orig_text, const char *word,
> @@ -6207,35 +6208,12 @@ symbol_completion_add (VEC(char_ptr) **sv,
>  {
>    const char *match = symbol_completion_match (sym_name, text, text_len,
>                                                 wild_match_p, encoded_p);
> -  char *completion;
> -
>    if (match == NULL)
> -    return;
> +    return ADD_COMPLETION_OK;
>  
>    /* We found a match, so add the appropriate completion to the given
>       string vector.  */
> -
> -  if (word == orig_text)
> -    {
> -      completion = xmalloc (strlen (match) + 5);
> -      strcpy (completion, match);
> -    }
> -  else if (word > orig_text)
> -    {
> -      /* Return some portion of sym_name.  */
> -      completion = xmalloc (strlen (match) + 5);
> -      strcpy (completion, match + (word - orig_text));
> -    }
> -  else
> -    {
> -      /* Return some of ORIG_TEXT plus sym_name.  */
> -      completion = xmalloc (strlen (match) + (orig_text - word) + 5);
> -      strncpy (completion, word, orig_text - word);
> -      completion[orig_text - word] = '\0';
> -      strcat (completion, match);
> -    }
> -
> -  VEC_safe_push (char_ptr, *sv, completion);
> +  return add_completion (cdata, sv, match, orig_text, word);
>  }
>  
>  /* An object of this type is passed as the user_data argument to the
> @@ -6283,6 +6261,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
>    int i;
>    struct block_iterator iter;
>    struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
> +  enum add_completion_status status;
>  
>    gdb_assert (code == TYPE_CODE_UNDEF);
>  
> @@ -6333,9 +6312,15 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
>    ALL_MSYMBOLS (objfile, msymbol)
>    {
>      QUIT;
> -    symbol_completion_add (&completions, MSYMBOL_LINKAGE_NAME (msymbol),
> -			   text, text_len, text0, word, wild_match_p,
> -			   encoded_p);
> +    status = symbol_completion_add (&completions, cdata,
> +					 MSYMBOL_LINKAGE_NAME (msymbol),
> +					 text, text_len, text0, word,
> +					 wild_match_p, encoded_p);
> +    if (status == ADD_COMPLETION_MAX_REACHED)
> +      {
> +	do_cleanups (old_chain);
> +	return completions;
> +      }
>    }
>  
>    /* Search upwards from currently selected frame (so that we can
> @@ -6348,9 +6333,15 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
>  
>        ALL_BLOCK_SYMBOLS (b, iter, sym)
>        {
> -        symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
> -                               text, text_len, text0, word,
> -                               wild_match_p, encoded_p);
> +        status = symbol_completion_add (&completions, cdata,
> +					     SYMBOL_LINKAGE_NAME (sym),
> +					     text, text_len, text0, word,
> +					     wild_match_p, encoded_p);
> +	if (status == ADD_COMPLETION_MAX_REACHED)
> +	  {
> +	    do_cleanups (old_chain);
> +	    return completions;
> +	  }
>        }
>      }
>  
> @@ -6363,9 +6354,15 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
>      b = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (s), GLOBAL_BLOCK);
>      ALL_BLOCK_SYMBOLS (b, iter, sym)
>      {
> -      symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
> -                             text, text_len, text0, word,
> -                             wild_match_p, encoded_p);
> +      status = symbol_completion_add (&completions, cdata,
> +					   SYMBOL_LINKAGE_NAME (sym),
> +					   text, text_len, text0, word,
> +					   wild_match_p, encoded_p);
> +      if (status == ADD_COMPLETION_MAX_REACHED)
> +	{
> +	  do_cleanups (old_chain);
> +	  return completions;
> +	}
>      }
>    }
>  
> @@ -6378,9 +6375,15 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
>        continue;
>      ALL_BLOCK_SYMBOLS (b, iter, sym)
>      {
> -      symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
> -                             text, text_len, text0, word,
> -                             wild_match_p, encoded_p);
> +      status = symbol_completion_add (&completions, cdata,
> +					   SYMBOL_LINKAGE_NAME (sym),
> +					   text, text_len, text0, word,
> +					   wild_match_p, encoded_p);
> +      if (status == ADD_COMPLETION_MAX_REACHED)
> +	{
> +	  do_cleanups (old_chain);
> +	  return completions;
> +	}
>      }
>    }
>  
> diff --git a/gdb/testsuite/gdb.ada/complete.exp b/gdb/testsuite/gdb.ada/complete.exp
> index 9919bdf..b4efc68 100644
> --- a/gdb/testsuite/gdb.ada/complete.exp
> +++ b/gdb/testsuite/gdb.ada/complete.exp
> @@ -44,6 +44,29 @@ proc test_gdb_no_completion { expr } {
>      gdb_test_no_output "complete p $expr"
>  }
>  
> +# A convenience function that joins all the arguments together,
> +# with a regexp that matches zero-or-more end of lines in between
> +# each argument.  This function is ideal to write the expected output
> +# of a GDB command that generates more than a couple of lines, as
> +# this allows us to write each line as a separate string, which is
> +# easier to read by a human being.
> +
> +proc multi_line { args } {
> +    return [join $args "\[\r\n\]*"]
> +}
> +
> +# Like multi_line above, but limiting the return result to MAX
> +# elements and adding the completer's truncation message.
> +
> +proc limit_multi_line { max args } {
> +    set result [join [lrange $args 0 [expr {$max - 1}]] "\[\r\n\]*"]
> +    if {$max <= [llength $args]} {
> +	append result ".*\\\*\\\*\\\* List may be truncated, "
> +	append result "max-completions reached\\\. \\\*\\\*\\\*"
> +    }
> +    return $result
> +}
> +
>  # Try a global variable, only one match should be found:
>  
>  test_gdb_complete "my_glob" \
> @@ -145,7 +168,11 @@ test_gdb_complete "pck" \
>                                "p pck.local_identical_one" \
>                                "p pck.local_identical_two" \
>                                "p pck.my_global_variable" \
> -                              "p pck.proc" ]
> +                              "p pck.proc" \
> +                              "p pck.repeat_variable_1" \
> +                              "p pck.repeat_variable_2" \
> +                              "p pck.repeat_variable_3" \
> +                              "p pck.repeat_variable_4" ]
>  
>  # Complete on the name of a package followed by a dot:
>  test_gdb_complete "pck." \
> @@ -156,12 +183,125 @@ test_gdb_complete "pck." \
>                                "p pck.local_identical_one" \
>                                "p pck.local_identical_two" \
>                                "p pck.my_global_variable" \
> -                              "p pck.proc" ]
> +                              "p pck.proc" \
> +                              "p pck.repeat_variable_1" \
> +                              "p pck.repeat_variable_2" \
> +                              "p pck.repeat_variable_3" \
> +                              "p pck.repeat_variable_4" ]
> +
> +# Complete on a repeated global name:
> +test_gdb_complete "repeat_" \
> +                 [multi_line "p repeat_variable_1" \
> +                             "p repeat_variable_2" \
> +                             "p repeat_variable_3" \
> +                             "p repeat_variable_4" ]
> +
> +# Complete a mangled symbol name, but using the '<...>' notation:
> +test_gdb_complete "<pck__repeat_" \
> +                  [multi_line "p <pck__repeat_variable_1>" \
> +                              "p <pck__repeat_variable_2>" \
> +                              "p <pck__repeat_variable_3>" \
> +                              "p <pck__repeat_variable_4>" ]
> +
> +# Complete on repeated mangled name, using '<...>' notation:
> +test_gdb_complete "<Repeated_" \
> +                  [multi_line "p <Repeated_1>" \
> +                              "p <Repeated_2>" \
> +                              "p <Repeated_3>" \
> +                              "p <Repeated_4>" ]
>  
>  # Complete a mangled symbol name, but using the '<...>' notation.
>  test_gdb_complete "<pck__my" \
>                    "p <pck__my_global_variable>"
>  
> +# Test completion limiting.
> +set max_completions 2
> +gdb_test_no_output "set max-completions $max_completions"
> +with_test_prefix "limit completion" {
> +    # Two matches, from the global scope:
> +    test_gdb_complete "local_ident" \
> +	[limit_multi_line $max_completions \
> +	     "p local_identical_one" \
> +	     "p local_identical_two" ]
> +
> +    # Two matches, from the global scope, but using fully qualified names:
> +    test_gdb_complete "pck.local_ident" \
> +	[limit_multi_line $max_completions "p pck.local_identical_one" \
> +	     "p pck.local_identical_two" ]
> +
> +    # Two matches, from the global scope, but using mangled fully qualified
> +    # names:
> +    test_gdb_complete "pck__local_ident" \
> +	[limit_multi_line $max_completions \
> +	     "p pck__local_identical_one" \
> +	     "p pck__local_identical_two" ]
> +
> +    # Two matches, one from the global scope, the other from the local
> +    # scope:
> +    test_gdb_complete "external_ident" \
> +	[limit_multi_line $max_completions \
> +	     "p external_identical_one" \
> +	     "p external_identical_two" ]
> +
> +    # Complete on the name of package.
> +    test_gdb_complete "pck" \
> +	[limit_multi_line $max_completions \
> +	     "(p pck\\.ad\[sb\])?" \
> +	     "(p pck\\.ad\[sb\])?" \
> +	     "p pck.external_identical_one" \
> +	     "p pck.inner.inside_variable" \
> +	     "p pck.local_identical_one" \
> +	     "p pck.local_identical_two" \
> +	     "p pck.my_global_variable" \
> +	     "p pck.proc" \
> +	     "p pck.repeat_variable_1" \
> +	     "p pck.repeat_variable_2" \
> +	     "p pck.repeat_variable_3" \
> +	     "p pck.repeat_variable_4" ]
> +
> +    # Complete on the name of a package followed by a dot:
> +    test_gdb_complete "pck." \
> +	[limit_multi_line $max_completions \
> +	     "(p pck\\.ad\[sb\])?" \
> +	     "(p pck\\.ad\[sb\])?" \
> +	     "p pck.external_identical_one" \
> +	     "p pck.inner.inside_variable" \
> +	     "p pck.local_identical_one" \
> +	     "p pck.local_identical_two" \
> +	     "p pck.my_global_variable" \
> +	     "p pck.proc" \
> +	     "p pck.repeat_variable_1" \
> +	     "p pck.repeat_variable_2" \
> +	     "p pck.repeat_variable_3" \
> +	     "p pck.repeat_variable_4"]
> +
> +    # Complete on a repeated global name:
> +    test_gdb_complete "repeat_" \
> +	[limit_multi_line $max_completions \
> +	     "p repeat_variable_1" \
> +	     "p repeat_variable_2" \
> +	     "p repeat_variable_3" \
> +	     "p repeat_variable_4" ]
> +    # Complete a mangled symbol name, but using the '<...>' notation:
> +    test_gdb_complete "<pck__repeat_" \
> +	[limit_multi_line $max_completions \
> +	     "p <pck__repeat_variable_1>" \
> +	     "p <pck__repeat_variable_2>" \
> +	     "p <pck__repeat_variable_3>" \
> +	     "p <pck__repeat_variable_4>" ]
> +
> +    # Complete on repeated mangled name, using '<...>' notation:
> +    test_gdb_complete "<Repeated_" \
> +	[limit_multi_line $max_completions \
> +	     "p <Repeated_1>" \
> +	     "p <Repeated_2>" \
> +	     "p <Repeated_3>" \
> +	     "p <Repeated_4>" ]
> +}
> +
> +# Reset completion-limiting to its default.
> +gdb_test_no_output "set max-completions 200"
> +
>  # Very simple completion, but using the interactive form, this time.
>  # The verification we are trying to make involves the event loop,
>  # and using the "complete" command is not sufficient to reproduce
> diff --git a/gdb/testsuite/gdb.ada/complete/foo.adb b/gdb/testsuite/gdb.ada/complete/foo.adb
> index 58d0ee3..ad6b5ec 100644
> --- a/gdb/testsuite/gdb.ada/complete/foo.adb
> +++ b/gdb/testsuite/gdb.ada/complete/foo.adb
> @@ -20,6 +20,10 @@ procedure Foo is
>     External_Identical_Two : Integer := 74;
>  begin
>     My_Global_Variable := Some_Local_Variable + 1; -- START
> +   Repeat_Variable_1 := My_Global_Variable + 1;
> +   Repeat_Variable_2 := Repeat_Variable_1 + 1;
> +   Repeat_Variable_3 := Repeat_Variable_2 + 1;
> +   Repeat_Variable_4 := Repeat_Variable_3 + 1;
>     Proc (External_Identical_Two);
>  end Foo;
>  
> diff --git a/gdb/testsuite/gdb.ada/complete/pck.ads b/gdb/testsuite/gdb.ada/complete/pck.ads
> index ab2c47b..5595f7f 100644
> --- a/gdb/testsuite/gdb.ada/complete/pck.ads
> +++ b/gdb/testsuite/gdb.ada/complete/pck.ads
> @@ -16,9 +16,21 @@
>  package Pck is
>  
>     My_Global_Variable : Integer := 1;
> +   Repeat_Variable_1 : Integer := 1;
> +   Repeat_Variable_2 : Integer := 1;
> +   Repeat_Variable_3 : Integer := 1;
> +   Repeat_Variable_4 : Integer := 1;
>  
>     Exported_Capitalized : Integer := 2;
>     pragma Export (C, Exported_Capitalized, "Exported_Capitalized");
> +   Repeated_1 : Integer := 21;
> +   pragma Export (C, Repeated_1, "Repeated_1");
> +   Repeated_2 : Integer := 22;
> +   pragma Export (C, Repeated_2, "Repeated_2");
> +   Repeated_3 : Integer := 23;
> +   pragma Export (C, Repeated_3, "Repeated_3");
> +   Repeated_4 : Integer := 24;
> +   pragma Export (C, Repeated_4, "Repeated_4");
>  
>     Local_Identical_One : Integer := 4;
>     Local_Identical_Two : Integer := 8;


  reply	other threads:[~2015-08-23  3:47 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-07  2:37 [PATCH v3 00/19] New completer API Keith Seitz
2015-08-06 19:18 ` [PATCH v3 09/19] Implement completion limiting for interpreter_completer Keith Seitz
2015-08-23  4:03   ` Doug Evans
2015-08-06 19:20 ` [PATCH v3 12/19] Implement completion limiting for sim_command_completer Keith Seitz
2015-08-23  4:11   ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 19/19] Remove the vector return result from the completion API Keith Seitz
2015-08-23 18:03   ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 02/19] Remove completion_tracker_t from the public " Keith Seitz
2015-08-23  1:02   ` Doug Evans
2015-08-24 16:06     ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 04/19] Implement completion limiting for add_filename_to_list Keith Seitz
2015-08-23  1:07   ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 06/19] Implement completion limiting for condition_completer Keith Seitz
2015-08-23  3:53   ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 18/19] Use the hashtable to accumulate completion results Keith Seitz
2015-08-23 17:53   ` Doug Evans
2015-08-06 20:03 ` [PATCH v3 01/19] Add struct completer_data to the completion API Keith Seitz
2015-08-23  0:29   ` Doug Evans
2015-08-06 21:06 ` [PATCH v3 05/19] Implement completion limiting for ada_make_symbol_completion_list Keith Seitz
2015-08-23  3:47   ` Doug Evans [this message]
2015-08-06 21:06 ` [PATCH v3 17/19] Make the completion API completely opaque Keith Seitz
2015-08-23 15:14   ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 11/19] Implement completion limiting for reg_or_group_completer Keith Seitz
2015-08-23  4:09   ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 07/19] Implement completion limiting for filename_completer Keith Seitz
2015-08-23  3:58   ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 16/19] Implement completion limiting for tui_reggroup_completer Keith Seitz
2015-08-23  4:25   ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 08/19] Implement completion limiting for signal_completer Keith Seitz
2015-08-23  3:59   ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 13/19] Implement completion limiting for complete_on_enum Keith Seitz
2015-08-23  4:19   ` Doug Evans
2015-08-06 22:12 ` [PATCH v3 14/19] Implement completion limiting in add_struct_fields Keith Seitz
2015-08-23  4:23   ` Doug Evans
2015-08-06 22:12 ` [PATCH v3 10/19] Implement completion limiting for cmdpy_completer Keith Seitz
2015-08-23  4:07   ` Doug Evans
2015-08-06 22:36 ` [PATCH v3 03/19] Implement completion-limiting for complete_on_cmdlist Keith Seitz
2015-08-23  1:05   ` Doug Evans
2015-08-07  2:37 ` [PATCH v3 15/19] Implement completion limiting for scmcmd_add_completion Keith Seitz
2015-08-23  4:24   ` Doug Evans
2015-08-07 22:57 ` [PATCH v3 00/19] New completer API Andrew Burgess
2015-08-08  0:04   ` Keith Seitz
2015-08-08  6:44     ` Andrew Burgess
2015-08-08 16:25       ` Keith Seitz
2015-08-22 22:25         ` Doug Evans

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m3bndyis5e.fsf@sspiff.org \
    --to=xdje42@gmail.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=keiths@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox