Re: [patch] testsuite/gdb.c++/local.exp: accept more nested types

Re: [patch] testsuite/gdb.c++/local.exp: accept more nested types

[Michael Elizabeth Chastain]

Daniel Jacobowitz writes:

mec> This patch updates local.exp to track an improvement in gdb.
mec> In a configuration with gcc HEAD -gstabs+, gdb prints nested types
mec> as "InnerLocal::NestedInnerLocal" instead of just "NestedInnerLocal".

dj> It's an improvement in GCC's debug output, I believe; I seem to recall
dj> Kevin posting a patch for this to gcc-patches recently.

I don't think it's a change in gcc because gcc does not show this change
with gdb 5.2 or gdb gdb_5_2-branch.  It happens only with gdb HEAD.

Although there have been changes in gcc this week in this area.  Perhaps
it really is a gcc change, and gdb HEAD is the only gdb looking at the
area of the change.

dj> Is it really?  It seems to me that we should strip off the name of the
dj> enclosing class from nested types.  This becomes more of an issue with
dj> the DWARF-2 type printing I'm experimenting with.

This is a philosophical issue.  Let's explore it.

gdb's output is:

  # target=native, host=i686-pc-linux-gnu%rh-7.2, gdb=HEAD%20020526
  # gcc=HEAD%20020526, binutils=HEAD%20020526, glibc=VENDOR,
  # goption=-gstabs+

  ptype InnerLocal
  type = class InnerLocal {
    public:
      char ilc;
      int *ip;
      InnerLocal::NestedInnerLocal nest1;

      InnerLocal & operator=(InnerLocal const&);
      InnerLocal(InnerLocal const&);
      InnerLocal();
      int il_foo(unsigned char const&);
  }

gdb presents me with this output.  What result should I assign to it?

I think this is a PASS.  I think that the output is clear to humans.
The construct "InnerLocal::NestedInnerLocal nest1" is also valid C++;
if you add back in the definition of NestedInnerLocal, then it will
compile.

I also accept "NestedInnerLocal nest1;" as a PASS.

Philosophically, I'm not interested in the "one true output".  I look
at what gdb actually prints in each configuration, and then classify
these into PASS and FAIL.  This is especially true for "ptype" output,
where the output depends on the compiler and debug format.

What do you think?

Michael C

Re: [patch] testsuite/gdb.c++/local.exp: accept more nested types

[Daniel Jacobowitz]

On Mon, May 27, 2002 at 01:27:53PM -0500, Michael Elizabeth Chastain wrote:
> Daniel Jacobowitz writes:
> 
> mec> This patch updates local.exp to track an improvement in gdb.
> mec> In a configuration with gcc HEAD -gstabs+, gdb prints nested types
> mec> as "InnerLocal::NestedInnerLocal" instead of just "NestedInnerLocal".
> 
> dj> It's an improvement in GCC's debug output, I believe; I seem to recall
> dj> Kevin posting a patch for this to gcc-patches recently.
> 
> I don't think it's a change in gcc because gcc does not show this change
> with gdb 5.2 or gdb gdb_5_2-branch.  It happens only with gdb HEAD.
> 
> Although there have been changes in gcc this week in this area.  Perhaps
> it really is a gcc change, and gdb HEAD is the only gdb looking at the
> area of the change.

Let's go with a working philosophy of "both" then :)

> dj> Is it really?  It seems to me that we should strip off the name of the
> dj> enclosing class from nested types.  This becomes more of an issue with
> dj> the DWARF-2 type printing I'm experimenting with.
> 
> This is a philosophical issue.  Let's explore it.

Indeed it is.

> gdb's output is:
> 
>   # target=native, host=i686-pc-linux-gnu%rh-7.2, gdb=HEAD%20020526
>   # gcc=HEAD%20020526, binutils=HEAD%20020526, glibc=VENDOR,
>   # goption=-gstabs+
> 
>   ptype InnerLocal
>   type = class InnerLocal {
>     public:
>       char ilc;
>       int *ip;
>       InnerLocal::NestedInnerLocal nest1;
> 
>       InnerLocal & operator=(InnerLocal const&);
>       InnerLocal(InnerLocal const&);
>       InnerLocal();
>       int il_foo(unsigned char const&);
>   }
> 
> gdb presents me with this output.  What result should I assign to it?
> 
> I think this is a PASS.  I think that the output is clear to humans.
> The construct "InnerLocal::NestedInnerLocal nest1" is also valid C++;
> if you add back in the definition of NestedInnerLocal, then it will
> compile.
> 
> I also accept "NestedInnerLocal nest1;" as a PASS.
> 
> Philosophically, I'm not interested in the "one true output".  I look
> at what gdb actually prints in each configuration, and then classify
> these into PASS and FAIL.  This is especially true for "ptype" output,
> where the output depends on the compiler and debug format.
> 
> What do you think?

