Re: [Opencxx-users] Makefile.am fixes

[Grzegorz J. <ja...@he...>]

On Tue, 16 Sep 2003, Yann Dirson wrote:

> On Tue, Sep 16, 2003 at 09:30:18AM +0800, Grzegorz Jakacki wrote:
> > > Yes, but that makes a modified copy of the original code, and will require
> > > updating to take advantage of any changes in the original...
> >
> > You are right. I would say that this is acceptable on this scale, but that's
> > maybe because I am getting old. Let's try again.
>
> On the scale of TranslateCast() I tend to agree (so perhaps I'm getting old
> myself as well ;), but I only used this one as an example of what I'd see
> useful with most Walker::Translate*() methods.

I see.

> > I understand that you would like to add BeforeTransalteCast(),
> > AfterTranslateCastedExpression(), BeforeTransalteCastedExpression(),
> > TranslateCastType() and TranslateCastType() to the public interface of
> > Walker. I would like to be careful about it, as changes to interfaces are
> > difficult to reverse.
> >
> > First reservation is: why do we need FIVE new interface functions? It
> > seems to me that at most two should be enough.
>
> Your demonstration below is convincing to me :)
>
> > Too many functions makes interface diffucult to comprehend.
>
> Sure, and furthermore I think the resulting code would be much slower.

I agree, but I don't think we should worry about the performance unless
this becomes an issue (e.g. somebody presents an example that runs
unacceptably slow).

> > Second: function like 'BeforeTranslateCast()' seems to be a bad candidate for public
> > iface function, as it has very complicated postcondition. If you do not
> > return some very specific Ptree, the remaining code will fail. This has to
> > be documented and understood by the client, so I would like to avoid it if
> > possible.
>
> Yes, probably its call would need to be wrapped in a check that would raise
> an exception if the postcondition is violated.  And that again would make
> the code much slower.
>
> > >From what I can see the problem originates from the fact, that
> > 'TranslateCast()' violates Single Responsibility Principle.
> [...]
> > So maybe we can go on with something like this:
> [...]
>
> That looks fine to me.  Probably most other Translate*() methods would
> benefit from such a change.

I understand that you have some specific requirements in your project that
expose a need for certain functionality in Walker and that
'TranslateCast()' is an just one example. Could you post the list of
extensions that you would like to introduce?

> > Perhaps we also can give more meaningful types to arguments of
> > TranslateCastedExpression() and TranslateCastedType().
> >
> > What do you think?
>
> That may make sense as well.  Looks like it could help to catch silly
> mistakes when writing a custom walker.
>
> > > > Looks like we came to similar conclusions.
> > > >
> > > > The code that I wrote above works, but it violates Liskov Substitution
> > > > Principle --- the user cannot be sure about the behaviour of member
> > > > function 'TranslateCast()' when it is called via Walker*.
> > >
> > > Hm - what assumptions of Walker would this code break ?
> >
> > See
> [...]
>
> I already looked up the lsp to be sure about what you meant.  As I
> understand it it refers to derivative classes causing assumptions about the
> base class not to be valid any more on the derivative class.  What I fail to
> see is which assumptions about the Walker class would the
> MyWalker::TranslateCast method as described break.

E.g. "Walker::TranslateCast() does not modify the type in a cast".

As I understand it, LSP requires that object behaviour is determined by a
type of pointer to it. With 'Walker : public MyWalker' given the 'Walker*'
pointer I cannot be sure about the behaviour of pointee --- it may happen,
that it is a 'MyWalker' object, which differs wrt. the semantics of
'TranslateCast()'.

With decomposition into 'Walker' (abstract iface) and 'OpencxxWalker'
(implementation) I can can be sure that whatever (derived) object is at
the end of 'OpencxxWalker*' pointer, it behaves just like 'OpencxxWalker'
object.

On the other hand (new) 'Walker' is abstract, so there is nothing
like "Walker object", consequently 'OpencxxWalker*' pointer does not
imply any assumptions about the pointee behaviour.

Surely current walker hierarchy in OpenC++ violates LSP, but it should
not take much to get it straight.

Have I convinced you?

Greetings
Grzegorz

##################################################################
# Grzegorz Jakacki                       Huada Electronic Design #
# Senior Engineer, CAD Dept.              1 Gaojiayuan, Chaoyang #
# tel. +86-10-64365577 x2074               Beijing 100015, China #
# Copyright (C) 2002 Grzegorz Jakacki, HED. All Rights Reserved. #
##################################################################