[PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Felix Willgerodt]

During prefix resolution, if the parent is a subprogram, there is no need
to go to the parent of the subprogram.  The DIE will be local.

For a program like:
~~~
  class F1
  {
  public:
    int a;
    int
    vvv ()
    {
      class F2
      {
	int f;
      };
      F2 abcd;
      return 1;
    }
  };
~~~

The class F2 should not be seen as a member of F1.

Before:
~~~
(gdb) ptype abcd
type = class F1::F2 {
  private:
    int f;
}
~~~

After:
~~~
(gdb) ptype abcd
type = class F2 {
  private:
    int f;
}
~~~

gdb/ChangeLog:
2021-06-23  Felix Willgerodt  <felix.willgerodt@intel.com>

	* dwarf2/read.c (determine_prefix): Return an empty prefix if the
	parent is a subprogram.

gdb/testsuite/ChangeLog:
2021-06-23  Felix Willgerodt  <felix.willgerodt@intel.com>

	* gdb.cp/nested-class-func-class.cc: New file.
	* gdb.cp/nested-class-func-class.exp: New file.
---
 gdb/dwarf2/read.c                             |  2 +-
 .../gdb.cp/nested-class-func-class.cc         | 46 ++++++++++++++++++
 .../gdb.cp/nested-class-func-class.exp        | 47 +++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.cp/nested-class-func-class.cc
 create mode 100644 gdb/testsuite/gdb.cp/nested-class-func-class.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 07bc08fba14..ba1a3087b87 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -22635,7 +22635,7 @@ determine_prefix (struct die_info *die, struct dwarf2_cu *cu)
 		&& (dwarf2_name (parent, cu) != NULL))
 	      return dwarf2_name (parent, cu);
 	  }
-	return determine_prefix (parent, cu);
+	return "";
       case DW_TAG_enumeration_type:
 	parent_type = read_type_die (parent, cu);
 	if (parent_type->is_declared_class ())
diff --git a/gdb/testsuite/gdb.cp/nested-class-func-class.cc b/gdb/testsuite/gdb.cp/nested-class-func-class.cc
new file mode 100644
index 00000000000..a8d255fcd93
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/nested-class-func-class.cc
@@ -0,0 +1,46 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+foo ()
+{
+  class F1
+  {
+  public:
+    int a;
+    int
+    vvv ()
+    {
+      class F2
+      {
+	int f;
+      };
+      F2 abcd;
+      return 1; /* BP 1 */
+    }
+  };
+
+  F1 x;
+  int a = x.vvv ();
+  return a; /* BP 2 */
+}
+
+int
+main ()
+{
+  return foo ();
+}
diff --git a/gdb/testsuite/gdb.cp/nested-class-func-class.exp b/gdb/testsuite/gdb.cp/nested-class-func-class.exp
new file mode 100644
index 00000000000..584c47f3a22
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/nested-class-func-class.exp
@@ -0,0 +1,47 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Regression test for type printing of private nested classes.
+
+if {[skip_cplus_tests]} { continue }
+
+load_lib "cp-support.exp"
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "BP 1"]
+gdb_breakpoint [gdb_get_line_number "BP 2"]
+gdb_continue_to_breakpoint "BP 1"
+
+cp_test_ptype_class \
+    "F2" "" "class" "F2" \
+    {
+	{ field private "int f;" }
+    }
+
+gdb_continue_to_breakpoint "BP 2"
+
+with_test_prefix "BP 2" {
+    gdb_test "ptype F1::F2" "There is no field named F2"
+    gdb_test "ptype F2" "No symbol \"F2\" in current context."
+}
-- 
2.25.4

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] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Simon Marchi]

