[PATCH] Remove completion_list_add_msymbol

[PATCH] Remove completion_list_add_msymbol

[Christian Biesinger via gdb-patches]

Now that both symbol and minimal_symbol inherit from general_symbol_info,
we can use the same function for both here.

gdb/ChangeLog:

2019-12-18  Christian Biesinger  <cbiesinger@google.com>

	* symtab.c (completion_list_add_symbol): Update.
	(completion_list_add_msymbol): Remove.
	(default_collect_symbol_completion_matches_break_on): Update.

Change-Id: Ifa01837d5f7e3438e7e6599c08baf1a2e7f086c9
---
 gdb/symtab.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 26551372cb..a76c5d304f 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5250,11 +5250,11 @@ completion_list_add_name (completion_tracker &tracker,
   }
 }
 
-/* completion_list_add_name wrapper for struct symbol.  */
+/* completion_list_add_name wrapper for struct general_symbol_info.  */
 
 static void
 completion_list_add_symbol (completion_tracker &tracker,
-			    symbol *sym,
+			    general_symbol_info *sym,
 			    const lookup_name_info &lookup_name,
 			    const char *text, const char *word)
 {
@@ -5263,20 +5263,6 @@ completion_list_add_symbol (completion_tracker &tracker,
 			    lookup_name, text, word);
 }
 
-/* completion_list_add_name wrapper for struct minimal_symbol.  */
-
-static void
-completion_list_add_msymbol (completion_tracker &tracker,
-			     minimal_symbol *sym,
-			     const lookup_name_info &lookup_name,
-			     const char *text, const char *word)
-{
-  completion_list_add_name (tracker, sym->language (),
-			    sym->natural_name (),
-			    lookup_name, text, word);
-}
-
-
 /* ObjC: In case we are completing on a selector, look as the msymbol
    again and feed all the selectors into the mill.  */
 
@@ -5614,8 +5600,8 @@ default_collect_symbol_completion_matches_break_on
 	      if (completion_skip_symbol (mode, msymbol))
 		continue;
 
-	      completion_list_add_msymbol (tracker, msymbol, lookup_name,
-					   sym_text, word);
+	      completion_list_add_symbol (tracker, msymbol, lookup_name,
+					  sym_text, word);
 
 	      completion_list_objc_symbol (tracker, msymbol, lookup_name,
 					   sym_text, word);
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH] Remove completion_list_add_msymbol

[Andrew Burgess]

* Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> [2019-12-18 19:42:38 -0600]:

> Now that both symbol and minimal_symbol inherit from general_symbol_info,
> we can use the same function for both here.
> 
> gdb/ChangeLog:
> 
> 2019-12-18  Christian Biesinger  <cbiesinger@google.com>
> 
> 	* symtab.c (completion_list_add_symbol): Update.
> 	(completion_list_add_msymbol): Remove.
> 	(default_collect_symbol_completion_matches_break_on): Update.

I have a change I'm currently working on that I think will add some
additional code to completion_list_add_symbol but not
completion_list_add_msymbol, however, it might take me a week or so to
finish up the change.

I'm happy for this patch to go in, but just a warning I might end up
having to reverse this as part of my work when I get it finished.

In case you (or anyone) is interested, the problem I'm trying to fix
is something that has annoyed me for ages.  Consider this C++ code:

  struct object { int a; };
  typedef object *object_p;

  static int
  get_value (object_p obj)
  {
    return obj->a;
  }

  int
  main ()
  {
    object obj;
    obj.a = 0;

    int val = get_value (&obj);
    return val;
  }

Now in GDB:

  (gdb) complete break get_va
  break get_value(object*)
  break get_value(object_p)

Those two completions, that's what drives me mad.  I now have GDB
basically fixed up so that it only offers `break get_value(object_p)`
as a completion (though you can manually type either if you wish), but
I just have one more issue left to resolve.

Thanks,
Andrew


> 
> Change-Id: Ifa01837d5f7e3438e7e6599c08baf1a2e7f086c9
> ---
>  gdb/symtab.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 26551372cb..a76c5d304f 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -5250,11 +5250,11 @@ completion_list_add_name (completion_tracker &tracker,
>    }
>  }
>  
> -/* completion_list_add_name wrapper for struct symbol.  */
> +/* completion_list_add_name wrapper for struct general_symbol_info.  */
>  
>  static void
>  completion_list_add_symbol (completion_tracker &tracker,
> -			    symbol *sym,
> +			    general_symbol_info *sym,
>  			    const lookup_name_info &lookup_name,
>  			    const char *text, const char *word)
>  {
> @@ -5263,20 +5263,6 @@ completion_list_add_symbol (completion_tracker &tracker,
>  			    lookup_name, text, word);
>  }
>  
> -/* completion_list_add_name wrapper for struct minimal_symbol.  */
> -
> -static void
> -completion_list_add_msymbol (completion_tracker &tracker,
> -			     minimal_symbol *sym,
> -			     const lookup_name_info &lookup_name,
> -			     const char *text, const char *word)
> -{
> -  completion_list_add_name (tracker, sym->language (),
> -			    sym->natural_name (),
> -			    lookup_name, text, word);
> -}
> -
> -
>  /* ObjC: In case we are completing on a selector, look as the msymbol
>     again and feed all the selectors into the mill.  */
>  
> @@ -5614,8 +5600,8 @@ default_collect_symbol_completion_matches_break_on
>  	      if (completion_skip_symbol (mode, msymbol))
>  		continue;
>  
> -	      completion_list_add_msymbol (tracker, msymbol, lookup_name,
> -					   sym_text, word);
> +	      completion_list_add_symbol (tracker, msymbol, lookup_name,
> +					  sym_text, word);
>  
>  	      completion_list_objc_symbol (tracker, msymbol, lookup_name,
>  					   sym_text, word);
> -- 
> 2.24.1.735.g03f4e72817-goog
> 

