Re: [PATCH 2/6] invoke_xmethod & array_view

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2018-11-26 12:54, Pedro Alves wrote:
> 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.

It's hard to tell now because as you say, once I know, I know.  And I 
happen to know now :).  Also, this is a case of

    did_it_work = get_stuff (stuff);

So in this case, I admit it would be clear either way.

> 
> 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.

There, it kinda helps that the variable is named "result":

   get_syscalls_by_group (gdbarch, group_name, result)

If the result of get_syscalls_by_groups was used for something else than 
being returned, it probably wouldn't have been named "result".  But even 
with that name, I think the & adds a little something that makes it 
obvious at the first eyesight, even before parsing the text:

   get_syscalls_by_group (gdbarch, group_name, &result)

I'm not saying that it's a major impediment to reading the code, just 
one little thing that contribute to readability (IMO).  Again, if people 
believe this rule is more an annoyance than anything else, we can just 
remove it.

Simon