From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66535 invoked by alias); 23 Aug 2015 03:47:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 64750 invoked by uid 89); 23 Aug 2015 03:47:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.1 required=5.0 tests=AWL,BAYES_40,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,KAM_STOCKGEN,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pa0-f41.google.com Received: from mail-pa0-f41.google.com (HELO mail-pa0-f41.google.com) (209.85.220.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sun, 23 Aug 2015 03:47:07 +0000 Received: by pacgr6 with SMTP id gr6so4761865pac.0 for ; Sat, 22 Aug 2015 20:47:05 -0700 (PDT) X-Received: by 10.66.175.7 with SMTP id bw7mr33291578pac.155.1440301625228; Sat, 22 Aug 2015 20:47:05 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by smtp.gmail.com with ESMTPSA id oe10sm12649823pbc.11.2015.08.22.20.47.04 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 22 Aug 2015 20:47:04 -0700 (PDT) From: Doug Evans To: Keith Seitz , brobecker@adacore.com Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 05/19] Implement completion limiting for ada_make_symbol_completion_list. References: <20150806191404.32159.50755.stgit@valrhona.uglyboxes.com> <20150806191601.32159.610.stgit@valrhona.uglyboxes.com> Date: Sun, 23 Aug 2015 03:47:00 -0000 In-Reply-To: <20150806191601.32159.610.stgit@valrhona.uglyboxes.com> (Keith Seitz's message of "Thu, 06 Aug 2015 12:16:21 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00644.txt.bz2 Keith Seitz 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 " + [multi_line "p " \ > + "p " \ > + "p " \ > + "p " ] > + > +# Complete on repeated mangled name, using '<...>' notation: > +test_gdb_complete " + [multi_line "p " \ > + "p " \ > + "p " \ > + "p " ] > > # Complete a mangled symbol name, but using the '<...>' notation. > test_gdb_complete " "p " > > +# 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 " + [limit_multi_line $max_completions \ > + "p " \ > + "p " \ > + "p " \ > + "p " ] > + > + # Complete on repeated mangled name, using '<...>' notation: > + test_gdb_complete " + [limit_multi_line $max_completions \ > + "p " \ > + "p " \ > + "p " \ > + "p " ] > +} > + > +# 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;