On Feb 6, 2015, at 2:43 AM, Brian Burkhalter <brian.burkhal...@oracle.com>
wrote:
> Hi Paul,
>
> On Feb 5, 2015, at 6:06 AM, Paul Sandoz <paul.san...@oracle.com> wrote:
>
>> I don't claim to understand the fine details of these methods but i can see
>> how the new method avoid loosing bits.
>>
>> 4947 private static long[] divRemNegativeLong(long n, long d) {
>> 4948 if (n >= 0) {
>> 4949 throw new IllegalArgumentException("Non-negative
>> numerator");
>> 4950 } else if (d == 1) {
>> 4951 return new long[]{0, n};
>> 4952 }
>>
>> Why not use an assert instead of an IAE since this is an internal method.
>
> I had actually wondered whether this could just be a “trusted” method since
> the check of the first parameter is already done in the calling code, then
> the if-branch could be removed altogether, but I suppose the assert is safer.
Yes, asserts are good here, they will be enabled when testing.
>
>> Also the case of d ==1 could be pulled out just like for the case of tmp
>> being +ve:
>>
>> if (v1 == 1) {
>> q1 = tmp;
>> r_tmp = 0;
>> } else if (tmp >= 0) {
>> ...
>> } else {
>> ...
>> }
>>
>> then the asserts would be:
>>
>> assert n < 0 : n;
>> assert d != 1 : d;
>
> Thanks for the suggestions. Please see updated patch:
>
> http://cr.openjdk.java.net/~bpb/8066842/webrev.01/
>
+1, but you should remove the "@throws IAE" on divRemNegativeLong before you
push.
Paul;.
