[33/40] Make the linespec/location completer ignore data symbols

Message ID 1496406158-12663-34-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves June 2, 2017, 12:22 p.m. UTC
  Currently "b foo[TAB]" offers data symbols as completion candidates.
This doesn't make sense, since you can't set a breakpoint on data
symbols, only on code symbols.

 (gdb) b globa[TAB]
 (gdb) b global [ENTER]
 Function "global" not defined.
 Make breakpoint pending on future shared library load? (y or [n]) n
 (gdb) info symbol global
 global in section .rodata

So this patch makes linespec completion ignore data symbols.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* ada-lang.c (ada_make_symbol_completion_list): Use
	completion_skip_symbol.
	* symtab.c (symbol_is_function_or_method(minimal_symbol*)): New.
	(symbol_is_function_or_method(symbol*)): New.
	(add_symtab_completions): Add complete_symbol_mode parameter.  Use
	completion_skip_symbol.
	(default_collect_symbol_completion_matches_break_on): Use
	completion_skip_symbol.  Pass down mode.
	(collect_file_symbol_completion_matches): Pass down mode.
	* symtab.h (symbol_is_function_or_method): New declarations.
	(completion_skip_symbol): New template function.
---
 gdb/ada-lang.c | 12 ++++++++++++
 gdb/symtab.c   | 45 ++++++++++++++++++++++++++++++++++++++++++---
 gdb/symtab.h   | 20 ++++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)
  

Comments

Keith Seitz Aug. 9, 2017, 3:42 p.m. UTC | #1
On 06/02/2017 05:22 AM, Pedro Alves wrote:
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index bbc317d..52cf70a 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -5034,11 +5034,44 @@ completion_list_add_fields (completion_tracker &tracker,
>      }
>  }
>  
> +/* See symtab.h.  */
> +
> +bool
> +symbol_is_function_or_method (symbol *sym)
> +{
> +  switch (TYPE_CODE (SYMBOL_TYPE (sym)))
> +    {
> +    case TYPE_CODE_FUNC:
> +    case TYPE_CODE_METHOD:
> +      return true;
> +    default:
> +      return false;
> +    }
> +}
> +
> +/* Return whether MSYMBOL is a function/method.  */
> +

I think "See symtab.h." is more customary here?

> +bool
> +symbol_is_function_or_method (minimal_symbol *msymbol)
> +{
> +  switch (MSYMBOL_TYPE (msymbol))
> +    {
> +    case mst_text:
> +    case mst_text_gnu_ifunc:
> +    case mst_solib_trampoline:
> +    case mst_file_text:
> +      return true;
> +    default:
> +      return false;
> +    }
> +}
> +
>  /* Add matching symbols from SYMTAB to the current completion list.  */
>  
>  static void
>  add_symtab_completions (struct compunit_symtab *cust,
>  			completion_tracker &tracker,
> +			complete_symbol_mode mode,
>  			const lookup_name_info &lookup_name,
>  			const char *text, const char *word,
>  			enum type_code code)

Otherwise, LGTM.

Keith
  
Pedro Alves Nov. 8, 2017, 4:22 p.m. UTC | #2
On 08/09/2017 04:42 PM, Keith Seitz wrote:
> On 06/02/2017 05:22 AM, Pedro Alves wrote:
>>
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index bbc317d..52cf70a 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>> @@ -5034,11 +5034,44 @@ completion_list_add_fields (completion_tracker &tracker,
>>      }
>>  }
>>  
>> +/* See symtab.h.  */
>> +
>> +bool
>> +symbol_is_function_or_method (symbol *sym)
>> +{
>> +  switch (TYPE_CODE (SYMBOL_TYPE (sym)))
>> +    {
>> +    case TYPE_CODE_FUNC:
>> +    case TYPE_CODE_METHOD:
>> +      return true;
>> +    default:
>> +      return false;
>> +    }
>> +}
>> +
>> +/* Return whether MSYMBOL is a function/method.  */
>> +
> 
> I think "See symtab.h." is more customary here?

Indeed, fixed.

> 
>> +bool
>> +symbol_is_function_or_method (minimal_symbol *msymbol)
>> +{
>> +  switch (MSYMBOL_TYPE (msymbol))
>> +    {
>> +    case mst_text:
>> +    case mst_text_gnu_ifunc:
>> +    case mst_solib_trampoline:
>> +    case mst_file_text:
>> +      return true;
>> +    default:
>> +      return false;
>> +    }
>> +}
>> +
>>  /* Add matching symbols from SYMTAB to the current completion list.  */
>>  
>>  static void
>>  add_symtab_completions (struct compunit_symtab *cust,
>>  			completion_tracker &tracker,
>> +			complete_symbol_mode mode,
>>  			const lookup_name_info &lookup_name,
>>  			const char *text, const char *word,
>>  			enum type_code code)
> 
> Otherwise, LGTM.