On 2021-06-24 7:50 a.m., Felix Willgerodt via Gdb-patches wrote:
> During prefix resolution, if the parent is a subprogram, there is no need
> to go to the parent of the subprogram.  The DIE will be local.
> 
> For a program like:
> ~~~
>   class F1
>   {
>   public:
>     int a;
>     int
>     vvv ()
>     {
>       class F2
>       {
> 	int f;
>       };
>       F2 abcd;
>       return 1;
>     }
>   };
> ~~~
> 
> The class F2 should not be seen as a member of F1.
> 
> Before:
> ~~~
> (gdb) ptype abcd
> type = class F1::F2 {
>   private:
>     int f;
> }
> ~~~
> 
> After:
> ~~~
> (gdb) ptype abcd
> type = class F2 {
>   private:
>     int f;
> }
> ~~~

I played a bit with this and have a question.  The "fully-qualified
name" of F2 seems to be

  F1::vvv()::F2

since the symbol of method foo is

  _ZZN2F13vvvEvEN2F23fooEv
  F1::vvv()::F2::foo()

So would it make sense to display:

  (gdb) ptype abcd
  type = class F1::vvv()::F2 {
    ...
  }

?

And since these two work:

  (gdb) ptype F1
  type = class F1 {
    public:
      int a;

      int vvv(void);
  }
  (gdb) p 'F1::vvv()::F2::foo'
  $4 = {void (F1::F2 * const)} 0x555555555184 <F1::vvv()::F2::foo()>

I would kind of expect this to work:

  (gdb) ptype F1::vvv()::F2
  No symbol "F2" in specified context.
  (gdb) ptype 'F1::vvv()::F2'
  No symbol "F1::vvv()::F2" in current context.

Simon

RE: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Willgerodt, Felix]

> -----Original Message-----
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Sent: Freitag, 25. Juni 2021 17:15
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to
> get a prefix.
> 
> On 2021-06-24 7:50 a.m., Felix Willgerodt via Gdb-patches wrote:
> > During prefix resolution, if the parent is a subprogram, there is no need
> > to go to the parent of the subprogram.  The DIE will be local.
> >
> > For a program like:
> > ~~~
> >   class F1
> >   {
> >   public:
> >     int a;
> >     int
> >     vvv ()
> >     {
> >       class F2
> >       {
> > 	int f;
> >       };
> >       F2 abcd;
> >       return 1;
> >     }
> >   };
> > ~~~
> >
> > The class F2 should not be seen as a member of F1.
> >
> > Before:
> > ~~~
> > (gdb) ptype abcd
> > type = class F1::F2 {
> >   private:
> >     int f;
> > }
> > ~~~
> >
> > After:
> > ~~~
> > (gdb) ptype abcd
> > type = class F2 {
> >   private:
> >     int f;
> > }
> > ~~~
> 
> I played a bit with this and have a question.  The "fully-qualified
> name" of F2 seems to be
> 
>   F1::vvv()::F2
> 
> since the symbol of method foo is
> 
>   _ZZN2F13vvvEvEN2F23fooEv
>   F1::vvv()::F2::foo()
>


Are you using my testcase? Shouldn't this be foo()::F1::vvv()::F2? 


> So would it make sense to display:
> 
>   (gdb) ptype abcd
>   type = class F1::vvv()::F2 {
>     ...
>   }
> 
> ?

I am not sure GDB's convention is to show or use the fully qualified name usually.
We don't e.g. print 'main()::foo' for a class foo defined in main.
And to me that makes sense, as these names can get long.
Plus, for using these in a command, we would want to make sure that the "short name"
works when in scope.

Another problem I have with this is that - to me personally - vvv()::F2 
(getting "members" of functions) feels a bit "unnatural" and like a GDB
extension: https://sourceware.org/gdb/current/onlinedocs/gdb/Variables.html

What I also saw is that with clang GDB already uses the full name:

clang:
(gdb) bt
#0  foo()::F1::vvv() (this=0x7fffffffdf18) at nested-class-func-class.cc:33
#1  0x0000000000401121 in foo () at nested-class-func-class.cc:38
#2  0x0000000000401154 in main () at nested-class-func-class.cc:45
(gdb) ptype F1
No symbol "F1" in current context.
(gdb) ptype 'foo()::F1'
type = class F1 {
  public:
    int a;

    int vvv(void);
}

