Re: [RFC PATCH] ipa-prop.[ch] cleanup

["Diego Novillo" <dnovillo at google dot com>]

On Mon, Apr 7, 2008 at 14:57, Martin Jambor <mjambor@suse.cz> wrote:
>  4) The code used access getter and setter functions excessively, up to
>    a point  which made the code  terribly difficult to  read.  Most of
>    them  were  trivial and  useless,  there  was  for example  even  a
>    function returning cgraph_edge->callee for  a given cgraph edge.  I
>    strongly disliked this  approach (if we want to  have ultra OO-like
>    code, why don't we use an OO language in the first place?).  I have
>    removed most of these.

All of the cleanups are OK except this one.  You don't have to use an
OO language to write code with OO properties (the C++ discussion is
for another thread, I've lost that battle many times already).

There is also a fairly strong pragmatic reason for this.  In the
tuples branch, we are using getter/setter functions quite extensively.
 In more than a few occasions I have tracked down bugs fairly easily
by setting a breakpoint on a specific setter function when a statement
operand was being modified to the wrong value, for instance.

Yes, getter/setter functions can get in the way when you are stepping
through in gdb.  I hear that gdb should/will support the feature of
declaring opaque inline functions.

>  @@ -421,15 +430,15 @@ ipcp_method_cval_init (struct cgraph_nod
>    tree parm_tree;
>
>    ipcp_formal_create (mt);
>  -  for (i = 0; i < ipa_method_formal_count (mt); i++)
>  +  for (i = 0; i < IPA_NODE_REF (mt)->param_count ; i++)

This is what I don't like.  It's fine to turn ipa_method_formal_count
into a static inline.  Or rename it so it makes better sense.  But I
would like to move us away from direct field references as much as
possible.  The maintenance implications are also important for future
flexibility.


Diego.