Re: [PATCH] Remove completion_list_add_msymbol

[Christian Biesinger via gdb-patches]

On Thu, Dec 19, 2019 at 3:58 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> [2019-12-18 19:42:38 -0600]:
>
> > Now that both symbol and minimal_symbol inherit from general_symbol_info,
> > we can use the same function for both here.
> >
> > gdb/ChangeLog:
> >
> > 2019-12-18  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * symtab.c (completion_list_add_symbol): Update.
> >       (completion_list_add_msymbol): Remove.
> >       (default_collect_symbol_completion_matches_break_on): Update.
>
> I have a change I'm currently working on that I think will add some
> additional code to completion_list_add_symbol but not
> completion_list_add_msymbol, however, it might take me a week or so to
> finish up the change.
>
> I'm happy for this patch to go in, but just a warning I might end up
> having to reverse this as part of my work when I get it finished.

Oh, in that case there's no point in landing this just to get it reverted.

(I still think it's confusing that the same symbol can have an msymbol
and a psymbol/symbol referring to it. I would really like to do
"something" about that; but not sure exactly what....)

Christian

> In case you (or anyone) is interested, the problem I'm trying to fix
> is something that has annoyed me for ages.  Consider this C++ code:
>
>   struct object { int a; };
>   typedef object *object_p;
>
>   static int
>   get_value (object_p obj)
>   {
>     return obj->a;
>   }
>
>   int
>   main ()
>   {
>     object obj;
>     obj.a = 0;
>
>     int val = get_value (&obj);
>     return val;
>   }
>
> Now in GDB:
>
>   (gdb) complete break get_va
>   break get_value(object*)
>   break get_value(object_p)
>
> Those two completions, that's what drives me mad.  I now have GDB
> basically fixed up so that it only offers `break get_value(object_p)`
> as a completion (though you can manually type either if you wish), but
> I just have one more issue left to resolve.
>
> Thanks,
> Andrew
>
>
> >
> > Change-Id: Ifa01837d5f7e3438e7e6599c08baf1a2e7f086c9
> > ---
> >  gdb/symtab.c | 22 ++++------------------
> >  1 file changed, 4 insertions(+), 18 deletions(-)
> >
> > diff --git a/gdb/symtab.c b/gdb/symtab.c
> > index 26551372cb..a76c5d304f 100644
> > --- a/gdb/symtab.c
> > +++ b/gdb/symtab.c
> > @@ -5250,11 +5250,11 @@ completion_list_add_name (completion_tracker &tracker,
> >    }
> >  }
> >
> > -/* completion_list_add_name wrapper for struct symbol.  */
> > +/* completion_list_add_name wrapper for struct general_symbol_info.  */
> >
> >  static void
> >  completion_list_add_symbol (completion_tracker &tracker,
> > -                         symbol *sym,
> > +                         general_symbol_info *sym,
> >                           const lookup_name_info &lookup_name,
> >                           const char *text, const char *word)
> >  {
> > @@ -5263,20 +5263,6 @@ completion_list_add_symbol (completion_tracker &tracker,
> >                           lookup_name, text, word);
> >  }
> >
> > -/* completion_list_add_name wrapper for struct minimal_symbol.  */
> > -
> > -static void
> > -completion_list_add_msymbol (completion_tracker &tracker,
> > -                          minimal_symbol *sym,
> > -                          const lookup_name_info &lookup_name,
> > -                          const char *text, const char *word)
> > -{
> > -  completion_list_add_name (tracker, sym->language (),
> > -                         sym->natural_name (),
> > -                         lookup_name, text, word);
> > -}
> > -
> > -
> >  /* ObjC: In case we are completing on a selector, look as the msymbol
> >     again and feed all the selectors into the mill.  */
> >
> > @@ -5614,8 +5600,8 @@ default_collect_symbol_completion_matches_break_on
> >             if (completion_skip_symbol (mode, msymbol))
> >               continue;
> >
> > -           completion_list_add_msymbol (tracker, msymbol, lookup_name,
> > -                                        sym_text, word);
> > +           completion_list_add_symbol (tracker, msymbol, lookup_name,
> > +                                       sym_text, word);
> >
> >             completion_list_objc_symbol (tracker, msymbol, lookup_name,
> >                                          sym_text, word);
> > --
> > 2.24.1.735.g03f4e72817-goog
> >