This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patchv2] compile: Fix crash on cv-qualified self-reference
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org, Phil Muldoon <pmuldoon at redhat dot com>
- Date: Wed, 01 Jul 2015 15:59:37 +0100
- Subject: Re: [patchv2] compile: Fix crash on cv-qualified self-reference
- Authentication-results: sourceware.org; auth=none
- References: <20150418172843 dot GA17777 at host1 dot jankratochvil dot net> <20150516132555 dot GB17266 at host1 dot jankratochvil dot net> <86lhf0p1hf dot fsf at gmail dot com> <20150701132406 dot GA13975 at host1 dot jankratochvil dot net> <5593F10D dot 4020903 at redhat dot com> <20150701141003 dot GA19545 at host1 dot jankratochvil dot net>
On 07/01/2015 03:10 PM, Jan Kratochvil wrote:
> On Wed, 01 Jul 2015 15:54:21 +0200, Pedro Alves wrote:
>> > On 07/01/2015 02:24 PM, Jan Kratochvil wrote:
>> >
>>> > > I can change it that way but when you ask "isn't cleaner" then no, I think
>>> > > your hack is even a bit more ugly than my ugly hack.
>>> > >
>>> > > There should be two virtual methods, one pure for 'switch (TYPE_CODE (type))'
>>> > > and the other one checking TYPE_INSTANCE_FLAG* in superclass overriden only by
>>> > > TYPE_CODE_STRUCT and TYPE_CODE_UNION (there would be no TYPE_CODE_*, though).
>> >
>> > What would be the method name?
> class Type {
> protected:
> virtual GccType convert_unqualified ()=0;
> public:
> explicit virtual GccType operator() {
> if (instance_flags==0) return convert_unqualified();
> ...
> }
> };
>
> class StructType:public Type {
> protected:
> virtual GccType convert_unqualified () { assert(0) }
> public:
> explicit virtual GccType operator() override { ... }
> };
>
> class IntegerType:public Type {
> protected:
> virtual GccType convert_unqualified () { assert(instance_flags==0); ... }
> };
>
Well, I'd say that having the core GDB Type be aware of GccType
directly would be a misdesign, not a feature.
> Althoughth qualifications could be possibly also subclassed which would look
> differently again.
Yes, the "possibly" here is the crux. Subclassing isn't always the
best choice. There are trade offs.
>> > There's nothing preventing adding a new type_FOO function that takes a type
>> > pointer as parameter and hides the TYPE_CODE checks inside. From the
>> > caller's perspective, it'll be the same. Once we get to C++ and if we
>> > consider objectifying type, then converting that function to a method will
>> > be trivial.
> Do you mean simulation of C++ virtual method table by a struct of pointers,
> like in other cases in GDB?
>
No. I meant a straightforward conversion of your C++ methods
to C functions implemented in terms of switch/TYPE_CODE.
IIUC, in your C++ class tree you'd do:
gcctype = (GccType) type;
So a trivial 1-1 conversion or your code would be:
// Type::convert_unqualified ()
// StructType::convert_unqualified ()
gcc_type
type_convert_unqualified (struct type *)
{
switch (TYPE_CODE (type))
{
case TYPE_CODE_STRUCT:
assert(0);
default:
...
}
}
// Type::GccType operator()
gcc_type
type_as_gcc_type (struct type *type)
{
if (TYPE_INSTANCE_FLAGS (instance_flags) == 0)
return type_convert_unqualified (type);
...
}
Then the caller in question would use:
gcctype = type_as_gcc_type (type);
I'm very much looking forward to C++ as well, but switch/case vs
virtual methods here is really more about syntax sugar than design.
You can easily end up with a broken class inheritance tree
just as well. There's a lot more to it beyond design than language.
Thanks,
Pedro Alves