On Tue, May 13, 2014 at 3:27 PM, David Malcolm <dmalc...@redhat.com> wrote:
> On Tue, 2014-05-13 at 15:10 +0200, Richard Biener wrote:
>> On Tue, May 13, 2014 at 2:37 PM, Michael Matz <m...@suse.de> wrote:
>> > Hi,
>> >
>> > On Mon, 12 May 2014, David Malcolm wrote:
>> >
>> >> The "gfoo" type names are pleasantly terse, though I'm a little unhappy
>> >> about how they no longer match the prefix of the accessor functions e.g.
>> >> gimple_switch_num_labels (const gswitch *gs)
>> >> vs
>> >> gimple_switch_num_labels (const gimple_switch *gs)
>> >> But it works.
>> >
>> > That could also be changed with a followup to make it consistent again
>> > (i.e. rename the accessors to gswitch_num_labels). I'd be in favor of
>> > such renaming later.
>>
>> Yeah, or go all the way to member functions.
>>
>> I'd like to see this addresses as followup, together with a discussion
>> on whether we want standalone or member functions here.
>
> Or both, perhaps as a transition strategy e.g. something like:
>
> struct GTY((tag("GSS_WITH_OPS")))
> gswitch : public gimple_with_ops
> {
> public:
> unsigned num_labels () const
> {
> unsigned num_ops = num_ops ();
> gcc_gimple_checking_assert (num_ops > 1);
> return num_ops - 1;
> }
> };
>
> static inline unsigned
> gimple_switch_num_labels (const gswitch *gs)
> {
> return gs->num_labels ();
> }
>
> where the accessor function simply calls the member fn??? This
> both-worlds approach might not be desirable as a end-goal, but might be
> a useful tactic when creating the patches.
Well, we can then also simply rename the functions first. And not bother
touching each line twice eventually.
> FWIW, if we want to do a grand renaming - which would touch most lines
> in the middle-end - I'd prefer to go the whole way and have member
> functions (though the gimple_ -> gswitch_ would be much easier to
> automate :) )
>
> I can have a look at making the data fields be private too, I guess.
> (with judicious use of "friend", it may be possible to do that patch
> without needing to make the accessors be members).
>
> An idea I had for a method-naming convention: keep the type as a prefix
> of the name, so that instead of:
> (A) tree label = gimple_switch_label (switch_stmt, i);
> becoming:
> (B) tree label = switch_stmt->label (i);
> you'd have:
> (C) tree label = switch_stmt->switch_label (i);
> ^^^^^^^ note the prefix
> which might allow us to keep most of the "grepability" of the existing
> code (you can't easily find "label" *method* calls in approach (B),
> whereas "switch_label" should give much better results in approach (C)).
Eh :/ But then we can as well keep the function as gswitch_label
(switch_stmt, i).
C++ isn't really a good greppable language. But well. We voted for it.
Richard.
> (and perhaps omit the prefix when it's unambiguous e.g.
> gimple_cond_make_false would become
> simply cond_stmt->make_false () ?)
>
> Dave
>
>
