[Keith Seitz] Re: [tools-team] Status 2008-09-01

[Keith Seitz] Re: [tools-team] Status 2008-09-01

[Tom Tromey]

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]

Keith sent this useful email privately.  It details some areas of
investigation relating to virtual methods.  I'm forwarding it with his
permission, so we can have it in the archives.


[-- Attachment #2: Type: message/rfc822, Size: 6928 bytes --]

From: Keith Seitz <keiths@redhat.com>
To: Chris Moller <cmoller@redhat.com>
Cc: Tom Tromey <tromey@redhat.com>
Subject: Re: [tools-team] Status 2008-09-01
Date: Tue, 01 Sep 2009 14:55:11 -0700
Message-ID: <4A9D983F.5010707@redhat.com>

On 09/01/2009 02:15 PM, Chris Moller wrote:
> I kinda back-burnered that--to quote Keith, "I'm not even sure I
> completely understand what this task is," but I'll see what I can do
> with it.

For me, the biggest questions lie around virtual (pure and otherwise) 
classes and virtual methods -- printing, listing, breaking/running, 
variable/stack inspection. I've added none of this to realcpp yet. [And 
as you can imagine, gdb's own c++ test suite doesn't even come close to 
testing any real-world c++ usage.]

I'd like to see how gdb handles something like this (very contrived, 
very stupid example):

#include <iostream>

using namespace std;

template <typename T>
class virt_math
{
public:
   virtual T add (T num) = 0;
   virtual T sub (T num) = 0;
   virtual T mul (T num) = 0;
   virtual T div (T num) { return static_cast<T> (-3); } // Yes, I know 
this is really evil!
};

class base_int_math : public virt_math<int>
{
public:
   base_int_math (int a) : num_ (a) { };
   virtual ~base_int_math (void) { };

   virtual int add (int num) { return num_ + num; }
   int sub (int num) { return num_ - num; }
   virtual int mul (int num) { return num_ * num; }

protected:
   int num_;
};

class special_int_math : public base_int_math
{
public:
   special_int_math (int a) : base_int_math (a) { }
   int add (int num) { return num_ + num + 100; }
   int mul (int num) { return num_ * num + 100; }
};

int
main (int argc, char* argv[])
{
   virt_math<int> *obj = new special_int_math (2);
   cout << "obj.add (2) = " << obj->add (2) << endl;
   cout << "obj.sub (2) = " << obj->sub (2) << endl;
   cout << "obj.mul (2) = " << obj->mul (2) << endl;
   cout << "obj.div (2) = " << obj->div (2) << endl;
   delete obj;

   return 0;
}

In main, doing "print *obj" isn't particularly useful IMO (I don't know 
if we could do better, though):
(gdb) p *obj
$1 = {_vptr.virt_math = 0x8048c68}

I don't think this is very useful, either:

(gdb) ptype obj
type = class virt_math<int> {
   public:
     virtual int add(int);
     virtual int sub(int);
     virtual int mul(int);
     virtual int div(int);
} *

IMO, gdb should at least be able to tell us that obj is really a 
special_int_math object using RTTI or something.

I'm also not so sure about this:

(gdb) p special_int_math::add
Cannot reference virtual member function "add"
(gdb) p special_int_math::mul
Cannot reference virtual member function "mul"
(gdb) p special_int_math::div
Cannot reference virtual member function "div"
(gdb) p special_int_math::sub
Cannot reference virtual member function "sub"
(gdb) p base_int_math::add
Cannot reference virtual member function "add"
(gdb) p base_int_math::sub
Cannot reference virtual member function "sub"
(gdb) p base_int_math::div
Cannot reference virtual member function "div"
(gdb) p base_int_math::mul
Cannot reference virtual member function "mul"

IMO, we should be able to figure these things out.

I also don't think this is correct (or at least user-friendly):

(gdb) break virt_math<int>::mul
the class virt_math<int> does not have any method named mul
Hint: try 'virt_math<int>::mul<TAB> or 'virt_math<int>::mul<ESC-?>
(Note leading single quote.)
(gdb) ptype virt_math<int>
type = class virt_math<int> {
   public:
     virtual int add(int);
     virtual int sub(int);
     virtual int mul(int);
     virtual int div(int);
}

The ptype here should mention that most of these class members are pure 
virtual (=0). As it is, it does not tell you.

I also don't like the error messages here:

(gdb) b virt_math<int>::add
the class virt_math<int> does not have any method named add
Hint: try 'virt_math<int>::add<TAB> or 'virt_math<int>::add<ESC-?>
(Note leading single quote.)
(gdb) b virt_math<int>::sub
the class virt_math<int> does not have any method named sub
Hint: try 'virt_math<int>::sub<TAB> or 'virt_math<int>::sub<ESC-?>
(Note leading single quote.)
(gdb) b virt_math<int>::div
Breakpoint 1 at 0x8048acb: file virt_math.cc, line 12.

It's not that virt_math<int> /doesn't/ have methods named "add", "mul", 
and "sub", but that these are pure virtual. This is, IMO, misleading to 
the user, especially since "ptype" makes no distinction between these 
methods and "div" (which is virtual, but not "pure" virtual).

This one is a little less cut-n-dry IMO, but it should still "work":
(gdb) p obj->num_
There is no member or method named num_.

This isn't entirely correct. Yes, "obj" *is* literally a virt_math<int> 
(which contains no fields), but it *is* also a C++ class for which we 
should be able to figure out that it is *really* a special_int_math, and 
therefore num_ *is* a member of that class.

There's also the question about virtual inheritance (realcpp has an 
example): does printing/ptyping these things do something sane and 
USEFUL for the user? I imagine we're going to get multiple vtables. Blah.

Just some of the things that I've been wanting to investigate a bit, but 
haven't gotten around to it.