I think that I (as C++ maintainer rather than testsuite maintainer)
need to figure out what the correct output is. 
"InnerLocal::NestedInnerLocal nest1" is not actually valid C++ in
general, I don't believe; what if there is a class
InnerLocal::InnerLocal::NestedInnerLocal?  I believe it will resolve to
that one instead of the correct one.  This is also uglier; for std::
types there's going to be a lot of useless noise.

The ideal output, it seems to me, would be something like:

>   ptype InnerLocal
>   type = class InnerLocal {
>     public:
>       char ilc;
>       int *ip;
>	class NestedInnerLocal;
>       NestedInnerLocal nest1;
> 
>       InnerLocal & operator=(InnerLocal const&);
>       InnerLocal(InnerLocal const&);
>       InnerLocal();
>       int il_foo(unsigned char const&);
>   }

Unambiguous, quite obviously correct.  By this logic,
InnerLocal::NestedInnerLocal deserves a KFAIL comment for the moment,
since the code to do this is not yet present.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch] testsuite/gdb.c++/local.exp: accept more nested types

[Michael Elizabeth Chastain]

Hi Daniel,

> I think that I (as C++ maintainer rather than testsuite maintainer)
> need to figure out what the correct output is. 

Well, actually, the manual should specify the output of ptype,
and you should implement the manual, and I should test to the manual.
Without a written spec, we have to thrash this out among ourselves.

> "InnerLocal::NestedInnerLocal nest1" is not actually valid C++ in
> general, I don't believe; what if there is a class
> InnerLocal::InnerLocal::NestedInnerLocal?

Fortunately for us, that is not legal C++.  gcc 2.96-rh and gcc 3.1
both give me a "duplicate nested type" error.

I admit this is a technicality.  But for purposes of human readability,
it means that "InnerLocal::NestedInnerLocal nest1" will behave according
to the expectations of a reasonable human reader.

> This is also uglier; for std:: types there's going to be a lot of
> useless noise.

It's useful noise if the user has their own type with the same name
as a std:: type.

> By this logic, InnerLocal::NestedInnerLocal deserves a KFAIL comment
> for the moment, since the code to do this is not yet present.

I don't think it's a failure at all.

Frankly, if we do decide it's a failure, I'd rather just leave it as
a FAIL for a while.  I don't want to add a bunch more regular expressions
and file a bug report just to make it a different kind of failure.

We're at an impasse here ... does anyone else have an opinion?

Michael C

[patch] testsuite/gdb.c++/local.exp: accept more nested types

[Michael Elizabeth Chastain]

This patch updates local.exp to track an improvement in gdb.
In a configuration with gcc HEAD -gstabs+, gdb prints nested types
as "InnerLocal::NestedInnerLocal" instead of just "NestedInnerLocal".
This output is okay.

I tested this on my native i686-pc-linux-gnu test bed
with 6 gcc's, both -gdwarf-2 and -gstabs+.

I'm committing this now.

Michael C

2002-05-27  Michael Chastain  <mec@shout.net>

	* gdb.c++/local.exp: Accept more nested types in output.

