Re: [RFC] Decimal Floating Point support for GDB (Part 1: patch)

[Daniel Jacobowitz <drow@false.org>]

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