[9] RFR of 8066842: java.math.BigDecimal.divide(BigDecimal, RoundingMode) produces incorrect result

[Paul Sandoz]

Hi Brian.

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

Paul. 

On Jan 16, 2015, at 10:18 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:

> Hello,
> 
> Please review at your convenience.
> 
> Issue:	https://bugs.openjdk.java.net/browse/JDK-8066842
> Patch:	http://cr.openjdk.java.net/~bpb/8066842/webrev.00/
> 
> The problem appears to have been that at line 4941 of the old source, in the divWord() method, one or both of the long variables ‘r’ and ‘q’ overflowed the range of int so that information was lost when these variables were truncated to 32 bits. The code of divWord() seems to have been ported from a method of the same name in MutableBigInteger wherein its constraints were made explicit. In the patch, divWord() is replaced by divRemNegativeLong(), and the use of the former in divideAndRound128() replaced with inline code for the non-negative dividend cases, and by divRemNegativeLong() for the negative dividend cases.
> 
> Thanks,
> 
> Brian