Index: gdb.c++/local.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/local.exp,v
retrieving revision 1.8
diff -c -3 -p -r1.8 local.exp
*** gdb.c++/local.exp	23 Apr 2002 05:17:57 -0000	1.8
--- gdb.c++/local.exp	27 May 2002 17:54:30 -0000
*************** gdb_expect {
*** 124,135 ****
  # Pattern 1:
  # PASS
  #   dwarf-2
! #     2.95.3, 2.96-rh, 3.0.4, gcc-3_1-branch, HEAD
  #
  # Pattern 2:
  # PASS
  #   stabs+
! #     2.95.3, 2.96-rh, 3.0.4, gcc-3_1-branch, HEAD
  #
  # Pattern 3:
  # Old hppa pattern.
--- 124,135 ----
  # Pattern 1:
  # PASS
  #   dwarf-2
! #     2.95.3, 2.96-rh, 3.0.4, 3.1, gcc-3_1-branch, HEAD
  #
  # Pattern 2:
  # PASS
  #   stabs+
! #     2.95.3, 2.96-rh, 3.0.4, 3.1, gcc-3_1-branch, HEAD
  #
  # Pattern 3:
  # Old hppa pattern.
*************** gdb_expect {
*** 137,148 ****
  # Pattern 4:
  # Old hppa pattern.
  #
! # chastain 2002-04-08
  
  send_gdb "ptype InnerLocal\n"
  gdb_expect {
!   -re "type = class InnerLocal \{\[\r\n\t \]*public:\[\r\n\t \]*char ilc;\[\r\n\t \]*int \\* *ip;\[\r\n\t \]*NestedInnerLocal nest1;\[\r\n\t \]*int il_foo\\((unsigned char const|const unsigned char) *&\\);\[\r\n\t \]*\}.*$gdb_prompt $" { pass "ptype InnerLocal" }
!   -re "type = class InnerLocal \{\[\r\n\t \]*public:\[\r\n\t \]*char ilc;\[\r\n\t \]*int \\* *ip;\[\r\n\t \]*NestedInnerLocal nest1;\[\r\n\t \]*InnerLocal *& operator *=\\((main${sep}::|)InnerLocal const *&\\);\[\r\n\t \]*InnerLocal\\((main${sep}::|)InnerLocal const *&\\);\[\r\n\t \]*InnerLocal\\((void|)\\);\[\r\n\t \]*int il_foo\\(unsigned char const *&\\);\[\r\n\t \]*\}.*$gdb_prompt $" { pass "ptype InnerLocal" }
    -re "type = class InnerLocal \{\r\n\[\t \]*public:\r\n\[\t \]*char ilc;\r\n\[\t \]*int \\*ip;\r\n\[\t \]*InnerLocal::NestedInnerLocal nest1;\r\n\r\n\[\t \]*.int il_foo\\(unsigned char const &\\);\r\n\[\t \]*\}\[\t \]*\\(Local at.*local\\.cc:36\\).*$gdb_prompt $" { pass "ptype InnerLocal (old HP aCC)" }
    -re "type = class InnerLocal \{\r\n\[\t \]*public:\r\n\[\t \]*char ilc;\r\n\[\t \]*int \\*ip;\r\n\[\t \]*class InnerLocal4::NestedInnerLocal nest1;\r\n\r\n\[\t \]*int il_foo\\(unsigned char const &\\);\r\n\[\t \]*\\(Local at.*local\.cc:\[0-9\]+\\)\r\n\}.*$gdb_prompt $" { pass "ptype InnerLocal (old HP aCC)" }
    -re ".*$gdb_prompt $"   {  fail "ptype InnerLocal" }
--- 137,148 ----
  # Pattern 4:
  # Old hppa pattern.
  #
! # chastain 2002-05-27
  
  send_gdb "ptype InnerLocal\n"
  gdb_expect {
!   -re "type = class InnerLocal \{\[\r\n\t \]*public:\[\r\n\t \]*char ilc;\[\r\n\t \]*int \\* *ip;\[\r\n\t \]*(InnerLocal::|)NestedInnerLocal nest1;\[\r\n\t \]*int il_foo\\((unsigned char const|const unsigned char) *&\\);\[\r\n\t \]*\}.*$gdb_prompt $" { pass "ptype InnerLocal (pattern 1)" }
!   -re "type = class InnerLocal \{\[\r\n\t \]*public:\[\r\n\t \]*char ilc;\[\r\n\t \]*int \\* *ip;\[\r\n\t \]*(InnerLocal::|)NestedInnerLocal nest1;\[\r\n\t \]*InnerLocal *& operator *=\\((main${sep}::|)InnerLocal const *&\\);\[\r\n\t \]*InnerLocal\\((main${sep}::|)InnerLocal const *&\\);\[\r\n\t \]*InnerLocal\\((void|)\\);\[\r\n\t \]*int il_foo\\(unsigned char const *&\\);\[\r\n\t \]*\}.*$gdb_prompt $" { pass "ptype InnerLocal (pattern 2)" }
    -re "type = class InnerLocal \{\r\n\[\t \]*public:\r\n\[\t \]*char ilc;\r\n\[\t \]*int \\*ip;\r\n\[\t \]*InnerLocal::NestedInnerLocal nest1;\r\n\r\n\[\t \]*.int il_foo\\(unsigned char const &\\);\r\n\[\t \]*\}\[\t \]*\\(Local at.*local\\.cc:36\\).*$gdb_prompt $" { pass "ptype InnerLocal (old HP aCC)" }
    -re "type = class InnerLocal \{\r\n\[\t \]*public:\r\n\[\t \]*char ilc;\r\n\[\t \]*int \\*ip;\r\n\[\t \]*class InnerLocal4::NestedInnerLocal nest1;\r\n\r\n\[\t \]*int il_foo\\(unsigned char const &\\);\r\n\[\t \]*\\(Local at.*local\.cc:\[0-9\]+\\)\r\n\}.*$gdb_prompt $" { pass "ptype InnerLocal (old HP aCC)" }
    -re ".*$gdb_prompt $"   {  fail "ptype InnerLocal" }

Re: [patch] testsuite/gdb.c++/local.exp: accept more nested types

[Daniel Jacobowitz]

On Mon, May 27, 2002 at 12:58:43PM -0500, Michael Elizabeth Chastain wrote:
> This patch updates local.exp to track an improvement in gdb.
> In a configuration with gcc HEAD -gstabs+, gdb prints nested types
> as "InnerLocal::NestedInnerLocal" instead of just "NestedInnerLocal".

It's an improvement in GCC's debug output, I believe; I seem to recall
Kevin posting a patch for this to gcc-patches recently.

> This output is okay.

Is it really?  It seems to me that we should strip off the name of the
enclosing class from nested types.  This becomes more of an issue with
the DWARF-2 type printing I'm experimenting with.


-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer