Code review request for #6469160, #7088271

[David Holmes]

Hi Brandon,

On 25/01/2012 5:03 PM, Brandon Passanisi wrote:
> Hi David. Thank you for your review comments. My answers to your
> questions are below:
>
> On 1/23/2012 9:56 PM, David Holmes wrote:
>> More generally it is not clear to me that putting in this special case
>> is the right way to fix this. Though I admit I don't really understand
>> what the specification requires when you give a precision of 0 with a
>> 'g' conversion:
>>
>> "If the conversion is 'g' or 'G', then the precision is the total
>> number of digits in the resulting magnitude after rounding."
>>
>> So we asked for zero digits? What should that mean?
>
> The Formatter javadoc, within the "Float and Double" section, describes
> the following regarding a value of 0 for precision and the 'g'/'G'
> conversion [1]:
>
> "If the precision is 0, then it is taken to be 1"

Ah! Thanks. I hadn't seen that (wish they wouldn't split up the spec for 
this across different sections!).

Okay that explains the 0/1 situation.

But that makes me question the "rightness" of the fix even more. We took 
steps to change a precision of 0 to 1, but then the fix changes it back 
to 0 because otherwise something else breaks. Seems to me that the 
"something else" that handles the precision of 1 incorrectly is the code 
that really needs to be fixed. Further it suggest that there may be 
assumptions in later code that precision is in fact never 0.

David
-----


> The following code block within Formatter.java near line 3278 is run to
> do this:
>
> if (precision == -1)
> prec = 6;
> else if (precision == 0)
> prec = 1;
>
> And the precision value "prec" is given to FormattedFloatingDecimal.
> Therefore, when this particular error condition is seen and the proposed
> code fix is reached in FormattedFloatingDecimal.java, the precision will
> be 1. So, the proposed code fix ends up supporting a precision value of
> 0 and 1.
>
>
> [1]: http://docs.oracle.com/javase/7/docs/api/java/util/Formatter.html
>
>
> Thanks.
>
>>
>> David
>> -----
>>
>>> Since java has been throwing exceptions in these cases, I consulted with
>>> the output of C's printf to make sure that the outputted strings are the
>>> same. I updated the Formatter's Basic-X template of tests with a little
>>> over 20 test format strings that were causing exceptions without the
>>> change and the output of each is compared with the output from C's
>>> printf with the same format string. And, I ran all of the Basic-X tests
>>> to make sure there weren't any regressions in behavior.
>>>
>>> Thanks.
>