g++:
(gdb) bt
#0  F1::vvv (this=0x7fffffffdf20) at nested-class-func-class.cc:33
#1  0x0000555555555180 in foo () at nested-class-func-class.cc:38
#2  0x00005555555551a9 in main () at nested-class-func-class.cc:45
(gdb) ptype F1
type = class F1 {
  public:
    int a;

    int vvv(void);
}

Not being able to do 'ptype F1' seems wrong. I also prefer the backtrace for g++.
I couldn't find the reason for the output though. The Dwarf looks similar.
And my patch makes no difference here.


> And since these two work:
> 
>   (gdb) ptype F1
>   type = class F1 {
>     public:
>       int a;
> 
>       int vvv(void);
>   }
>   (gdb) p 'F1::vvv()::F2::foo'
>   $4 = {void (F1::F2 * const)} 0x555555555184 <F1::vvv()::F2::foo()>
> 
> I would kind of expect this to work:
> 
>   (gdb) ptype F1::vvv()::F2
>   No symbol "F2" in specified context.
>   (gdb) ptype 'F1::vvv()::F2'
>   No symbol "F1::vvv()::F2" in current context.
> 
> Simon


I get the point you are trying to make. Imo the short name should always work
when in scope. The rest probably should also work, but feels a bit more optional.
Especially when thinking about the print command, I can't really judge if the full
name wouldn't lead to bad behavior.

And I can't reproduce your output, the foo seems to be at the wrong place again.
But even with that fixed, I can't see this with clang nor g++ at any breakpoint.

I am more than happy about any further comments. I feel like my patch still improves
the situation a bit, but I am not very familiar with our dwarf parser and what we
actually want to achieve with the prefix function.

Thanks for the feedback,
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

Re: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Simon Marchi]

On 2021-06-28 6:21 a.m., Willgerodt, Felix wrote:>> -----Original Message-----
>> I played a bit with this and have a question.  The "fully-qualified
>> name" of F2 seems to be
>>
>>   F1::vvv()::F2
>>
>> since the symbol of method foo is
>>
>>   _ZZN2F13vvvEvEN2F23fooEv
>>   F1::vvv()::F2::foo()
>>
> 
> 
> Are you using my testcase? Shouldn't this be foo()::F1::vvv()::F2? 

Woops, I might have modified it while playing with it, not sure.  But
yeah, with 'foo()'.

>> So would it make sense to display:
>>
>>   (gdb) ptype abcd
>>   type = class F1::vvv()::F2 {
>>     ...
>>   }
>>
>> ?
> 
> I am not sure GDB's convention is to show or use the fully qualified name usually.
> We don't e.g. print 'main()::foo' for a class foo defined in main.

Well, like we currently don't print 'foo()' and 'vvv()' in your original
example.  But if we decide that they should be printed, then we would
also print 'main()::foo' for a foo structure defined in the main
function.  It's just another example of the same thing.

> And to me that makes sense, as these names can get long.

That's true, we need to balance accuracy and usability.  I don't have a
string opinion at the moment, it's something I could judge by using it
for a while.

> Plus, for using these in a command, we would want to make sure that the "short name"
> works when in scope.

Yes, mimicking the name lookup of the compiler.

> Another problem I have with this is that - to me personally - vvv()::F2 
> (getting "members" of functions) feels a bit "unnatural" and like a GDB
> extension: https://sourceware.org/gdb/current/onlinedocs/gdb/Variables.html

Hmm yes and no, it's not just GDB.  If I add a constructor to your F2
and run its symbol through c++filt, we get this same kind of syntax:

    $ echo "_ZZZ3foovEN2F13vvvEvEN2F2C2Ev" | c++filt
    foo()::F1::vvv()::F2::F2()

I think it's reasonable for GDB to use the same kind of syntax.

