From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 45832 invoked by alias); 26 Nov 2018 18:28:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 45819 invoked by uid 89); 26 Nov 2018 18:28:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:2124, kinda, HContent-Transfer-Encoding:8bit X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Nov 2018 18:28:54 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id wAQISlcN005239 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 26 Nov 2018 13:28:52 -0500 Received: by simark.ca (Postfix, from userid 112) id A9DD21E93F; Mon, 26 Nov 2018 13:28:47 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id D683A1E473; Mon, 26 Nov 2018 13:28:45 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Mon, 26 Nov 2018 18:28:00 -0000 From: Simon Marchi To: Pedro Alves Cc: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/6] invoke_xmethod & array_view In-Reply-To: <29ddaf6e-1700-4464-fb91-c3dc307b2e7f@redhat.com> References: <20181015151115.6356-1-palves@redhat.com> <20181015151115.6356-3-palves@redhat.com> <245315f6-dd74-b533-f998-762dbf181a54@ericsson.com> <8947e861-0974-f6b4-f5a8-a181df210e08@redhat.com> <43be2c078bbfbcccd8ab3ec181a8813c@polymtl.ca> <29ddaf6e-1700-4464-fb91-c3dc307b2e7f@redhat.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00444.txt.bz2 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