From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTP id 0322B385800E for ; Sun, 22 Nov 2020 11:14:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0322B385800E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecke@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 9640C1168AF; Sun, 22 Nov 2020 06:14:08 -0500 (EST) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id kqp7UXlerNhZ; Sun, 22 Nov 2020 06:14:08 -0500 (EST) Received: from tron.gnat.com (tron.gnat.com [205.232.38.10]) by rock.gnat.com (Postfix) with ESMTP id 82AB21165D2; Sun, 22 Nov 2020 06:14:07 -0500 (EST) Received: by tron.gnat.com (Postfix, from userid 4233) id 9EEC4149; Sun, 22 Nov 2020 06:14:07 -0500 (EST) From: Joel Brobecker To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: RFA v2: Various enhancements to the fixed-point support implementation Date: Sun, 22 Nov 2020 06:14:00 -0500 Message-Id: <1606043646-140022-1-git-send-email-brobecker@adacore.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1605430184-81335-1-git-send-email-brobecker@adacore.com> References: <1605430184-81335-1-git-send-email-brobecker@adacore.com> X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_NONE, 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: Sun, 22 Nov 2020 11:14:10 -0000 Hello, This is v2 of the following patch series: - [RFA v2 1/6] change and rename gmp_string_asprintf to return an - [RFA v2 2/6] gmp-utils: Convert the read/write methods to using - [RFA v2 3/6] gdbtypes.h: Get rid of the TYPE_FIXED_POINT_INFO macro - [RFA v2 4/6] Make fixed_point_type_base_type a method of struct type - [RFA v2 5/6] Make function fixed_point_scaling_factor a method of - [RFA v2 6/6] valarith.c: Replace INIT_VAL_WITH_FIXED_POINT_VAL macro The only changes I made were to patch #1 and patch #2, following the commends made by Simon and Pedro: - [RFA v2 1/6] change and rename gmp_string_asprintf to return an Rename the function to gmp_string_printf as recommended, and use the suggestion to rely on gmp_vsnprintf to only allocate the string buffer once. - [RFA v2 2/6] gmp-utils: Convert the read/write methods to using Improve the patch using either the form {ptr, len} to construct the gdb_byte array, or else gdb::make_array_view. The other patches are just plain rebases. Each patch was individually tested on x86_64-linux, with no regression. There are still a number of comments which were made by Simon and Pedro (xfail for m2 and pascal, NEWS entries, selftest error using Ubuntu's GMP, etc), and I will address those next, in separate submissions. Note that I almost dropped patch #5 (turning fixed_point_scaling_factor from function to method), since Simon replied to the discussion about this topic that the the current form (a separate function) was fine. In particular, I tried to think about it in a possible future context where types are a tree of classes rather than one class with a type-code. In that situation, the fixed_point_scaling_factor method would logically be "attached" to the fixed point type class. This in turn means that ranges of fixed point types would not have this method, forcing users to have to call fixed_point_type_base_type first before they can query the fixed_point_scaling_factor method. In other words, because the functionality applies to two types where one is not the child of the other (in terms of class inheritance), a functioned did seem better. However, one could argue that the fixed_point_type_base_type method is in exactly the same boat: It applies to both range types and fixed point. So, in the name of being consistent, this patch series keeps them both. My thinking process is based on a hypothetical scenario, which I'm not even sure ever got discussed here anyways. That being said, now that I write my thoughts, for the case of fixed_point_type_base_type, if we did have a hierarchy of classes, I could perhaps see how all types would have a "base_type" method whose default implementation would simply return the type itself, and were range types would return the target type. In that scenario, the two methods are no longer in the same boat, and we can think of dropping the fixed_point_scaling_factor patch if we wanted to. I do not have a strong opinion and thus can easily drop any patch. The reason I kept the change in the end is that it just seemed more logical to act on what is today, rather than decide based on what the code might be tomorrow. Let me know what you guys think. Thanks, -- Joel