> What I also saw is that with clang GDB already uses the full name:
> 
> clang:
> (gdb) bt
> #0  foo()::F1::vvv() (this=0x7fffffffdf18) at nested-class-func-class.cc:33
> #1  0x0000000000401121 in foo () at nested-class-func-class.cc:38
> #2  0x0000000000401154 in main () at nested-class-func-class.cc:45
> (gdb) ptype F1
> No symbol "F1" in current context.
> (gdb) ptype 'foo()::F1'
> type = class F1 {
>   public:
>     int a;
> 
>     int vvv(void);
> }
> 
> g++:
> (gdb) bt
> #0  F1::vvv (this=0x7fffffffdf20) at nested-class-func-class.cc:33
> #1  0x0000555555555180 in foo () at nested-class-func-class.cc:38
> #2  0x00005555555551a9 in main () at nested-class-func-class.cc:45
> (gdb) ptype F1
> type = class F1 {
>   public:
>     int a;
> 
>     int vvv(void);
> }

Interesting!

> Not being able to do 'ptype F1' seems wrong.

I agree with you completely here.

> I also prefer the backtrace for g++.

In this particular example, I kinda like the output from clang, it's
just more precise.  I might change opinion once I will see an
example with many parameters and template that will make a monster of a
line.

> I couldn't find the reason for the output though. The Dwarf looks similar.
> And my patch makes no difference here.

I don't know off-hand, and I don't have time to investigate.  But
getting the same behavior out of the two compilers would be desirable.
Does you patch accomplish that?

>> And since these two work:
>>
>>   (gdb) ptype F1
>>   type = class F1 {
>>     public:
>>       int a;
>>
>>       int vvv(void);
>>   }
>>   (gdb) p 'F1::vvv()::F2::foo'
>>   $4 = {void (F1::F2 * const)} 0x555555555184 <F1::vvv()::F2::foo()>
>>
>> I would kind of expect this to work:
>>
>>   (gdb) ptype F1::vvv()::F2
>>   No symbol "F2" in specified context.
>>   (gdb) ptype 'F1::vvv()::F2'
>>   No symbol "F1::vvv()::F2" in current context.
>>
>> Simon
> 
> 
> I get the point you are trying to make. Imo the short name should always work
> when in scope.

For input, indeed.

> The rest probably should also work, but feels a bit more optional.
> Especially when thinking about the print command, I can't really judge if the full
> name wouldn't lead to bad behavior.
> 
> And I can't reproduce your output, the foo seems to be at the wrong place again.
> But even with that fixed, I can't see this with clang nor g++ at any breakpoint.
> 
> I am more than happy about any further comments. I feel like my patch still improves
> the situation a bit, but I am not very familiar with our dwarf parser and what we
> actually want to achieve with the prefix function.

I do think that showing "class F1::F2" is wrong for sure.  It would be
the right thing to show for for:

  struct F1 {
    struct F2 {
    }
  }

I'd just like to have a third opinion on the matter.  If others think
that showing the short name is fine, then that's fine with me too.

Simon

Re: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Keith Seitz]

On 6/28/21 12:21 PM, Simon Marchi via Gdb-patches wrote:
> I do think that showing "class F1::F2" is wrong for sure.  It would be
> the right thing to show for for:
> 
>   struct F1 {
>     struct F2 {
>     }
>   }
> 
> I'd just like to have a third opinion on the matter.  If others think
> that showing the short name is fine, then that's fine with me too.

This situation isn't altogether dissimilar from how we handle static variables
defined inside functions, e.g.,

1	static int
2	foo()
3	{
4		static const int value = 21;
5		return value;
6	}
7	
8	int
9	main()
10	{
(gdb) p foo()::value
$2 = 21

FWIW,
Keith

RE: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Willgerodt, Felix]

