From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30510 invoked by alias); 2 Oct 2005 21:00:11 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 30496 invoked by uid 22791); 2 Oct 2005 21:00:07 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sun, 02 Oct 2005 21:00:07 +0000 Received: from drow by nevyn.them.org with local (Exim 4.52) id 1EMAtg-0008Um-OG; Sun, 02 Oct 2005 16:57:24 -0400 Date: Sun, 02 Oct 2005 21:00:00 -0000 From: Daniel Jacobowitz To: Wu Zhou Cc: gdb-patches@sources.redhat.com Subject: Re: [RFC] Decimal Floating Point support for GDB (Part 1: patch) Message-ID: <20051002205724.GB31820@nevyn.them.org> Mail-Followup-To: Wu Zhou , gdb-patches@sources.redhat.com References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.8i X-SW-Source: 2005-10/txt/msg00001.txt.bz2 On Wed, Sep 28, 2005 at 12:42:36PM +0800, Wu Zhou wrote: > This patch is intended to add decimal floating point (DFP) support into > GDB. It currently supports the printing and setting of DFP. And also > support displaying the dfp types in function argument and backtrace. > If you think that any other features are also desired, please let me know. > I will try to see whether it can also be supported. I'm afraid I am not very happy with this approach :-( * c-exp.y (parse_number): Parse the decimal floating point, which has a suffix ('df', 'dd' or 'dl') and return STRING here. This certainly isn't right. It produces several problems: (gdb) p 1.2df evaluation of this expression requires the target program to be active [Side effect of string handling] ... (gdb) p 1.2df $1 = "1.2" (gdb) p $1[1] $2 = 46 '.' (gdb) p (1.2df) A syntax error in expression, near `'. If we have a GDB type system representation for decimal floats, the C parser should use it, and for lexing purposes it should probably be a FLOAT. I'm not familiar with the proposed C language changes, but GDB should plan to support them properly; the same set of implicit or invalid conversions, supported operations, et cetera. We certainly don't need to implement all that; the C interpreter is pretty casual. But let's consider them at least. And maybe issue intelligent error messages. > * dfp.h: New header file for decimal floating point support in GDB. > main functions is decimal_from_string and decimal_to_string. > * dfp.c: New source file for decimal floating point support in GDB. > Implement decimal_from_string and decimal_to_strin. The decNumber library is going to be (or already is) contributed to the FSF and GPL-licensed. In that case, perhaps we can move it to the top level of the repository, and use it to implement support in GDB. The alternative is adding a lot of sensitive numeric code to GDB that the GDB developers won't know much about. > * valops.c (value_set_dfp): New function, calling decimal_from_string > to set the value of decimal floating point types. This is wrong for a couple of reasons. I think you're doing this to avoid the cast to arg1's type in evaluate_subexp_standard; but in the process you've ignored "noside" and binop_user_defined_p (assuming eventual C++ support), and you're assuming that fromval is a string. Which means that 'dfp_var1 = dfp_var2' is going to crash. And 'dfp_var1 = "1.2"' would probably work, which is unlikely to be what you wanted. I recommend representing decimal floating point numbers in GDB as decimal floating point numbers, not as strings. Then it will be pretty clear where in e.g. valarith.c to handle it. Or, initially, reject it. Oh, and in a couple of places your code says: + Contributed by Cygnus Support, using pieces from other GDB modules. which is not true since you're contributing it :-) -- Daniel Jacobowitz CodeSourcery, LLC