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 DDB353861C30 for ; Thu, 19 Mar 2020 21:24:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DDB353861C30 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 [172.16.0.95] (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) (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 356671E5F8; Thu, 19 Mar 2020 17:24:45 -0400 (EDT) Subject: Re: [PATCH] Fix Ada val_print removal regression To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20200317180034.26934-1-tromey@adacore.com> <6a432b2a-4ae9-101d-32c3-efee7da031d1@simark.ca> <874kuksb4q.fsf@tromey.com> <87a74cp1w4.fsf@tromey.com> <87zhccnj42.fsf@tromey.com> From: Simon Marchi Message-ID: <6e0acd7d-a2a0-627d-ef3b-9abf0dc35687@simark.ca> Date: Thu, 19 Mar 2020 17:24:44 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <87zhccnj42.fsf@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Language: tl Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=0.0 required=5.0 tests=KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS 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: Thu, 19 Mar 2020 21:24:46 -0000 On 2020-03-19 5:13 p.m., Tom Tromey wrote: >>> + struct value *variant = value_field (value, which); > > Simon> A bit of a nit, but it's to make sure I understand what's > Simon> happening. From what I understand, the variant is the enclosing > Simon> type, from which only one component is active at a given time. > Simon> This value variable represents the active component, right? > > Yep. > > Simon> If > Simon> so, I'd suggest naming it active_component or something like > Simon> that. > > Done. > > Simon> The sole caller of this function passes the same `val` twice, so > Simon> I suppose you could remove `original_value`. > > I did this too. > > Tom Thanks, I think this is fine. Simon