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 073BB385B836 for ; Mon, 30 Mar 2020 15:57:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 073BB385B836 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.11] (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 988B31E581; Mon, 30 Mar 2020 11:57:13 -0400 (EDT) Subject: Re: [PATCH 15/20] Add attribute::get_unsigned method To: Tom Tromey , gdb-patches@sourceware.org References: <20200328192208.11324-1-tom@tromey.com> <20200328192208.11324-16-tom@tromey.com> From: Simon Marchi Message-ID: <7a56ce4e-28f6-fe7f-2005-0516152e36d1@simark.ca> Date: Mon, 30 Mar 2020 11:57:13 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200328192208.11324-16-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-25.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, URIBL_CSS, URIBL_CSS_A 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: Mon, 30 Mar 2020 15:57:15 -0000 On 2020-03-28 3:22 p.m., Tom Tromey wrote: > This introduces a new attribute::get_unsigned method and changes a few > spots to use it. > > gdb/ChangeLog > 2020-03-28 Tom Tromey > > * dwarf2/read.c (dw2_get_file_names_reader) > (dwarf2_build_include_psymtabs, handle_DW_AT_stmt_list) > (dwarf2_cu::setup_type_unit_groups, fill_in_loclist_baton) > (dwarf2_symbol_mark_computed): Use get_unsigned. > * dwarf2/attribute.h (struct attribute) : New > method. > : Update comment. > --- > gdb/ChangeLog | 10 ++++++++++ > gdb/dwarf2/attribute.h | 11 ++++++++++- > gdb/dwarf2/read.c | 22 +++++++++++----------- > 3 files changed, 31 insertions(+), 12 deletions(-) > > diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h > index 9b387e5df05..546156283b3 100644 > --- a/gdb/dwarf2/attribute.h > +++ b/gdb/dwarf2/attribute.h > @@ -82,9 +82,18 @@ struct attribute > return u.unsnd; > } > > + /* Return the unsigned value. Requires that the form be an unsigned > + form, and that reprocessing not be needed. */ > + ULONGEST get_unsigned () const > + { > + gdb_assert (form_is_unsigned ()); > + gdb_assert (!requires_reprocessing); I see what you're trying to do here, but I don't think it's really useful to assert !requires_reprocessing. For string and address forms that require reprocessing, it's true that we set an unsigned value, but the form is still some string of address form (we don't change it to some unsigned form). So it's not really possible for form_is_unsigned() to return true, and requires_reprocessing to be true. Instead, gdb_assert (!requires_reprocessing) should be added to ::address and ::string. Note that I don't mind if we leave the assert in this function, it's true in any case that requires_reprocessing should be false at this point. Also, I noted that this method is named "get_unsigned", since you obviously can't name it "unsigned". If you used my idea to name the others "as_string" and "as_address", this one could be "as_unsigned", and the names would all be coherent. Simon