Re: [pygccxml-development] Function Transformation - comments

[Matthias B. <ba...@ir...>]

Roman Yakovenko wrote:
> 0. small but important issue: in some places within code you use
>     next convention: wrapperfunc or virtualfunc. Can you separate words 
> by '_'? Thanks.

ok

> 1. code_creators.mem_fun_v_t and mem_fun_v_wrapper_t
>     I think that for a time being it is better to derive another class 
> from calldef_t and
>     calldef_wrapper_t. There are few reasons I think why it should be 
> done this way:
>     1. safety - we can play as much as we want, without breaking 
> existing code
>     2. you added a lot of code to the classes, I think we will better 
> understand the
>         whole feature if it will be separated from the existing code.
> 
>    So I did it for you. I did not add the code you send.

I didn't do an update yet because I think this will lead to quite a few 
conflicts now (I've noticed that you modified calldef.py as well).
So how do you want me to proceed? Stick all my stuff into the 
mem_fun_v_transformed_* classes?
Are you planning on keeping those classes (transformed and 
non-transformed versions) separate permanently or is this only for as 
long as this is work in progress?

By the way, I think the "standard" way to approach this problem would 
have been to create a branch 
(http://svnbook.red-bean.com/nightly/en/svn.branchmerge.html). But the 
repository layout doesn't seem to be prepared for this option.

> 2. I don't understand the role of substitution_manager_t, but I'd like 
> to comment
>    on it's implementation. Why don't you use dictionary and standard 
> Python string
>    substitution mechanism?
> 
>    "%(return_)s" % dict( return_="123" )

I didn't use this because there are some variables that may contain an 
entire block of source code (i.e. multiple lines) and each line should 
be indented with the same depth as the variable in the template string.
Using the Python formatting functionality only the first line would have 
been indented.

> 3. I don't understand subst_t hierarchy.  For example code_manager_t 
> derives from
>     subst_t.  subst_t class perform text substitutions, while 
> code_manager_t manages
>     pieces of source code for a C++ function. It seems that subst_t 
> class should be an
>     implementation details of code_manager_t class.

You mean I should have used delegation instead of derivation?
Well, I was considering this for a moment and decided for derivation 
because...

- ...the code manager is also supposed to do substitutions but with a 
specialized set of variables. So the "is a" relationship is given.
- ...I would have to duplicate the definition of substitute() when using 
delegation.
- ...the user still has the option to attach variables right to the code 
manager and use those for substitutions (it's a rather special case but 
who knows when it might be handy). With delegation this option would 
have to be provided explicitly (with additional code).

> 4. function_transformer_t completely operates on string level. It does 
> not even contain
>     reference to declaration it works on.

Yes?
(at least in my cases, I never needed to know the declaration)

> I expected to see something completely different. I thought about 
> defining hierarchy of
> "transformation" classes, then the role of function_transformer_t could 
> be an
> aggregation of all transformations that user want to do for the function 
> to convert it
> to another function ( 1:1 ). 

The function transformers are individual and independent pieces of code. 
The aggregation is done by the substitution manager.

> Obviously, user will have to write some 
> "glue code" to help
> all these transformations to work together and we should find the right 
> place to it.

What "glue code" do you mean? As mentioned above, it's the substitution 
manager that "glues" together the individual pieces.

> Also, what I did not see is responsibilities and contracts. I mean you 
> wrote documentation
> but it is not clear from the class interfaces.

What parts of the interfaces didn't you understand?

> 1. Why do you start to count arguments from 1 and not from zero?

An index of 0 occasionally refers to the return value.

> 2. Why Output is not according to coding convention?

Well, I left this one for the time being as this is actually a class 
that belongs into the "high level" realm and the conclusion in the wiki 
on this topic was to use the Python style guide.


- Matthias -