From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 79345 invoked by alias); 26 Nov 2018 17:54:07 -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 79331 invoked by uid 89); 26 Nov 2018 17:54:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=201803, 2018-03, helping, angle X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Nov 2018 17:54:05 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 265102D2BF2; Mon, 26 Nov 2018 17:54:04 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 442BE6090E; Mon, 26 Nov 2018 17:54:03 +0000 (UTC) Subject: Re: [PATCH 2/6] invoke_xmethod & array_view To: Simon Marchi 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> Cc: Simon Marchi , gdb-patches@sourceware.org From: Pedro Alves Message-ID: <29ddaf6e-1700-4464-fb91-c3dc307b2e7f@redhat.com> Date: Mon, 26 Nov 2018 17:54:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <43be2c078bbfbcccd8ab3ec181a8813c@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-11/txt/msg00443.txt.bz2 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