JDK 9 RFR of 8043740: Doubles with large exponents overflow to Infinity incorrectly

[Joe Darcy]

Hello,

Do the additional cases in the regression tests full cover the proposed 
revision of the code changes?

Thanks,

-Joe

On 9/20/2014 4:06 PM, Sandipan Razzaque wrote:
> Hi Brian -
>
> Thanks for your review!
>
> I think your point about adding !expOverflow to that conditional makes
> perfect sense. We're only looking to account for expVal > expLimit where
> decExp would be adjusted downward. Please adjust as appropriate.
>
> Cheers,
> SR
>
>
>
>
>
> Sandipan Razzaque | www.sandipan.net
>
> On Fri, Sep 19, 2014 at 4:54 PM, Brian Burkhalter <
> brian.burkhalter at oracle.com> wrote:
>
>> Hello Sandipan,
>>
>> Finally got this off the back burner …
>>
>> This review request follows this thread:
>>
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027086.html
>>
>> in which you provided a patch (thank you!) for:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8043740
>>
>> I’ve created an updated webrev here:
>>
>> http://cr.openjdk.java.net/~bpb/8043740/webrev.00/
>>
>> Aside from minor reformatting there is no update to the proposed
>> FloatingDecimal change. I have not however included the test class
>> Bug8043740 from the contributed patch opting instead to update the existing
>> ParseDouble test by adding a few more strings to the goodStrings array.
>>
>> The changes to FloatingDecimal appear reasonable to me. I am wondering
>> however if lines 2001-2002 should not be changed to include !expOverflow in
>> the conditional:
>>
>> 2001                     if (!expOverflow && expSign == 1 && decExp < 0
>> 2002                             && (expVal + decExp) < expLimit) {
>> 2003                         // Cannot overflow: adding a positive and negative number.
>> 2004                         decExp += expVal;
>>
>> I don’t think that it’s possible for both expOverflow and the conditionals
>> at lines 2001-2002 of the webrev to all be true, but the additional test
>> would guarantee branching to the correct block.
>>
>> Thanks,
>>
>> Brian
>>
>> On Jun 2, 2014, at 6:08 AM, Sandipan Razzaque <me at sandipan.net> wrote:
>>
>> I've made a quick revision to that last patch. Please find inline the
>> latest link + patch.
>> http://www.sandipan.net/public/webrevs/8043740/webrev.01/
>>
>>
>>
>>