Re: [PATCH] Use visitors for make_gdb_type

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 2018-01-29 10:31 AM, Alan Hayward wrote:
>> On Sun, 28 Jan 2018 21:24:19 -0500
>> Simon Marchi <simark@simark.ca> wrote:
>>
>>> On 2018-01-26 10:30 AM, Alan Hayward wrote:
>>>> I appear to still have email issues - previous post had the tabs stripped out.
>>>> Hoping this version is ok. Apologies.  
>>>
>>> Hi Alan,
>>>
>>> I was able to apply it correctly.
>>>>
> 
> Very strange! Think I’m ok now.

Well, now I am unable to apply this one :(.  The message body is encoded in base64,
I tried to decode it and apply it with git-apply, but it doesn't apply, not sure why.
git-send-email is really the safest way.

>>>> +    void visit_pre (const target_desc *e)
>>>> +    {}  
>>>
>>> I think we should have empty default implementations of these visit functions in the
>>> base class, instead of having to declare them empty when unnecessary.  Maybe Yao had
>>> a reason not to do this initially?
>>>
> 
> I think Yao didn’t add the method because tdesc_element_visitor is meant to be an interface
> and remain abstract.

A type can be abstract but have implementations for some of its methods.  To make a type abstract,
it is common to make the destructor pure virtual, if no other method is pure virtual.

> I considered putting the types into a parent class of tdesc_element_visitor, but that breaks
> the accept() functions horribly.
> Instead, I’ve created a new subclass from tdesc_element_visitor called tdesc_element_type_visitor
> which provides null implementations for all the non type visit functions. gdb_type_creator can
> now inherit from tdesc_element_type_visitor and only has to provide the three visitors.
> 
> Are you happy with that?

That seems like unnecessary boilerplate to me.  I really don't see why classes derived
from tdesc_element_visitor have to implement methods for nodes they don't care about.

I added Yao in CC so he can chime in.

Simon