Re: [PATCH 2/6] invoke_xmethod & array_view

[Pedro Alves <palves at redhat dot com>]

On 11/26/2018 05:18 PM, Simon Marchi wrote:

> I acknowledge all these downsides.  My opinion (as of today) is that they are reasonable trade-offs.  I'm in the camp of "code is written once, read many times".  I think that when writing it, it's not difficult to look up what the function you are calling expects (can a pointer be null).  But when reading a function, if the little & sign can save a round trip to the called function's doc, I think it's worth it.

In the patch series in question, do you think it helps?  IMHO it
doesn't.  Passing a vector by copy would be so strange that I'd
never think that that is what is going on.  Which leaves non
or non-const references.  And then if one has to look up
the function description to know the function is adding to the vector,
then that means that one doesn't really have a clue what the function
is doing, IMO.  Once you know, then you just know and the & doesn't
really help.

John's series is also passing output vectors by non-const reference:

 https://sourceware.org/ml/gdb-patches/2018-11/msg00171.html

and that code looks pretty clear to me as is.

Maybe it's particular to vectors/containers, not sure.  Maybe
a prototype like:
  void foo (int i, int &bar, float f, S *);
would be more confusing to me.  Can't say either, I guess because
I'm not seeing myself writing a function with such a prototype.

> 
>> So in sum, I nowadays tend to look at reference vs parameter more from
>> the "pointer: optional, can be NULL", vs "reference: non-optional" angle.
>> Though, given GDB's history, we definetely use pointers pervasively
>> in function parameters even when they're not supposed to be optional,
>> that's for sure.
> 
> For "in" parameters, I think it's a no-brainer to do that.
> 
>> Maybe we could come up with some middle ground rule, like always
>> putting in-out and output parameters last, and stressing to use
>> clear symbol names.
> 
> Hmmm maybe, I would have to see it in practice to judge how feasible it is.
>> Anyway, I don't want to block on that discussion (my 1 month round trip
>> time not helping! :-D), so I did the change.
> 
> Thanks!  Note that I have introduced this rule kind of unilaterally after having no response when I proposed it:
> 
> https://sourceware.org/ml/gdb-patches/2018-03/msg00145.html
> https://sourceware.org/ml/gdb-patches/2018-05/msg00450.html
> 
> I thought it was reasonable to do so because it's not a huge commitment and easily reversible if people ended up disagreeing.  So it's not as if it's set in stone.
Thanks,
Pedro Alves