This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] A patch for parse_number (c-exp.y) to recognize 1.25f
On Thu, Sep 15, 2005 at 04:05:48PM +0800, Wu Zhou wrote:
>
> > To fix this kind of false-negative and not to introduce false-positive, I
> > am thinking of changing the second format in sscanf to "%s" (string).
> > Thus the parser could get the trailing characters following the float.
> > Then we can add some code to parse them to eliminate false-positive. Any
> > points on this idea? Is it worthwhile to do this? Please comment. TIA.
>
> Following the above point, I coded another patch. It can fix the problem
> and don't introduce any false-positive mentioned above. I had also coded
> a testcase for that. Had tested on i386. With the patched GDB, it
> reported 18 PASS; with the original GDB, it report 9 PASS and 9 FAIL (all
> of these failure is due to the fact that GDB can't handle the suffix
> following the float constants.)
>
> IMHO, there are yet another advantage for this patch: it is easy to be
> extended to handle other suffixes, such "df" for _Decimal32, "dd" for
> _Decimal64 and "dl" for _Decimal128, all of which are described in IEEE
> 754R for decimal floating point.
>
> Here goes the patch and the testcase.
This looks good. I have some comments on formatting and testing, but
that's it.
> 2005-09-15 Wu Zhou <woodzltc@cn.ibm.com>
>
> * c-exp.y (parse-number): Modify the float parsing logic to let it
> recognize the suffix.
"a suffix" rather than "the suffix", please.
> ! if (num == 1)
> ! putithere->typed_val_float.type = builtin_type (current_gdbarch)
> ! ->builtin_double;
Instead of splitting this line (and the two below) in the middle of a
single reference, you can split it at the assignment. Like this - two
spaces in the continued line:
putithere->typed_val_float.type
= builtin_type (current_gdbarch)->builtin_double;
Emacs'll butcher your version, I'm afraid.
> Index: gdb.base/bfp-test.c
> ===================================================================
> RCS file: gdb.base/bfp-test.c
> diff -N gdb.base/bfp-test.c
> *** /dev/null 1 Jan 1970 00:00:00 -0000
> --- gdb.base/bfp-test.c 15 Sep 2005 06:49:51 -0000
> ***************
> *** 0 ****
> --- 1,15 ----
It's a pretty trivial test case; but please add a copyright notice
anyway.
> + # Run to the breakpoint at return.
> + set bp_location [gdb_get_line_number "return"]
> + gdb_test "break $bp_location" \
> + "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> + "breakpoint at return"
> + gdb_test "continue" \
> + "Continuing\\..*Breakpoint.*" \
> + "continue to breakpoint"
This is fine, but you can also use gdb_breakpoint and
gdb_continue_to_brekpoint or gdb_continue.
> + # Test that gdb could handle the above correctly with "set var" command.
> + send_gdb "set var b32=10.5f\n"
> + gdb_expect {
> + -re "$gdb_prompt $" { pass "set var b32=10.5f" }
> + -re "Invalid number" { fail "do not recognize 10.5f" }
> + timeout {fail "set var b32=10.5f" }
> + }
Please don't use "send_gdb" and "gdb_expect" directly. Also, the
passing and failing tests should have the same message - an optional
string at the end in parentheses explaining the failure is OK, but
anything else is bad for automated testing.
gdb_test_multiple should work OK everywhere you used send_gdb.
> + # Test that gdb could handle invalid suffix correctly.
> + send_gdb "set var b32=100.5a\n"
> + gdb_expect {
> + -re "$gdb_prompt $" { fail "don't report error on 100.5a" }
> + -re "Invalid number" { pass "100.5a is invalid number" }
> + timeout {fail "set var b32=100.5a" }
> + }
... although all of the invalid tests could just use gdb_test.
--
Daniel Jacobowitz
CodeSourcery, LLC