Re: [PATCH] Trivial cleanup

[Andrew MacLeod <amacleod at redhat dot com>]

On 09/27/2013 01:03 AM, Jeff Law wrote:
> On 09/26/2013 08:15 AM, Michael Matz wrote:
> > Hi,
> >
> > On Wed, 25 Sep 2013, Jeff Law wrote:
> >
> > > > > I was going to bring it up at some point too.  My preference is
> > > > > strongly to simply eliminate the space on methods...
> > > > Which wouldn't be so weird: in the libstdc++-v3 code we do it all 
> > > > the time.
> > > Yea.  I actually reviewed the libstdc++ guidelines to see where they 
> > > differed
> > > from GNU's C guidelines.
> > >
> > > I'm strongly in favor of dropping the horizontal whitespace between the
> > > method name and its open paren when the result is then dereferenced.
> > > ie foo.last()->e rather than foo.last ()->e.
> >
> > I'd prefer to not write in this style at all, like Jakub.  If we must
> > absolutely have it, then I agree that the space before _empty_ 
> > parentheses
> > are ugly if followed by references.  I.e. I'd like to see spaces before
> > parens as is customary, except in one case: empty parens in the 
> > middle of
> > expressions (which don't happen very often right now in GCC, and hence
> > wouldn't introduce a coding style confusion):
> >
> > do.this ();
> > give.that()->flag;
> > get.list (one)->clear ();
> >
> > I'd prefer to not have further references to return values be applied,
> > though (as in, the parentheses should be the end of statement), which
> > would avoid the topic (at the expensive of having to invent names for
> > those temporaries, or to write trivial wrapper methods contracting 
> > several
> > method calls).
> Should we consider banning dereferencing the result of a method call 
> and instead prefer to use a more functional interface such as Jakub 
> has suggested, or have the result of the method call put into a 
> temporary and dereference the temporary.
>
> I considered suggesting the latter.  I wouldn't be a huge fan of the 
> unnecessary temporaries, but they may be better than the horrid 
> foo.last()->argh()->e->src or whatever.
>
> Stuffing the result into a temporary does have one advantage, it 
> encourages us to CSE across the method calls in cases where the 
> compiler might not be able to do so.  Of course, being humans, we'll 
> probably mess it up.
>
> jeff
I don't like the more functional interface... I thought the suggestion 
might be a little tongue in cheek, but wasn't sure :-)  I can't imagine 
the number of templates that would introduce... and the impact on 
compile/link time would probably not be trivial.

temps would be OK with me, but there are a couple of concerns.
 - I'd want to be able to declare the temps at the point of use, not 
the top of the function. this would actually help with clarity I think. 
Not sure what the current coding standard says about that.
 - the compiler better do an awesome job of sharing stack  space for 
user variables in a function... I wouldn't want to blow up the stack 
with a bazillion unrelatd temps each wit their own location.

My example in this form would look something like:
int unsignedsrcp = ptrvar.type().type().type_unsigned();

<...>
GimpleType t1 = ptrvar.type ();
GimpleType t2 = t1.type ();
int unsignedsrcp = t2.type.unsigned ();

And yes, we'll probably introduce the odd human CSE error.. hopefully 
the test suite will catch them :-)

I think I still prefer matz's suggestion, but I could be on board with 
this one too.   some expressions are crazy complicated

Andrew