Re: [patchv2] compile: Fix crash on cv-qualified self-reference

[Pedro Alves <palves@redhat.com>]

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