From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id c/B4LV6gGmH2EQAAWB0awg (envelope-from ) for ; Mon, 16 Aug 2021 13:29:02 -0400 Received: by simark.ca (Postfix, from userid 112) id A9DAA1EDFB; Mon, 16 Aug 2021 13:29:02 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D0F1F1E4A3 for ; Mon, 16 Aug 2021 13:29:01 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6C014383A832 for ; Mon, 16 Aug 2021 17:29:01 +0000 (GMT) Received: from mx2.freebsd.org (mx2.freebsd.org [IPv6:2610:1c1:1:606c::19:2]) by sourceware.org (Postfix) with ESMTPS id 43BC6385840A for ; Mon, 16 Aug 2021 17:28:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 43BC6385840A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits)) (Client CN "mx1.freebsd.org", Issuer "R3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 1B4B696D08; Mon, 16 Aug 2021 17:28:50 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GpLjs74vsz4qkl; Mon, 16 Aug 2021 17:28:49 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from John-Baldwins-MacBook-Pro.local (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 95C4D4F95; Mon, 16 Aug 2021 17:28:49 +0000 (UTC) (envelope-from jhb@FreeBSD.org) To: Simon Marchi , Simon Marchi via Gdb-patches References: <4bd1cb46-fdcc-cb6a-c7ad-22aba3294772@FreeBSD.org> <00698c41-1af7-b9a8-f127-449a1a06911c@polymtl.ca> From: John Baldwin Subject: Re: Coding standards proposal, usage of "this" Message-ID: <0f7b71be-962e-7bed-e8de-f796ec428c38@FreeBSD.org> Date: Mon, 16 Aug 2021 10:28:48 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <00698c41-1af7-b9a8-f127-449a1a06911c@polymtl.ca> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 8/16/21 10:11 AM, Simon Marchi wrote: > On 2021-08-16 1:06 p.m., John Baldwin wrote: >> On 8/13/21 7:26 AM, Simon Marchi via Gdb-patches wrote: >>> Hi all, >>> >>> Here's something I had in mind for a while. We don't consistently use >>> `this` when referring to fields or methods of the current object. I >>> never now if I should use it or not, or point it out in review. I >>> therefore propose these rules so that we have something to refer to. >>> >>> - Use `this` when referring to a data member that is not prefixed by >>> `m_`. Rationale: without `this`, it's not clear that you are >>> referring to a member of the current class, versus a local or global >>> variable. >>> - Don't use `this` when referring to a data member that is prefixed by >>> `m_`. Rationale: the prefix already makes it clear that you are >>> referring to a member of the current class, so adding `this` would >>> just add noise. >> >> These seem fine to me. >> >>> - Use `this` when referring to a method of the current class. >>> Rationale: without `this, it's not clear that you are referring to a >>> method of the current class, versus a free function. >> >> This one feels a bit odd to me, though it may just be something I'm not >> used to. It is something I haven't seen used before in C++ at least. > > So, the first two seem to be more accepted, and this last one less. I'd > be fine just going with the first two then (even though in my opinion > the reason for using `this` to refer to a non-prefixed data member > applies the same when referring to a non-prefied member function). I would say I'm more on the fence about the third option. One point I almost brought up that you mentioned in your reply to Christian is whether or not this would encourage the use of more static free functions rather than private member methods. This is somewhat interesting since GDB still has a lot of "C" code. For example, in my async FreeBSD series I added a private "wait_1" method to fbsd_nat_target in fbsd-nat.c that is functionally identical to 'linux_nat_wait_1' in linux-nat.c (which is not a member method, but a static free function). My initial thought was that this might discourage adding more private members, but OTOH, right now linux_nat_target::wait() uses 'linux_nat_wait_1' to invoke the helper while my modified fbsd_nat_target::wait() uses just 'wait_1'. Changing it to 'this->wait_1' instead would still be shorter than what is done in linux-nat.c. On the question of static free functions vs private methods, I do think private methods is probably the "right" thing to do for new changes going forward as using C++'s namespacing is cleaner than what was previously done manually in C. I'm not really sure adding 'this' prefixes would discourage this. If you want a case in point to evaluate, perhaps look at the last patch in my FreeBSD async series and the various 'async_file_*' methods I added in inf_ptrace_target for use in subclasses. -- John Baldwin