[PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Felix Willgerodt]

This problem shows with anonymous structs/classes:

~~~
  struct {
    private:
      int unique_name_foo = 5;
    public:
      int get() { return unique_name_foo; } /* breakpoint.  */
  } a;
~~~

Before:

~~~
(gdb) p unique_name_foo
$1 = 5
(gdb) complete p unique_name_fo
(gdb)

~~~

After:

~~~
(gdb) p unique_name_foo
$1 = 5
(gdb) complete p unique_name_fo
p unique_name_foo
(gdb)
~~~

As we are able to print the member we should be able to complete on it.
GDB doesn't look at "this" and its members for complete, while it does
when printing.  So I tried fixing that.
I saw that "this" is always represented as a PTR type with the symbol
class LOC_COMPUTED (with g++ 11.3.1, clang++ 10.0.1 and icpx 2022.1).

Not knowing too much about LOC_COMPUTED, I am assuming that this is the right
symbol class for this case and that we should adjust
completion_list_add_fields() for it.
But it could very well be that I missed something.  Any comments welcome!
---
 gdb/symtab.c                          | 18 ++++++++++++------
 gdb/testsuite/gdb.cp/cpcompletion.exp | 16 +++++++++++++---
 gdb/testsuite/gdb.cp/pr9594.cc        |  9 ++++++---
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 40887f59d1f..6edeb4ea9db 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5688,14 +5688,20 @@ completion_list_add_fields (completion_tracker &tracker,
 			    const lookup_name_info &lookup_name,
 			    const char *text, const char *word)
 {
-  if (sym->aclass () == LOC_TYPEDEF)
+  struct type *t = sym->type ();
+
+  if (sym->aclass () == LOC_TYPEDEF
+      || (sym->aclass () == LOC_COMPUTED && t->code () == TYPE_CODE_PTR))
     {
-      struct type *t = sym->type ();
-      enum type_code c = t->code ();
-      int j;
+      /* Anonymous classes/structs are often/always represented as a
+	 pointer with LOC_COMPUTED.  Since we also want to show their
+	 fields as a completion result (as we can print them) we resolve
+	 their target type.  */
+      if (t->code () == TYPE_CODE_PTR)
+	t = TYPE_TARGET_TYPE (t);
 
-      if (c == TYPE_CODE_UNION || c == TYPE_CODE_STRUCT)
-	for (j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
+      if (t->code () == TYPE_CODE_UNION || t->code () == TYPE_CODE_STRUCT)
+	for (int j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
 	  if (t->field (j).name ())
 	    completion_list_add_name (tracker, sym->language (),
 				      t->field (j).name (),
diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp b/gdb/testsuite/gdb.cp/cpcompletion.exp
index d19ac9c1b69..07ec936e95a 100644
--- a/gdb/testsuite/gdb.cp/cpcompletion.exp
+++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
@@ -76,10 +76,11 @@ test_class_complete Foo F "complete class methods beginning with F" \
 
 # The tests below depend on the current code scope.
 
-set bp_location [gdb_get_line_number "Set breakpoint here" ${srcfile}]
+set bp_1 [gdb_get_line_number "BP1" ${srcfile}]
+set bp_2 [gdb_get_line_number "BP2" ${srcfile}]
 
-if {![runto "${srcfile}:$bp_location"]} {
-    perror "test suppressed"
+if {![runto "${srcfile}:$bp_1"]} {
+    untested "failed to run to first breakpoint"
     return
 }
 
@@ -135,3 +136,12 @@ with_test_prefix "expression with namespace" {
     # Add a disambiguating character and we get a unique completion.
     test_gdb_complete_unique "p Test_NS::f" "p Test_NS::foo"
 }
+
+# Test completion when stopped in a method of an anonymous struct.
+gdb_breakpoint "$srcfile:$bp_2"
+gdb_continue_to_breakpoint "continue to second bp" ".*$srcfile:$bp_2.*"
+
+# We should be able to complete on the members.  We are able to print
+# them after all.
+gdb_test "p unique_name_foo" "= 5"
+gdb_test "complete p unique_name_fo" "p unique_name_foo"
diff --git a/gdb/testsuite/gdb.cp/pr9594.cc b/gdb/testsuite/gdb.cp/pr9594.cc
index 54ddaafc0ca..a3823f9b84f 100644
--- a/gdb/testsuite/gdb.cp/pr9594.cc
+++ b/gdb/testsuite/gdb.cp/pr9594.cc
@@ -56,10 +56,13 @@ int main ()
 {
   // Anonymous struct with method.
   struct {
-    int get() { return 5; }
+    private:
+      int unique_name_foo = 5;
+    public:
+      int get() { return unique_name_foo; } /* BP2.  */
   } a;
   Foo foo1;
-  foo1.set_foo (42);		// Set breakpoint here.
-  a.get();			// Prevent compiler from throwing 'a' away.
+  foo1.set_foo (42);		/* BP1.  */
+  a.get ();
   return 0;
 }
-- 
2.34.3

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Bruno Larsen]

On 06/09/2022 09:21, Felix Willgerodt via Gdb-patches wrote:
> This problem shows with anonymous structs/classes:
>
> ~~~
>    struct {
>      private:
>        int unique_name_foo = 5;
>      public:
>        int get() { return unique_name_foo; } /* breakpoint.  */
>    } a;
> ~~~
>
> Before:
>
> ~~~
> (gdb) p unique_name_foo
> $1 = 5
> (gdb) complete p unique_name_fo
> (gdb)
>
> ~~~
>
> After:
>
> ~~~
> (gdb) p unique_name_foo
> $1 = 5
> (gdb) complete p unique_name_fo
> p unique_name_foo
> (gdb)
> ~~~
>
> As we are able to print the member we should be able to complete on it.
> GDB doesn't look at "this" and its members for complete, while it does
> when printing.  So I tried fixing that.
> I saw that "this" is always represented as a PTR type with the symbol
> class LOC_COMPUTED (with g++ 11.3.1, clang++ 10.0.1 and icpx 2022.1).
>
> Not knowing too much about LOC_COMPUTED, I am assuming that this is the right
> symbol class for this case and that we should adjust
> completion_list_add_fields() for it.
> But it could very well be that I missed something.  Any comments welcome!

Hi Felix,

Thanks for working on this, this is a good improvement, I'm just not 
sure if this is the best solution, from reading around the adress_class 
enum.

LOC_COMPUTED seems to have  to do with how GDB finds the address of the 
variable: a constant location expression. From what I understand, any 
global variable may end up with this aclass. If this understanding is 
correct, identifying anonymous structs from this information seems 
sketchy, and they should instead also have an address class of 
LOC_TYPEDEF because classes and structs are defining a struct... but I'm 
not sure, if someone who understands this system could pitch in, would 
be nice.

Even if my understanding is correct, the completion function still needs 
updating, as it doesn't handle TYPE_CODE_PTR. If I'm misunderstanding, 
the patch looks alright and fixes the problem, so I give it a +1, but I 
can't approve patches.

-- 
Cheers,
Bruno

[Ping] [PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Willgerodt, Felix]

*Ping*

One non-maintainer has "+1"-ed this: https://sourceware.org/pipermail/gdb-patches/2022-September/191705.html

But there is still the open question, if the current code is actually right filtering for LOC_TYPEDEF
(or for LOC_COMPUTED now). Any hints are appreciated.

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix <felix.willgerodt@intel.com>
> Sent: Dienstag, 6. September 2022 09:22
> To: gdb-patches@sourceware.org
> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
> Subject: [PATCH 1/1] gdb: Enable complete to show members of anonymous
> classes/structs.
> 
> This problem shows with anonymous structs/classes:
> 
> ~~~
>   struct {
>     private:
>       int unique_name_foo = 5;
>     public:
>       int get() { return unique_name_foo; } /* breakpoint.  */
>   } a;
> ~~~
> 
> Before:
> 
> ~~~
> (gdb) p unique_name_foo
> $1 = 5
> (gdb) complete p unique_name_fo
> (gdb)
> 
> ~~~
> 
> After:
> 
> ~~~
> (gdb) p unique_name_foo
> $1 = 5
> (gdb) complete p unique_name_fo
> p unique_name_foo
> (gdb)
> ~~~
> 
> As we are able to print the member we should be able to complete on it.
> GDB doesn't look at "this" and its members for complete, while it does
> when printing.  So I tried fixing that.
> I saw that "this" is always represented as a PTR type with the symbol
> class LOC_COMPUTED (with g++ 11.3.1, clang++ 10.0.1 and icpx 2022.1).
> 
> Not knowing too much about LOC_COMPUTED, I am assuming that this is the
> right
> symbol class for this case and that we should adjust
> completion_list_add_fields() for it.
> But it could very well be that I missed something.  Any comments welcome!
> ---
>  gdb/symtab.c                          | 18 ++++++++++++------
>  gdb/testsuite/gdb.cp/cpcompletion.exp | 16 +++++++++++++---
>  gdb/testsuite/gdb.cp/pr9594.cc        |  9 ++++++---
>  3 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 40887f59d1f..6edeb4ea9db 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -5688,14 +5688,20 @@ completion_list_add_fields (completion_tracker
> &tracker,
>  			    const lookup_name_info &lookup_name,
>  			    const char *text, const char *word)
>  {
> -  if (sym->aclass () == LOC_TYPEDEF)
> +  struct type *t = sym->type ();
> +
> +  if (sym->aclass () == LOC_TYPEDEF
> +      || (sym->aclass () == LOC_COMPUTED && t->code () ==
> TYPE_CODE_PTR))
>      {
> -      struct type *t = sym->type ();
> -      enum type_code c = t->code ();
> -      int j;
> +      /* Anonymous classes/structs are often/always represented as a
> +	 pointer with LOC_COMPUTED.  Since we also want to show their
> +	 fields as a completion result (as we can print them) we resolve
> +	 their target type.  */
> +      if (t->code () == TYPE_CODE_PTR)
> +	t = TYPE_TARGET_TYPE (t);
> 
> -      if (c == TYPE_CODE_UNION || c == TYPE_CODE_STRUCT)
> -	for (j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
> +      if (t->code () == TYPE_CODE_UNION || t->code () ==
> TYPE_CODE_STRUCT)
> +	for (int j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
>  	  if (t->field (j).name ())
>  	    completion_list_add_name (tracker, sym->language (),
>  				      t->field (j).name (),
> diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp
> b/gdb/testsuite/gdb.cp/cpcompletion.exp
> index d19ac9c1b69..07ec936e95a 100644
> --- a/gdb/testsuite/gdb.cp/cpcompletion.exp
> +++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
> @@ -76,10 +76,11 @@ test_class_complete Foo F "complete class methods
> beginning with F" \
> 
>  # The tests below depend on the current code scope.
> 
> -set bp_location [gdb_get_line_number "Set breakpoint here" ${srcfile}]
> +set bp_1 [gdb_get_line_number "BP1" ${srcfile}]
> +set bp_2 [gdb_get_line_number "BP2" ${srcfile}]
> 
> -if {![runto "${srcfile}:$bp_location"]} {
> -    perror "test suppressed"
> +if {![runto "${srcfile}:$bp_1"]} {
> +    untested "failed to run to first breakpoint"
>      return
>  }
> 
> @@ -135,3 +136,12 @@ with_test_prefix "expression with namespace" {
>      # Add a disambiguating character and we get a unique completion.
>      test_gdb_complete_unique "p Test_NS::f" "p Test_NS::foo"
>  }
> +
> +# Test completion when stopped in a method of an anonymous struct.
> +gdb_breakpoint "$srcfile:$bp_2"
> +gdb_continue_to_breakpoint "continue to second bp" ".*$srcfile:$bp_2.*"
> +
> +# We should be able to complete on the members.  We are able to print
> +# them after all.
> +gdb_test "p unique_name_foo" "= 5"
> +gdb_test "complete p unique_name_fo" "p unique_name_foo"
> diff --git a/gdb/testsuite/gdb.cp/pr9594.cc b/gdb/testsuite/gdb.cp/pr9594.cc
> index 54ddaafc0ca..a3823f9b84f 100644
> --- a/gdb/testsuite/gdb.cp/pr9594.cc
> +++ b/gdb/testsuite/gdb.cp/pr9594.cc
> @@ -56,10 +56,13 @@ int main ()
>  {
>    // Anonymous struct with method.
>    struct {
> -    int get() { return 5; }
> +    private:
> +      int unique_name_foo = 5;
> +    public:
> +      int get() { return unique_name_foo; } /* BP2.  */
>    } a;
>    Foo foo1;
> -  foo1.set_foo (42);		// Set breakpoint here.
> -  a.get();			// Prevent compiler from throwing 'a' away.
> +  foo1.set_foo (42);		/* BP1.  */
> +  a.get ();
>    return 0;
>  }
> --
> 2.34.3

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[Ping v2] [PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Willgerodt, Felix]

*Ping* v2

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Dienstag, 20. September 2022 11:49
> To: gdb-patches@sourceware.org
> Subject: [Ping] [PATCH 1/1] gdb: Enable complete to show members of
> anonymous classes/structs.
> 
> *Ping*
> 
> One non-maintainer has "+1"-ed this:
> https://sourceware.org/pipermail/gdb-patches/2022-
> September/191705.html
> 
> But there is still the open question, if the current code is actually right
> filtering for LOC_TYPEDEF
> (or for LOC_COMPUTED now). Any hints are appreciated.
> 
> Thanks,
> Felix
> 
> > -----Original Message-----
> > From: Willgerodt, Felix <felix.willgerodt@intel.com>
> > Sent: Dienstag, 6. September 2022 09:22
> > To: gdb-patches@sourceware.org
> > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
> > Subject: [PATCH 1/1] gdb: Enable complete to show members of
> anonymous
> > classes/structs.
> >
> > This problem shows with anonymous structs/classes:
> >
> > ~~~
> >   struct {
> >     private:
> >       int unique_name_foo = 5;
> >     public:
> >       int get() { return unique_name_foo; } /* breakpoint.  */
> >   } a;
> > ~~~
> >
> > Before:
> >
> > ~~~
> > (gdb) p unique_name_foo
> > $1 = 5
> > (gdb) complete p unique_name_fo
> > (gdb)
> >
> > ~~~
> >
> > After:
> >
> > ~~~
> > (gdb) p unique_name_foo
> > $1 = 5
> > (gdb) complete p unique_name_fo
> > p unique_name_foo
> > (gdb)
> > ~~~
> >
> > As we are able to print the member we should be able to complete on it.
> > GDB doesn't look at "this" and its members for complete, while it does
> > when printing.  So I tried fixing that.
> > I saw that "this" is always represented as a PTR type with the symbol
> > class LOC_COMPUTED (with g++ 11.3.1, clang++ 10.0.1 and icpx 2022.1).
> >
> > Not knowing too much about LOC_COMPUTED, I am assuming that this is
> the
> > right
> > symbol class for this case and that we should adjust
> > completion_list_add_fields() for it.
> > But it could very well be that I missed something.  Any comments
> welcome!
> > ---
> >  gdb/symtab.c                          | 18 ++++++++++++------
> >  gdb/testsuite/gdb.cp/cpcompletion.exp | 16 +++++++++++++---
> >  gdb/testsuite/gdb.cp/pr9594.cc        |  9 ++++++---
> >  3 files changed, 31 insertions(+), 12 deletions(-)
> >
> > diff --git a/gdb/symtab.c b/gdb/symtab.c
> > index 40887f59d1f..6edeb4ea9db 100644
> > --- a/gdb/symtab.c
> > +++ b/gdb/symtab.c
> > @@ -5688,14 +5688,20 @@ completion_list_add_fields
> (completion_tracker
> > &tracker,
> >  			    const lookup_name_info &lookup_name,
> >  			    const char *text, const char *word)
> >  {
> > -  if (sym->aclass () == LOC_TYPEDEF)
> > +  struct type *t = sym->type ();
> > +
> > +  if (sym->aclass () == LOC_TYPEDEF
> > +      || (sym->aclass () == LOC_COMPUTED && t->code () ==
> > TYPE_CODE_PTR))
> >      {
> > -      struct type *t = sym->type ();
> > -      enum type_code c = t->code ();
> > -      int j;
> > +      /* Anonymous classes/structs are often/always represented as a
> > +	 pointer with LOC_COMPUTED.  Since we also want to show their
> > +	 fields as a completion result (as we can print them) we resolve
> > +	 their target type.  */
> > +      if (t->code () == TYPE_CODE_PTR)
> > +	t = TYPE_TARGET_TYPE (t);
> >
> > -      if (c == TYPE_CODE_UNION || c == TYPE_CODE_STRUCT)
> > -	for (j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
> > +      if (t->code () == TYPE_CODE_UNION || t->code () ==
> > TYPE_CODE_STRUCT)
> > +	for (int j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
> >  	  if (t->field (j).name ())
> >  	    completion_list_add_name (tracker, sym->language (),
> >  				      t->field (j).name (),
> > diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp
> > b/gdb/testsuite/gdb.cp/cpcompletion.exp
> > index d19ac9c1b69..07ec936e95a 100644
> > --- a/gdb/testsuite/gdb.cp/cpcompletion.exp
> > +++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
> > @@ -76,10 +76,11 @@ test_class_complete Foo F "complete class methods
> > beginning with F" \
> >
> >  # The tests below depend on the current code scope.
> >
> > -set bp_location [gdb_get_line_number "Set breakpoint here" ${srcfile}]
> > +set bp_1 [gdb_get_line_number "BP1" ${srcfile}]
> > +set bp_2 [gdb_get_line_number "BP2" ${srcfile}]
> >
> > -if {![runto "${srcfile}:$bp_location"]} {
> > -    perror "test suppressed"
> > +if {![runto "${srcfile}:$bp_1"]} {
> > +    untested "failed to run to first breakpoint"
> >      return
> >  }
> >
> > @@ -135,3 +136,12 @@ with_test_prefix "expression with namespace" {
> >      # Add a disambiguating character and we get a unique completion.
> >      test_gdb_complete_unique "p Test_NS::f" "p Test_NS::foo"
> >  }
> > +
> > +# Test completion when stopped in a method of an anonymous struct.
> > +gdb_breakpoint "$srcfile:$bp_2"
> > +gdb_continue_to_breakpoint "continue to second bp"
> ".*$srcfile:$bp_2.*"
> > +
> > +# We should be able to complete on the members.  We are able to print
> > +# them after all.
> > +gdb_test "p unique_name_foo" "= 5"
> > +gdb_test "complete p unique_name_fo" "p unique_name_foo"
> > diff --git a/gdb/testsuite/gdb.cp/pr9594.cc
> b/gdb/testsuite/gdb.cp/pr9594.cc
> > index 54ddaafc0ca..a3823f9b84f 100644
> > --- a/gdb/testsuite/gdb.cp/pr9594.cc
> > +++ b/gdb/testsuite/gdb.cp/pr9594.cc
> > @@ -56,10 +56,13 @@ int main ()
> >  {
> >    // Anonymous struct with method.
> >    struct {
> > -    int get() { return 5; }
> > +    private:
> > +      int unique_name_foo = 5;
> > +    public:
> > +      int get() { return unique_name_foo; } /* BP2.  */
> >    } a;
> >    Foo foo1;
> > -  foo1.set_foo (42);		// Set breakpoint here.
> > -  a.get();			// Prevent compiler from throwing 'a' away.
> > +  foo1.set_foo (42);		/* BP1.  */
> > +  a.get ();
> >    return 0;
> >  }
> > --
> > 2.34.3

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [Ping v3] [PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Willgerodt, Felix]

*Ping* v3

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Montag, 17. Oktober 2022 10:34
> To: gdb-patches@sourceware.org
> Subject: [Ping v2] [PATCH 1/1] gdb: Enable complete to show members of
> anonymous classes/structs.
> 
> *Ping* v2
> 
> Thanks,
> Felix
> 
> > -----Original Message-----
> > From: Willgerodt, Felix
> > Sent: Dienstag, 20. September 2022 11:49
> > To: gdb-patches@sourceware.org
> > Subject: [Ping] [PATCH 1/1] gdb: Enable complete to show members of
> > anonymous classes/structs.
> >
> > *Ping*
> >
> > One non-maintainer has "+1"-ed this:
> > https://sourceware.org/pipermail/gdb-patches/2022-
> > September/191705.html
> >
> > But there is still the open question, if the current code is actually right
> > filtering for LOC_TYPEDEF
> > (or for LOC_COMPUTED now). Any hints are appreciated.
> >
> > Thanks,
> > Felix
> >
> > > -----Original Message-----
> > > From: Willgerodt, Felix <felix.willgerodt@intel.com>
> > > Sent: Dienstag, 6. September 2022 09:22
> > > To: gdb-patches@sourceware.org
> > > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
> > > Subject: [PATCH 1/1] gdb: Enable complete to show members of
> > anonymous
> > > classes/structs.
> > >
> > > This problem shows with anonymous structs/classes:
> > >
> > > ~~~
> > >   struct {
> > >     private:
> > >       int unique_name_foo = 5;
> > >     public:
> > >       int get() { return unique_name_foo; } /* breakpoint.  */
> > >   } a;
> > > ~~~
> > >
> > > Before:
> > >
> > > ~~~
> > > (gdb) p unique_name_foo
> > > $1 = 5
> > > (gdb) complete p unique_name_fo
> > > (gdb)
> > >
> > > ~~~
> > >
> > > After:
> > >
> > > ~~~
> > > (gdb) p unique_name_foo
> > > $1 = 5
> > > (gdb) complete p unique_name_fo
> > > p unique_name_foo
> > > (gdb)
> > > ~~~
> > >
> > > As we are able to print the member we should be able to complete on it.
> > > GDB doesn't look at "this" and its members for complete, while it does
> > > when printing.  So I tried fixing that.
> > > I saw that "this" is always represented as a PTR type with the symbol
> > > class LOC_COMPUTED (with g++ 11.3.1, clang++ 10.0.1 and icpx 2022.1).
> > >
> > > Not knowing too much about LOC_COMPUTED, I am assuming that this is
> > the
> > > right
> > > symbol class for this case and that we should adjust
> > > completion_list_add_fields() for it.
> > > But it could very well be that I missed something.  Any comments
> > welcome!
> > > ---
> > >  gdb/symtab.c                          | 18 ++++++++++++------
> > >  gdb/testsuite/gdb.cp/cpcompletion.exp | 16 +++++++++++++---
> > >  gdb/testsuite/gdb.cp/pr9594.cc        |  9 ++++++---
> > >  3 files changed, 31 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/gdb/symtab.c b/gdb/symtab.c
> > > index 40887f59d1f..6edeb4ea9db 100644
> > > --- a/gdb/symtab.c
> > > +++ b/gdb/symtab.c
> > > @@ -5688,14 +5688,20 @@ completion_list_add_fields
> > (completion_tracker
> > > &tracker,
> > >  			    const lookup_name_info &lookup_name,
> > >  			    const char *text, const char *word)
> > >  {
> > > -  if (sym->aclass () == LOC_TYPEDEF)
> > > +  struct type *t = sym->type ();
> > > +
> > > +  if (sym->aclass () == LOC_TYPEDEF
> > > +      || (sym->aclass () == LOC_COMPUTED && t->code () ==
> > > TYPE_CODE_PTR))
> > >      {
> > > -      struct type *t = sym->type ();
> > > -      enum type_code c = t->code ();
> > > -      int j;
> > > +      /* Anonymous classes/structs are often/always represented as a
> > > +	 pointer with LOC_COMPUTED.  Since we also want to show their
> > > +	 fields as a completion result (as we can print them) we resolve
> > > +	 their target type.  */
> > > +      if (t->code () == TYPE_CODE_PTR)
> > > +	t = TYPE_TARGET_TYPE (t);
> > >
> > > -      if (c == TYPE_CODE_UNION || c == TYPE_CODE_STRUCT)
> > > -	for (j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
> > > +      if (t->code () == TYPE_CODE_UNION || t->code () ==
> > > TYPE_CODE_STRUCT)
> > > +	for (int j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
> > >  	  if (t->field (j).name ())
> > >  	    completion_list_add_name (tracker, sym->language (),
> > >  				      t->field (j).name (),
> > > diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > b/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > index d19ac9c1b69..07ec936e95a 100644
> > > --- a/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > +++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > @@ -76,10 +76,11 @@ test_class_complete Foo F "complete class methods
> > > beginning with F" \
> > >
> > >  # The tests below depend on the current code scope.
> > >
> > > -set bp_location [gdb_get_line_number "Set breakpoint here" ${srcfile}]
> > > +set bp_1 [gdb_get_line_number "BP1" ${srcfile}]
> > > +set bp_2 [gdb_get_line_number "BP2" ${srcfile}]
> > >
> > > -if {![runto "${srcfile}:$bp_location"]} {
> > > -    perror "test suppressed"
> > > +if {![runto "${srcfile}:$bp_1"]} {
> > > +    untested "failed to run to first breakpoint"
> > >      return
> > >  }
> > >
> > > @@ -135,3 +136,12 @@ with_test_prefix "expression with namespace" {
> > >      # Add a disambiguating character and we get a unique completion.
> > >      test_gdb_complete_unique "p Test_NS::f" "p Test_NS::foo"
> > >  }
> > > +
> > > +# Test completion when stopped in a method of an anonymous struct.
> > > +gdb_breakpoint "$srcfile:$bp_2"
> > > +gdb_continue_to_breakpoint "continue to second bp"
> > ".*$srcfile:$bp_2.*"
> > > +
> > > +# We should be able to complete on the members.  We are able to print
> > > +# them after all.
> > > +gdb_test "p unique_name_foo" "= 5"
> > > +gdb_test "complete p unique_name_fo" "p unique_name_foo"
> > > diff --git a/gdb/testsuite/gdb.cp/pr9594.cc
> > b/gdb/testsuite/gdb.cp/pr9594.cc
> > > index 54ddaafc0ca..a3823f9b84f 100644
> > > --- a/gdb/testsuite/gdb.cp/pr9594.cc
> > > +++ b/gdb/testsuite/gdb.cp/pr9594.cc
> > > @@ -56,10 +56,13 @@ int main ()
> > >  {
> > >    // Anonymous struct with method.
> > >    struct {
> > > -    int get() { return 5; }
> > > +    private:
> > > +      int unique_name_foo = 5;
> > > +    public:
> > > +      int get() { return unique_name_foo; } /* BP2.  */
> > >    } a;
> > >    Foo foo1;
> > > -  foo1.set_foo (42);		// Set breakpoint here.
> > > -  a.get();			// Prevent compiler from throwing 'a' away.
> > > +  foo1.set_foo (42);		/* BP1.  */
> > > +  a.get ();
> > >    return 0;
> > >  }
> > > --
> > > 2.34.3

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [Ping v4] [PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Willgerodt, Felix]

*Ping* v3

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Mittwoch, 26. Oktober 2022 14:06
> To: gdb-patches@sourceware.org
> Subject: RE: [Ping v3] [PATCH 1/1] gdb: Enable complete to show members of
> anonymous classes/structs.
> 
> *Ping* v3
> 
> Thanks,
> Felix
> 
> > -----Original Message-----
> > From: Willgerodt, Felix
> > Sent: Montag, 17. Oktober 2022 10:34
> > To: gdb-patches@sourceware.org
> > Subject: [Ping v2] [PATCH 1/1] gdb: Enable complete to show members of
> > anonymous classes/structs.
> >
> > *Ping* v2
> >
> > Thanks,
> > Felix
> >
> > > -----Original Message-----
> > > From: Willgerodt, Felix
> > > Sent: Dienstag, 20. September 2022 11:49
> > > To: gdb-patches@sourceware.org
> > > Subject: [Ping] [PATCH 1/1] gdb: Enable complete to show members of
> > > anonymous classes/structs.
> > >
> > > *Ping*
> > >
> > > One non-maintainer has "+1"-ed this:
> > > https://sourceware.org/pipermail/gdb-patches/2022-
> > > September/191705.html
> > >
> > > But there is still the open question, if the current code is actually right
> > > filtering for LOC_TYPEDEF
> > > (or for LOC_COMPUTED now). Any hints are appreciated.
> > >
> > > Thanks,
> > > Felix
> > >
> > > > -----Original Message-----
> > > > From: Willgerodt, Felix <felix.willgerodt@intel.com>
> > > > Sent: Dienstag, 6. September 2022 09:22
> > > > To: gdb-patches@sourceware.org
> > > > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
> > > > Subject: [PATCH 1/1] gdb: Enable complete to show members of
> > > anonymous
> > > > classes/structs.
> > > >
> > > > This problem shows with anonymous structs/classes:
> > > >
> > > > ~~~
> > > >   struct {
> > > >     private:
> > > >       int unique_name_foo = 5;
> > > >     public:
> > > >       int get() { return unique_name_foo; } /* breakpoint.  */
> > > >   } a;
> > > > ~~~
> > > >
> > > > Before:
> > > >
> > > > ~~~
> > > > (gdb) p unique_name_foo
> > > > $1 = 5
> > > > (gdb) complete p unique_name_fo
> > > > (gdb)
> > > >
> > > > ~~~
> > > >
> > > > After:
> > > >
> > > > ~~~
> > > > (gdb) p unique_name_foo
> > > > $1 = 5
> > > > (gdb) complete p unique_name_fo
> > > > p unique_name_foo
> > > > (gdb)
> > > > ~~~
> > > >
> > > > As we are able to print the member we should be able to complete on
> it.
> > > > GDB doesn't look at "this" and its members for complete, while it does
> > > > when printing.  So I tried fixing that.
> > > > I saw that "this" is always represented as a PTR type with the symbol
> > > > class LOC_COMPUTED (with g++ 11.3.1, clang++ 10.0.1 and icpx 2022.1).
> > > >
> > > > Not knowing too much about LOC_COMPUTED, I am assuming that this
> is
> > > the
> > > > right
> > > > symbol class for this case and that we should adjust
> > > > completion_list_add_fields() for it.
> > > > But it could very well be that I missed something.  Any comments
> > > welcome!
> > > > ---
> > > >  gdb/symtab.c                          | 18 ++++++++++++------
> > > >  gdb/testsuite/gdb.cp/cpcompletion.exp | 16 +++++++++++++---
> > > >  gdb/testsuite/gdb.cp/pr9594.cc        |  9 ++++++---
> > > >  3 files changed, 31 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/gdb/symtab.c b/gdb/symtab.c
> > > > index 40887f59d1f..6edeb4ea9db 100644
> > > > --- a/gdb/symtab.c
> > > > +++ b/gdb/symtab.c
> > > > @@ -5688,14 +5688,20 @@ completion_list_add_fields
> > > (completion_tracker
> > > > &tracker,
> > > >  			    const lookup_name_info &lookup_name,
> > > >  			    const char *text, const char *word)
> > > >  {
> > > > -  if (sym->aclass () == LOC_TYPEDEF)
> > > > +  struct type *t = sym->type ();
> > > > +
> > > > +  if (sym->aclass () == LOC_TYPEDEF
> > > > +      || (sym->aclass () == LOC_COMPUTED && t->code () ==
> > > > TYPE_CODE_PTR))
> > > >      {
> > > > -      struct type *t = sym->type ();
> > > > -      enum type_code c = t->code ();
> > > > -      int j;
> > > > +      /* Anonymous classes/structs are often/always represented as a
> > > > +	 pointer with LOC_COMPUTED.  Since we also want to show their
> > > > +	 fields as a completion result (as we can print them) we resolve
> > > > +	 their target type.  */
> > > > +      if (t->code () == TYPE_CODE_PTR)
> > > > +	t = TYPE_TARGET_TYPE (t);
> > > >
> > > > -      if (c == TYPE_CODE_UNION || c == TYPE_CODE_STRUCT)
> > > > -	for (j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
> > > > +      if (t->code () == TYPE_CODE_UNION || t->code () ==
> > > > TYPE_CODE_STRUCT)
> > > > +	for (int j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
> > > >  	  if (t->field (j).name ())
> > > >  	    completion_list_add_name (tracker, sym->language (),
> > > >  				      t->field (j).name (),
> > > > diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > > b/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > > index d19ac9c1b69..07ec936e95a 100644
> > > > --- a/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > > +++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
> > > > @@ -76,10 +76,11 @@ test_class_complete Foo F "complete class
> methods
> > > > beginning with F" \
> > > >
> > > >  # The tests below depend on the current code scope.
> > > >
> > > > -set bp_location [gdb_get_line_number "Set breakpoint here"
> ${srcfile}]
> > > > +set bp_1 [gdb_get_line_number "BP1" ${srcfile}]
> > > > +set bp_2 [gdb_get_line_number "BP2" ${srcfile}]
> > > >
> > > > -if {![runto "${srcfile}:$bp_location"]} {
> > > > -    perror "test suppressed"
> > > > +if {![runto "${srcfile}:$bp_1"]} {
> > > > +    untested "failed to run to first breakpoint"
> > > >      return
> > > >  }
> > > >
> > > > @@ -135,3 +136,12 @@ with_test_prefix "expression with namespace"
> {
> > > >      # Add a disambiguating character and we get a unique completion.
> > > >      test_gdb_complete_unique "p Test_NS::f" "p Test_NS::foo"
> > > >  }
> > > > +
> > > > +# Test completion when stopped in a method of an anonymous struct.
> > > > +gdb_breakpoint "$srcfile:$bp_2"
> > > > +gdb_continue_to_breakpoint "continue to second bp"
> > > ".*$srcfile:$bp_2.*"
> > > > +
> > > > +# We should be able to complete on the members.  We are able to
> print
> > > > +# them after all.
> > > > +gdb_test "p unique_name_foo" "= 5"
> > > > +gdb_test "complete p unique_name_fo" "p unique_name_foo"
> > > > diff --git a/gdb/testsuite/gdb.cp/pr9594.cc
> > > b/gdb/testsuite/gdb.cp/pr9594.cc
> > > > index 54ddaafc0ca..a3823f9b84f 100644
> > > > --- a/gdb/testsuite/gdb.cp/pr9594.cc
> > > > +++ b/gdb/testsuite/gdb.cp/pr9594.cc
> > > > @@ -56,10 +56,13 @@ int main ()
> > > >  {
> > > >    // Anonymous struct with method.
> > > >    struct {
> > > > -    int get() { return 5; }
> > > > +    private:
> > > > +      int unique_name_foo = 5;
> > > > +    public:
> > > > +      int get() { return unique_name_foo; } /* BP2.  */
> > > >    } a;
> > > >    Foo foo1;
> > > > -  foo1.set_foo (42);		// Set breakpoint here.
> > > > -  a.get();			// Prevent compiler from throwing 'a' away.
> > > > +  foo1.set_foo (42);		/* BP1.  */
> > > > +  a.get ();
> > > >    return 0;
> > > >  }
> > > > --
> > > > 2.34.3

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Tom Tromey]

>>>>> "Felix" == Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> writes:

Hello.  Thank you for the patch.  I'm sorry it took so long to review,
but I appreciate your persistence in pinging it.

Felix> After:
Felix> ~~~
Felix> (gdb) p unique_name_foo
Felix> $1 = 5
Felix> (gdb) complete p unique_name_fo
Felix> p unique_name_foo
Felix> (gdb)
Felix> ~~~

The outcome seems like what we want.

Felix> As we are able to print the member we should be able to complete on it.
Felix> GDB doesn't look at "this" and its members for complete, while it does
Felix> when printing.  So I tried fixing that.
Felix> I saw that "this" is always represented as a PTR type with the symbol
Felix> class LOC_COMPUTED (with g++ 11.3.1, clang++ 10.0.1 and icpx 2022.1).

Felix> Not knowing too much about LOC_COMPUTED, I am assuming that this is the right
Felix> symbol class for this case and that we should adjust
Felix> completion_list_add_fields() for it.
Felix> But it could very well be that I missed something.  Any comments welcome!

I don't think there's any requirement that 'this' be LOC_COMPUTED.

LOC_COMPUTED just means that the symbol's address is computed via some
function call (normally into the DWARF code to handle an exprloc).
However, nothing says it couldn't be LOC_REGISTER or LOC_ARG or
something.

Felix> +  if (sym->aclass () == LOC_TYPEDEF
Felix> +      || (sym->aclass () == LOC_COMPUTED && t->code () == TYPE_CODE_PTR))

It seems like this could match many local symbols -- like, nearly any
local variable that has pointer type.  But, completing on these fields
is not desirable; instead you only want to examine 'this'.

Instead you probably want something like what lookup_symbol_aux does:

  if (is_a_field_of_this != NULL && domain != STRUCT_DOMAIN)
    {
      result = lookup_language_this (langdef, block);

      if (result.symbol)

That is, use the current language (or in this case perhaps some search
language, I don't really know) to find the name of 'this', then use it
if this symbol matches.  Probably this should be done in the caller to
avoid a lot of repeated calls.

Felix> +	for (int j = TYPE_N_BASECLASSES (t); j < t->num_fields (); j++)
Felix>  	  if (t->field (j).name ())
Felix>  	    completion_list_add_name (tracker, sym->language (),

To me this loop looks incomplete, because it seems like it ought to walk
over superclasses as well.  Not your bug but if you really want this to
work correctly, it's worth fixing (and adding a test for).

Tom

RE: [PATCH 1/1] gdb: Enable complete to show members of anonymous classes/structs.

[Willgerodt, Felix]

> Felix> Not knowing too much about LOC_COMPUTED, I am assuming that this
> is the right
> Felix> symbol class for this case and that we should adjust
> Felix> completion_list_add_fields() for it.
> Felix> But it could very well be that I missed something.  Any comments
> welcome!
> 
> I don't think there's any requirement that 'this' be LOC_COMPUTED.
> 
> LOC_COMPUTED just means that the symbol's address is computed via some
> function call (normally into the DWARF code to handle an exprloc).
> However, nothing says it couldn't be LOC_REGISTER or LOC_ARG or
> something.

Why is the code in completion_list_add_fields() restricted to LOC_TYPEDEF
currently? That is what I don't understand at all, and why I (over-) cautiously
added LOC_COMPUTED in the first place.


> Felix> +  if (sym->aclass () == LOC_TYPEDEF
> Felix> +      || (sym->aclass () == LOC_COMPUTED && t->code () ==
> TYPE_CODE_PTR))
> 
> It seems like this could match many local symbols -- like, nearly any
> local variable that has pointer type.  But, completing on these fields
> is not desirable; instead you only want to examine 'this'.
> 
> Instead you probably want something like what lookup_symbol_aux does:
> 
>   if (is_a_field_of_this != NULL && domain != STRUCT_DOMAIN)
>     {
>       result = lookup_language_this (langdef, block);
> 
>       if (result.symbol)
> 
> That is, use the current language (or in this case perhaps some search
> language, I don't really know) to find the name of 'this', then use it
> if this symbol matches.  Probably this should be done in the caller to
> avoid a lot of repeated calls.

So we need to compare the symbol from lookup_language_this() with the
symbol we are currently looking at? Okay, I will try to find a solution
that is not repeating the call per symbol basis, but per block basis in the caller.
I will try to post a new revision soon, but it might take a bit.

Thanks,
Felix
 
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928