> -----Original Message-----
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Sent: Montag, 28. Juni 2021 21:21
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to
> get a prefix.
> 
> On 2021-06-28 6:21 a.m., Willgerodt, Felix wrote:>> -----Original Message-----
> > Another problem I have with this is that - to me personally - vvv()::F2
> > (getting "members" of functions) feels a bit "unnatural" and like a GDB
> > extension:
> > https://sourceware.org/gdb/current/onlinedocs/gdb/Variables.html
> 
> Hmm yes and no, it's not just GDB.  If I add a constructor to your F2
> and run its symbol through c++filt, we get this same kind of syntax:
> 
>     $ echo "_ZZZ3foovEN2F13vvvEvEN2F2C2Ev" | c++filt
>     foo()::F1::vvv()::F2::F2()
> 
> I think it's reasonable for GDB to use the same kind of syntax.
> 

My reasoning was that you can't compile anything like e.g. vvv()::F2 or
vvv::F2. At least I couldn't when I tried, if someone else can, let me know!
I am totally fine with using the syntax though, this is just my personal view.

> > What I also saw is that with clang GDB already uses the full name:
> >
> > clang:
> > (gdb) bt
> > #0  foo()::F1::vvv() (this=0x7fffffffdf18) at nested-class-func-class.cc:33
> > #1  0x0000000000401121 in foo () at nested-class-func-class.cc:38
> > #2  0x0000000000401154 in main () at nested-class-func-class.cc:45
> > (gdb) ptype F1
> > No symbol "F1" in current context.
> > (gdb) ptype 'foo()::F1'
> > type = class F1 {
> >   public:
> >     int a;
> >
> >     int vvv(void);
> > }
> >
> > g++:
> > (gdb) bt
> > #0  F1::vvv (this=0x7fffffffdf20) at nested-class-func-class.cc:33
> > #1  0x0000555555555180 in foo () at nested-class-func-class.cc:38
> > #2  0x00005555555551a9 in main () at nested-class-func-class.cc:45
> > (gdb) ptype F1
> > type = class F1 {
> >   public:
> >     int a;
> >
> >     int vvv(void);
> > }
> 
> Interesting!
> 
> > Not being able to do 'ptype F1' seems wrong.
> 
> I agree with you completely here.
> 
> > I also prefer the backtrace for g++.
> 
> In this particular example, I kinda like the output from clang, it's
> just more precise.  I might change opinion once I will see an
> example with many parameters and template that will make a monster of a
> line.
> 

Yes that is one of my fears, plus large call stacks with a lot of frames.
It is also inconsistent right now, as it only uses the full name for frame 0,
but not for frame 1.

> > I couldn't find the reason for the output though. The Dwarf looks similar.
> > And my patch makes no difference here.
> 
> I don't know off-hand, and I don't have time to investigate.  But
> getting the same behavior out of the two compilers would be desirable.
> Does you patch accomplish that?
> 

No, Clang and g++ output are still different. That is what I meant with my
patch makes no difference. The output is the same for both, with or without my
patch.

> 
> I do think that showing "class F1::F2" is wrong for sure.  It would be
> the right thing to show for for:
> 
>   struct F1 {
>     struct F2 {
>     }
>   }
> 
> I'd just like to have a third opinion on the matter.  If others think
> that showing the short name is fine, then that's fine with me too.
> 
> Simon

I am happy about other opinions as well!

Regards,
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

Re: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Simon Marchi]

>> Hmm yes and no, it's not just GDB.  If I add a constructor to your F2
>> and run its symbol through c++filt, we get this same kind of syntax:
>>
>>     $ echo "_ZZZ3foovEN2F13vvvEvEN2F2C2Ev" | c++filt
>>     foo()::F1::vvv()::F2::F2()
>>
>> I think it's reasonable for GDB to use the same kind of syntax.
>>
> 
> My reasoning was that you can't compile anything like e.g. vvv()::F2 or
> vvv::F2. At least I couldn't when I tried, if someone else can, let me know!
> I am totally fine with using the syntax though, this is just my personal view.

I don't think that syntax would ever work in the compiled program
itself.  It's just that in GDB we need a syntax to do additional things
that are not possible in the code itself.  For example, as Keith showed,
to print the value of a static variable placed inside a function:

  (gdb) print foo()::the_static_variable

