Re: [RFC] A patch for parse_number (c-exp.y) to recognize 1.25f

[Wu Zhou <woodzltc@cn.ibm.com>]

Hi Daniel,

> This looks good.  I have some comments on formatting and testing, but
> that's it.

Thanks for your comment. They are very helpful and informative indeed.

> > 
> > 	* c-exp.y (parse-number): Modify the float parsing logic to let it 
> > 	recognize the suffix.
> 
> "a suffix" rather than "the suffix", please.

Changed.

> 
> > !       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.

I am not familar with Emacs.  But I believe that you are right.  Fixed.

[snip]
> 
> It's a pretty trivial test case; but please add a copyright notice
> anyway.

Added.

> 
> > + # 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.

Switched to gdb_breakpoint and gdb_continue_to_brekpoint.

> 
> > + # 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.

Good guideline.  I am now using gdb_test_multiple.

I had run the modified testcase against patched and un-patched GDB. It 
report expected test result.  Both the patch and testcases are checked in.  

Thanks.

Regards
- Wu Zhou