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 31591385702E for ; Sat, 14 Nov 2020 13:20:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 31591385702E 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] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (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 83AEF1E58F; Sat, 14 Nov 2020 08:20:15 -0500 (EST) Subject: Re: [PATCH 5/9] Add support for printing value of DWARF-based fixed-point type objects To: Joel Brobecker Cc: GDB patches References: <1604817017-25807-1-git-send-email-brobecker@adacore.com> <1604817017-25807-6-git-send-email-brobecker@adacore.com> <2d12525a-3a85-3f69-bfea-22166f7fd358@simark.ca> <20201114104829.GA404828@adacore.com> From: Simon Marchi Message-ID: Date: Sat, 14 Nov 2020 08:20:14 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20201114104829.GA404828@adacore.com> Content-Type: text/plain; charset=windows-1252 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, NICE_REPLY_A, 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, 14 Nov 2020 13:20:17 -0000 On 2020-11-14 5:48 a.m., Joel Brobecker wrote: > Hi Simon, > >>> + of the corresponding TYPE by setting its TYPE_FIXED_POINT_INFO. >>> + CU is the DIE's CU. */ >>> + >>> +static void >>> +finish_fixed_point_type (struct type *type, struct die_info *die, >>> + struct dwarf2_cu *cu) >> >> Unless there's a good reason not to (coming up in the following >> patches), I would make this function create the "struct type", instead >> of having the caller create it and pass it. In other words, this >> signature: >> >> struct type *create_fixed_point_type (die_info *die, dwarf2_cu *cu) >> >> That just makes it more obvious that it's a simple "die" to "type" >> transform. > > While implementing this suggestion, I'm realizing that doing so > requires either: > > (1) Recomputing the following information for the CU: encoding, > bitsize if any, and type name, which means: > > | attr = dwarf2_attr (die, DW_AT_encoding, cu); > | if (attr != nullptr && attr->form_is_constant ()) > | encoding = attr->constant_value (0); > | attr = dwarf2_attr (die, DW_AT_byte_size, cu); > | if (attr != nullptr) > | bits = attr->constant_value (0) * TARGET_CHAR_BIT; > | name = dwarf2_name (die, cu); > | if (!name) > | complaint (_("DW_AT_name missing from DW_TAG_base_type")); > > Or: > > (2) Passing that information as arguments to the function. > > I find option (1) really sad because of the code duplication. > And I find option (2) somewhat unsatisfactory, because we then > run the risk of passing inconsistent information. > > This might explain partly why the init + finish approach isn't > new in this unit... > > I'm attaching the patch which shows how option (1) looks like. > For me, the current approach strikes a better balance between > API cleanliness and implementation considerations. But I don't > have a strong opinion against implementing what you suggest, > including through option (2) (or other options I missed). > > Let me know what you think. > > -- > Joel > Ok I see, well that convinces me the original code is fine. Simon