I don't think this syntax is standardized or specified anywhere, it's
just the syntax GDB has chosen.  But again, the fact c++filt produces
the same kind of syntax makes it somewhat consistent across the
toolchain.

Note that this syntax isn't completely bullet-proof.  If you define two
structures of the same name in a function, like this:

    int main(int argc, char *argv[])
    {
        {
          struct foo { void bar () {} };
          foo().bar();
        }
        {
          struct foo { void bar () {} };
          foo().bar();
        }
      return 0;
    }

The two produced symbols for the bar methods are:

  - _ZZ4funcvEN3foo3barEv
  - _ZZ4funcvEN3foo3barE_0v

which c++filt both translate to

  func()::foo::bar()

>> In this particular example, I kinda like the output from clang, it's
>> just more precise.  I might change opinion once I will see an
>> example with many parameters and template that will make a monster of a
>> line.
>>
> 
> Yes that is one of my fears, plus large call stacks with a lot of frames.
> It is also inconsistent right now, as it only uses the full name for frame 0,
> but not for frame 1.

Huh!  So, if frame #0 is a method of a function-local struct, it will
show the full name, but if it's frame #1 that is a method of a
function-local struct, it will show the short name?  If so we'll
definitely want to fix that (and have a test for that).

>>> I couldn't find the reason for the output though. The Dwarf looks similar.
>>> And my patch makes no difference here.
>>
>> I don't know off-hand, and I don't have time to investigate.  But
>> getting the same behavior out of the two compilers would be desirable.
>> Does you patch accomplish that?
>>
> 
> No, Clang and g++ output are still different. That is what I meant with my
> patch makes no difference. The output is the same for both, with or without my
> patch.

Ok.

>> I do think that showing "class F1::F2" is wrong for sure.  It would be
>> the right thing to show for for:
>>
>>   struct F1 {
>>     struct F2 {
>>     }
>>   }
>>
>> I'd just like to have a third opinion on the matter.  If others think
>> that showing the short name is fine, then that's fine with me too.
>>
>> Simon
> 
> I am happy about other opinions as well!

What you are showing is that things are really not consistent in this
area.  Making things consistent (e.g. always show the short name) and
ensuring they are well tested would already be a good improvement, I
would be happy with merging this.

Simon

RE: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Willgerodt, Felix]

> >> I'd just like to have a third opinion on the matter.  If others think
> >> that showing the short name is fine, then that's fine with me too.
> >>
> >> Simon
> >
> > I am happy about other opinions as well!
> 
> What you are showing is that things are really not consistent in this area.
> Making things consistent (e.g. always show the short name) and ensuring
> they are well tested would already be a good improvement, I would be
> happy with merging this.
> 
> Simon

It has been two weeks. Should I just merge this, as there hasn't been any comments?
I think it brings a small improvement. I unfortunately didn't have any time to dig deeper
into the other inconsistencies we discussed here.

Regards,
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

Re: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Simon Marchi]

On 2021-07-13 3:42 a.m., Willgerodt, Felix wrote:
> It has been two weeks. Should I just merge this, as there hasn't been any comments?
> I think it brings a small improvement. I unfortunately didn't have any time to dig deeper
> into the other inconsistencies we discussed here.

Yes, this is fine with me, if I remember correclty there wasn't any more
unresolved issues.

Simon

RE: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to get a prefix.

[Willgerodt, Felix]

> -----Original Message-----
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Sent: Dienstag, 13. Juli 2021 15:02
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH] gdb, dwarf: Don't follow the parent of a subprogram to
> get a prefix.
> 
> On 2021-07-13 3:42 a.m., Willgerodt, Felix wrote:
> > It has been two weeks. Should I just merge this, as there hasn't been any
> comments?
> > I think it brings a small improvement. I unfortunately didn't have any
> > time to dig deeper into the other inconsistencies we discussed here.
> 
> Yes, this is fine with me, if I remember correclty there wasn't any more
> unresolved issues.
> 
> Simon

Thanks, I pushed it!

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