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 23D3B388E838 for ; Sat, 16 May 2020 16:15:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 23D3B388E838 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.193] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 6E5FE1ED39; Sat, 16 May 2020 12:15:04 -0400 (EDT) Subject: Re: [PATCH] Make BLOCK_START and BLOCK_END into rvalues To: Tom Tromey , gdb-patches@sourceware.org References: <20200516151021.7080-1-tom@tromey.com> From: Simon Marchi Message-ID: Date: Sat, 16 May 2020 12:15:03 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200516151021.7080-1-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Language: tl Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-List-Received-Date: Sat, 16 May 2020 16:15:06 -0000 On 2020-05-16 11:10 a.m., Tom Tromey wrote: > This changes the BLOCK_START and BLOCK_END macros to expand to > rvalues, and then updates some code to assign to these fields > directly. > > I considered adding setters here, but that seemed a bit needless, > given that these are "open" structures. However, I can do that if > it's preferable. Note that when the time comes to remove the > BLOCK_START and BLOCK_END macros, I think they will be replaced with > methods, because that will make it simpler to achieve objfile > independence for blocks. This is similar to what I'm doing to struct type, so it would be nice to agree on the way forward for all these structures, and change them in a consistent manner. What I'm doing/considering for struct type is to add getters / setters for all properties. The fields themselves should really become private, but it's probably not possible to do it right now, since the structures need to remain POD. I think we could still prefix them with `m_` while leaving them public, doing so will make sure to catch any place that was accessing the field directly, not using the macro. And when writing new code, it will be semi-obvious by the naming that you shouldn't access the field directly. Also, the getter can't have the same name as the field, so in some cases that would be another reason to rename the field. Although in the case of this patch, I would name the getters simply `start` and `end`, just like the macros are named `..._START` and `..._END`, so there's no conflict (we could still rename the fields to `m_start` and `m_end` for consistency). Since you are changing the places that modify the field values, I'd suggest adding the setter right away, it's a step in the right direction. And I'd suggest adding the getter too, changing the BLOCK_START macro to use it, it shouldn't be much more work. And it will achieve the goal of making the macro yield an rvalue. If you want, I can look into removing the macros after that, like I do for the type macros. Simon