I pushed this one in too, even though I haven't pushed patches
#31 nor #32 yet.  (Because I haven't yet addressed your reviews).
(I don't really recall why I originally ordered this one after.
There seems to be no good reason.)

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 868ee35..c53f84b 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -6463,6 +6463,9 @@  ada_collect_symbol_completion_matches (completion_tracker &tracker,
   {
     QUIT;
 
+    if (completion_skip_symbol (mode, msymbol))
+      continue;
+
     completion_list_add_name (tracker,
 			      MSYMBOL_LANGUAGE (msymbol),
 			      MSYMBOL_LINKAGE_NAME (msymbol),
@@ -6479,6 +6482,9 @@  ada_collect_symbol_completion_matches (completion_tracker &tracker,
 
       ALL_BLOCK_SYMBOLS (b, iter, sym)
       {
+	if (completion_skip_symbol (mode, sym))
+	  continue;
+
 	completion_list_add_name (tracker,
 				  SYMBOL_LANGUAGE (sym),
 				  SYMBOL_LINKAGE_NAME (sym),
@@ -6495,6 +6501,9 @@  ada_collect_symbol_completion_matches (completion_tracker &tracker,
     b = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (s), GLOBAL_BLOCK);
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
+      if (completion_skip_symbol (mode, sym))
+	continue;
+
       completion_list_add_name (tracker,
 				SYMBOL_LANGUAGE (sym),
 				SYMBOL_LINKAGE_NAME (sym),
@@ -6511,6 +6520,9 @@  ada_collect_symbol_completion_matches (completion_tracker &tracker,
       continue;
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
+      if (completion_skip_symbol (mode, sym))
+	continue;
+
       completion_list_add_name (tracker,
 				SYMBOL_LANGUAGE (sym),
 				SYMBOL_LINKAGE_NAME (sym),
diff --git a/gdb/symtab.c b/gdb/symtab.c
index bbc317d..52cf70a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5034,11 +5034,44 @@  completion_list_add_fields (completion_tracker &tracker,
     }
 }
 
+/* See symtab.h.  */
+
+bool
+symbol_is_function_or_method (symbol *sym)
+{
+  switch (TYPE_CODE (SYMBOL_TYPE (sym)))
+    {
+    case TYPE_CODE_FUNC:
+    case TYPE_CODE_METHOD:
+      return true;
+    default:
+      return false;
+    }
+}
+
+/* Return whether MSYMBOL is a function/method.  */
+
+bool
+symbol_is_function_or_method (minimal_symbol *msymbol)
+{
+  switch (MSYMBOL_TYPE (msymbol))
+    {
+    case mst_text:
+    case mst_text_gnu_ifunc:
+    case mst_solib_trampoline:
+    case mst_file_text:
+      return true;
+    default:
+      return false;
+    }
+}
+
 /* Add matching symbols from SYMTAB to the current completion list.  */
 
 static void
 add_symtab_completions (struct compunit_symtab *cust,
 			completion_tracker &tracker,
+			complete_symbol_mode mode,
 			const lookup_name_info &lookup_name,
 			const char *text, const char *word,
 			enum type_code code)
@@ -5057,6 +5090,9 @@  add_symtab_completions (struct compunit_symtab *cust,
       b = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (cust), i);
       ALL_BLOCK_SYMBOLS (b, iter, sym)
 	{
+	  if (completion_skip_symbol (mode, sym))
+	    continue;
+
 	  if (code == TYPE_CODE_UNDEF
 	      || (SYMBOL_DOMAIN (sym) == STRUCT_DOMAIN
 		  && TYPE_CODE (SYMBOL_TYPE (sym)) == code))
@@ -5156,6 +5192,9 @@  default_collect_symbol_completion_matches_break_on
 	{
 	  QUIT;
 
+	  if (completion_skip_symbol (mode, msymbol))
+	    continue;
+
 	  completion_list_add_msymbol (tracker, msymbol, lookup_name,
 				       sym_text, word);
 
@@ -5166,7 +5205,7 @@  default_collect_symbol_completion_matches_break_on
 
   /* Add completions for all currently loaded symbol tables.  */
   ALL_COMPUNITS (objfile, cust)
-    add_symtab_completions (cust, tracker, lookup_name,
+    add_symtab_completions (cust, tracker, mode, lookup_name,
 			    sym_text, word, code);
 
   /* Look through the partial symtabs for all symbols which begin by
@@ -5177,7 +5216,7 @@  default_collect_symbol_completion_matches_break_on
 			   [&] (compunit_symtab *symtab) /* expansion notify */
 			     {
 			       add_symtab_completions (symtab,
-						       tracker, lookup_name,
+						       tracker, mode, lookup_name,
 						       sym_text, word, code);
 			     },
 			   ALL_DOMAIN);
@@ -5384,7 +5423,7 @@  collect_file_symbol_completion_matches (completion_tracker &tracker,
   iterate_over_symtabs (srcfile, [&] (symtab *s)
     {
       add_symtab_completions (SYMTAB_COMPUNIT (s),
-			      tracker, lookup_name,
+			      tracker, mode, lookup_name,
 			      sym_text, word, TYPE_CODE_UNDEF);
       return false;
     });
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 736fea0..26444ad 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1809,6 +1809,26 @@  extern void collect_file_symbol_completion_matches
 extern completion_list
   make_source_files_completion_list (const char *, const char *);
 
+/* Return whether SYM is a function/method, as opposed to a data symbol.  */
+
+extern bool symbol_is_function_or_method (symbol *sym);
+
+/* Return whether MSYMBOL is a function/method, as opposed to a data
+   symbol */
+
+extern bool symbol_is_function_or_method (minimal_symbol *msymbol);
+
+/* Return whether SYM should be skipped in completion mode MODE.  In
+   linespec mode, we're only interested in functions/methods.  */
+
+template <typename Symbol>
+static bool
+completion_skip_symbol (complete_symbol_mode mode, Symbol *sym)
+{
+  return (mode == complete_symbol_mode::LINESPEC
+	  && !symbol_is_function_or_method (sym));
+}
+
 /* symtab.c */
 
 int matching_obj_sections (struct obj_section *, struct obj_section *);