Keith


[-- Attachment #3: Type: text/plain, Size: 6 bytes --]



Tom

Re: [Keith Seitz] Re: [tools-team] Status 2008-09-01

[Jan Kratochvil]

On Wed, 02 Sep 2009 21:44:06 +0200, Tom Tromey wrote:
> From: Keith Seitz <keiths@redhat.com>
[...]
> This one is a little less cut-n-dry IMO, but it should still "work":
> (gdb) p obj->num_
> There is no member or method named num_.

Just this is due the wrong printing defaults:

(gdb) p obj->num_
There is no member or method named num_.
(gdb) set print object on 
(gdb) p obj->num_
$1 = 2

One should change this (+some other related options in `user_print_options')
and in some way fix the testsuite regressions afterwards by one of:
(a) Updating the specific testsuite expectations by hand.
(b) Making the "old" options default globally (deprecated)
(c) Making the "old" options default only in the current testcases.


Regards,
Jan

Re: [Keith Seitz] Re: [tools-team] Status 2008-09-01

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Just this is due the wrong printing defaults:
Jan> (gdb) p obj->num_
Jan> There is no member or method named num_.
Jan> (gdb) set print object on 
Jan> (gdb) p obj->num_
Jan> $1 = 2

I'm really surprised that "set print object" affects this.

It seems to me that this should always follow the language rules, and
that "obj->num_" should therefore be an error.  (I used to hold the
opposite position for Java, funnily enough, but I have come around.)

Instead I would expect this to work:

(gdb) set print object on
(gdb) print *obj
$1 = ... # something of type special_int_math
(gdb) print $.num_
$2 = whatever


That is, I think "set print object on" ought to affect the type of the
resulting history variable -- but not the type of any intermediate
values in an expression.

Jan> One should change this (+some other related options in
Jan> `user_print_options') and in some way fix the testsuite regressions
Jan> afterwards by one of:

I agree, we should change this default.

Tom

Re: [Keith Seitz] Re: [tools-team] Status 2008-09-01

[Tom Tromey]

>>>>> "Dragos" == Dragos Tatulea <dragos.tatulea@gmail.com> writes:

Tom> This seems like a generic bug, though.  So, it could be fixed either
Tom> separately or at the same time the default is flipped.

Dragos> Why is this a bug? Isn't objectprint supposed to follow up to
Dragos> the original/true type?

I see what you mean.

But, it seems like it would be a little more user-friendly to print the
original type name if we examine the runtime type and find that it is
identical to the declared type (after typdef stripping).

Dragos> Maybe it's not such a good idea to show up in mid struct. Maybe
Dragos> display the usual output and then print warnings on following
Dragos> lines. Dunno' if it's actually possible (noob alert!).

Probably not easily, but I'm sure it could be managed.
But, when printing a long structure this is likely to be more confusing.

Tom

Re: [Keith Seitz] Re: [tools-team] Status 2008-09-01

[Dragos Tatulea]

Hi,

> Dragos> I changed objectprint to on by default on Jan's suggestion, but
> Dragos> this breaks some other cases like this one (from ptr-typedef
> Dragos> test)
>
> Dragos> gdb> p foz_ptr
> Dragos> $1 = (struct foo *) 0x0
> Dragos> instead of
> Dragos> (foz*)
>
> This seems like a generic bug, though.  So, it could be fixed either
> separately or at the same time the default is flipped.
>
Why is this a bug? Isn't objectprint supposed to follow up to the original/true
type?

> Dragos> Back to the previous case: Jan suggested printing an
> Dragos> error/warning for the user saying that the ptr has a different
> Dragos> type (and maybe printing the type). What do you think?
>
> I think it depends on how often the warning would trigger, and where it
> would be printed (e.g., would it show up mid-struct?).
>
Maybe it's not such a good idea to show up in mid struct. Maybe display the
usual output and then print warnings on following lines. Dunno' if it's actually
possible (noob alert!).

-- Dragos

Re: [Keith Seitz] Re: [tools-team] Status 2008-09-01

[Tom Tromey]

>>>>> "Dragos" == Dragos Tatulea <dragos.tatulea@gmail.com> writes:

Dragos> I changed objectprint to on by default on Jan's suggestion, but
Dragos> this breaks some other cases like this one (from ptr-typedef
Dragos> test)

Dragos> gdb> p foz_ptr
Dragos> $1 = (struct foo *) 0x0
Dragos> instead of
Dragos> (foz*)

This seems like a generic bug, though.  So, it could be fixed either
separately or at the same time the default is flipped.

Dragos> Back to the previous case: Jan suggested printing an
Dragos> error/warning for the user saying that the ptr has a different
Dragos> type (and maybe printing the type). What do you think?

I think it depends on how often the warning would trigger, and where it
would be printed (e.g., would it show up mid-struct?).

Tom

Re: [Keith Seitz] Re: [tools-team] Status 2008-09-01

[Dragos Tatulea]

Hi Keith,

> That is, I think "set print object on" ought to affect the type of the
> resulting history variable -- but not the type of any intermediate
> values in an expression.
>
> Jan> One should change this (+some other related options in
> Jan> `user_print_options') and in some way fix the testsuite regressions
> Jan> afterwards by one of:
>
> I agree, we should change this default.

I changed objectprint to on by default on Jan's suggestion, but this breaks some
other cases like this one (from ptr-typedef test)

struct foo {
  int x;
};

typedef struct foo *foz;

int
main (void)
{
  foz_ptr = NULL;
}

gdb> p foz_ptr
$1 = (struct foo *) 0x0
instead of
(foz*)

So the change isn't acceptable.

Back to the previous case: Jan suggested printing an error/warning for the user
saying that the ptr has a different type (and maybe printing the type). What do
you think?

Thanks,
Dragos