From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 334D83858C50 for ; Fri, 10 Feb 2023 02:54:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 334D83858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 7DF6C1E110; Thu, 9 Feb 2023 21:54:18 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1675997658; bh=mnZBUpMZBoDCq/xdEI6tJA6i3fsK02xbyviDErdnRbc=; h=Date:Subject:To:References:From:In-Reply-To:From; b=l0MV8duBEH3sVsgV0XFao9J/qv3TEzSfMYvVHmt5hOVAtm5yYDQHj2zWfpMDon/AN Qn5BqYdRsFJv/HbS3ZKFE899VycP470HvrGnM1su+0CSjvhOnDkOExLLVg3uu320RU /ZWPDiThXkglNtEAgclo1x6KWpS47wALipNKDy/U= Message-ID: <43ad228f-55d0-d1b4-8dd3-20fec653967e@simark.ca> Date: Thu, 9 Feb 2023 21:54:18 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH 00/47] Use methods for struct value Content-Language: en-US To: Tom Tromey , gdb-patches@sourceware.org References: <20230209-submit-value-fixups-2023-v1-0-55dc2794dbb9@tromey.com> From: Simon Marchi In-Reply-To: <20230209-submit-value-fixups-2023-v1-0-55dc2794dbb9@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2/9/23 16:38, Tom Tromey wrote: > This series changes many value-related functions to be methods. It > also moves the definition of struct value to value.h, so that callers > will benefit by inlining the smaller methods. The members of value > are all made private at the end of the series. > > It's difficult to decide exactly which value functions ought to be > converted, because there are so many. In the end I chose a relatively > minimal approach -- only those that require direct access to the inner > workings of value. > > It's possible, even easy, to convert more functions. If there are > some in particular that seem worthwhile, let me know. > > I tacked on one small cleanup at the end of this series, but as it was > already fairly long, I didn't try to do all the possible cleanups. > For example, many things could use bool, some of the setter functions > could be removed in favor of specialized constructors, etc. > > Regression tested on x86-64 Fedora 36. > > Tom > > PS - I'm testing the "b4 send" command, so my apologies if something > goes wrong. I was able to fetch it fine using "b4 am". However, it told me the DKIM signature for "tromey.com" was incorrect :P. I went relatively quickly over the whole series. I concentrated on changes in value.h, since the rest is fairly mechanical. It looks really nice, I had started to do that a little bit of these changes, but I was sooo far away from what you have here. If you want to go over the series again quickly, there are a few comments in value.h that refer to now non-existent parameters (which because "this"). They could be reworded to talk about "this value" instead. I agree with the instances where you removed "deprecated" from some functions. I guess some refactoring work was started many moons ago to get rid of them, but wasn't completed, and now the context is completely lost. It's pointless to call something deprecated but also leave it there forever. Maybe others will want to take a look, but from my side: Approved-By: Simon Marchi Simon